Giter Site home page Giter Site logo

Comments (39)

mercmobily avatar mercmobily commented on April 28, 2024 4

I am sorry, there was a communication issue here.
I closed this issue because this issue is about the module not working on Node 14 or lower -- which is... not true! I should have called it "Inept development (me) is trying to use RC1 instead of RC4"... which didn't sound as flattering for me!

Plus:

I guess we will talk about it in 2034...

This was NOT a disgruntled complaint! It was really my forecast. What I should have written is:


that feature is released, unflagged, in every supported node version, and a few years have passed,

I guess that's going to be around 2034 at least. So, at least for now, I assume you should revert the module to CJS and wait a few years...


In either case, the issue at hand (it doesn't work with modern Node) really is resolved, which is why I closed it.
[Plus, I think it would be better to have future important discussions on a more suitable issue, especially since the issue I created is basically fixed with "Just install RC4"... :D

Sorry for the noise and for the misunderstanding. But please rest assured that I am not "that" kind of user/developer...!

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 3

If your company is in need of this update, I would suggest sponsoring the folks working on this stuff.

I didn't think of this. I will ask.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 2

I am normally the first guy who puts his hand up to help in any way -- especially a project as awesome as Express. But, at the moment I am in Spain (not my country) and extremely time-strapped... I am sorry.
I didn't realise (or, better, I had forgotten) Multer was part of the Express ombrella. I am sorry if I was a pain when I created this ticket... I think the most important thing to do is to make sure that it uses a new version of fs-temp (2.0 fixes the Node Version problem) and update any old packages that can create security problems.

Sorry if I can't be of much more help.

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024 2

Being ESM (both v2 here and in fs-temp) is likely going to be an issue isn't it? I don't see any tooling to dual publish, and as a project we do not have guidance on publishing CJS/ESM/dual but I think it is likely that we need that. Obviously I wouldn't want to block active work, but I would be quite disappointed if the project stopped supporting CJS users.

from multer.

ljharb avatar ljharb commented on April 28, 2024 2

@mercmobily https://majors.nullvoxpopuli.com/q?packages=got shows the problem more clearly.

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024 2

Hey @mercmobily, I understand maybe you are frustrated here but this kind of thing is not helpful. We are trying to get more contributors on boarded to the project to help prevent the situation these packages are in from continuing or ever happening again. If you would like to work with us on that we would gladly have your help. But closing the issue with that type of comment is not productive.

@joeyguerra If I understand the situation correctly @LinusU was interested in still working on this but has been pretty busy. Maybe we could get just some of his time soon to help you and onboard so we can start getting some of this stuff fixed up? I personally need to focus on the v5 express release, but I believe that having this package updated for current node versions and then also ready to support v5 is quite important. If you are interested in helping make this side of that work happen it would be AWESOME and very helpful.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 2

Seriously though, we are using on a huge production project, and it's worked really well for years. Coming up to 7 now.

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024 2

Yeah, the impact of these projects the Express org owns is quite large. I guess I should have mentioned above on the topic of user outreach that we really don't need to do that to get a feel for usage. As an example to add on to @mercmobily's usage, I just checked and we have about 20 apps installing multer with commits in the last year at Netflix. This package is absolutely a critical dependency to keep working and healthy.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 1

and that's likely to continue forever (or until the JS spec and node and browsers agree to make a change that allows requiring ESM

You are right.

Not being able to "require" EMS from CJS is what makes packages tragic. I think they assumed there would be a huge wave of EJS and CJS would have been a "thing of the past" much more quickly than is actually happened (or is actually happening)

from multer.

yeya avatar yeya commented on April 28, 2024 1

you can look at literally any of their per-major-version download numbers to see how few users actually end up adopting ESM-only versions.

Check got for example here

There is 6M weekly downloads for v11 that is "no longer maintained and we will not accept any backport requests"!
Just because it is the latest version before moving to ESM.

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024 1

@mercmobily I've been reviewing the code and the rc 2 version. fs-temp is a dev dependency and appears to be only used when running the tests. They all pass in Node 16 and 20+. I'm curious about the scenario the app your running is in so we can prioritize work on this repo better.

RE: the multer Release Candidate is written as an ESM: For refrence, I tested the following code with [email protected] on Node version v16.20.2 on MacOS.

var express = require('express');
var app = express();
var http = require('http').Server(app);
var fs = require('fs');
import('multer').then(multer => {
    multer = multer.default
    var upload = multer()

    app.get('/', async (req, res) => {
        res.sendFile(__dirname + '/testing.html')
    })
    app.post('/', upload.single('small0'), async (req, res) => {
        req.file.stream.pipe(fs.createWriteStream(`uploads/${req.file.originalName}`))
        res.send('ok')
    })
    http.listen(3001)
    console.log('Server listening on port http://localhost:' + http.address().port)
})

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 1

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 1

There is a certain anti-ESM sentiment. I had some discussions with the Express guys. They are worried that adoption and upgrading will slow down if it's released as an ESM.
However... are you aware of this that was recently merged into Node? nodejs/node#51977
This is behind a flag, but with a bit of luck, we will have the last obstacle out of the way....

from multer.

mercmobily avatar mercmobily commented on April 28, 2024 1

Among many things, it shows that at least 1 person is using multer and express for something valuable and I think that’s amazing.

Well... what can I say, I feel I am representing one of the 4 mil people who downloaded multer this week!!!

image

from multer.

UlisesGascon avatar UlisesGascon commented on April 28, 2024

Hi @mercmobily! Thanks for open this issue, currently there is a plan to maintain this module and many others from the Express.js ecosystem, see: expressjs/discussions#160. I will check the multer status with the TC team, so we can prioritize it. 👍

BTW feel free to join us in the #express channel in the OpenJS foundation Slack if you want to get more involved in the project :-)

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024

Additionally to what @UlisesGascon mentioned above, if you would like to help us pick things back up and get development revived we would love your help! If It looks like you might have a fix in mind, would you be able to open a PR to fix that?

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024

No worries at all @mercmobily! This is a larggely volunteer effort and we totally understand time constraints. And it is not a pain at all, we rely on community members raising issues like this to know what to focus on, keep doing so!

make sure that it uses a new version of fs-temp (2.0 fixes the Node Version problem) and update any old packages that can create security problems.

We are working toward updates which would unblock some major version upgrades in these packages and will see if this is one of those blocked by old version support. Just might not be something we can get to immediately without someone to volunteer to steward it. Maybe @LinusU can, but I think he is pretty busy right now so might not be available (again, it's all volunteer work so this is just sometimes how it goes). If your company is in need of this update, I would suggest sponsoring the folks working on this stuff.

from multer.

jonchurch avatar jonchurch commented on April 28, 2024

For reference, fs-temp is also maintained by Linus.

Here is the changelog for 2.0.0

Of note, minimum node support is 12.20.0 and the package is now ESM.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

Any CJS project can import/use an ESM project. That's why there is no required tooling to publish both. In fact, using an ESM module from a CJS project requires tricketr, whereas using a CJS module from an EJS project is basically completely transparent. Talking about "supporting CJS developers" in 2024, given the above, isn't really that meaningful.

Same for node support: anybody using Node 10 really, and has reached EOL since April 2021.

Please note that many, many NPM package maintainers have indeed moved to ESM without even telling anybody -- and nobody basically realised, even CJS projects, since the usage was still identical.

from multer.

ljharb avatar ljharb commented on April 28, 2024

@mercmobily not synchronously it can't - only with import(), which is async.

importing CJS in node Just Works, period - so it's the easiest possible thing.

from multer.

ljharb avatar ljharb commented on April 28, 2024

It is a breaking change to go ESM-only, and for packages that dual publish, CJS users are using the CJS, not the ESM, which means they didn't "move" to ESM. The small number of maintainers that have gone ESM-only have caused tons of pain for users, and you can look at literally any of their per-major-version download numbers to see how few users actually end up adopting ESM-only versions.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

Ah yes, I am thinking more like a consumer, rather than a provider, in terms of produced software...

You are right in that developers using CJS will find it painful to use Multer if it's only available as ESM. Multer being something used by loads of people... the sheer number of people you upset is enough to create riots!

Dual publishing is also hell-ish and non-trivial.

At the same time, EMS is the way to go, and the number of EJS-only projects will be more and more.

For something like Multer I agree, it's a very difficult situation.

(As a side note, I actually ported a huge enterprise application to EJS in less than a day, when I thought that Multer 2 was going to be released shortly!).

[Edit: I wrote Express instead of Multer... multiple times!]

from multer.

ljharb avatar ljharb commented on April 28, 2024

"ESM is the way to go" is a statement that I've yet to see any compelling evidence behind. You can author in ESM all you like (and use a transpiler), but why should anyone care if you ship that ESM?

Also, "will be more and more" - i disagree. It's already increasing VERY slowly for syntax that everyone has used for a decade and that everyone's excited about, and imo that's entirely because using it in packages hurts users. Do it in your apps, certainly! but there's just no benefit to doing it in a package, and tons of downsides, and that's likely to continue forever (or until the JS spec and node and browsers agree to make a change that allows requiring ESM)

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

you can look at literally any of their per-major-version download numbers to see how few users actually end up adopting ESM-only versions.

Check got for example here

There is 6M weekly downloads for v11 that is "no longer maintained and we will not accept any backport requests"! Just because it is the latest version before moving to ESM.

I am not sure this is a good example. After a quick awk/openoffice, you can see that version 11 has 8,025,208
weekly downloads, while all newer versions (12 up) have 3,455,238 weekly downloads.

I didn't count older versions -- if somebody is using version 10 or lower, they wouldn't be upgrading either way.

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

@mercmobily can you please describe the scenario? I want to write a failing test and submit a fix for it.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

It took me a while to make it happen...
Here is the full stack.

If you look at the patch I am manually running, it boils down to path not accepting null anymore. fs-temp has been fixed ever since, but you guys are using an old version.

I don't think only tests use it -- this line in the stack below shows:

    at Busboy.<anonymous> (/home/merc/Development/compAs/server/node_modules/multer/lib/read-body.js:70:27)

Full stack:

UNHANDLED REJECTION! TypeError: The "path" argument must be of type string or an instance of Buffer or URL. Received null
    at TempWriteStream.WriteStream (node:internal/fs/streams:340:5)
    at new TempWriteStream (/home/merc/Development/compAs/server/node_modules/fs-temp/lib/write-stream.js:6:15)
    at Object.createWriteStream (/home/merc/Development/compAs/server/node_modules/fs-temp/lib/temp.js:121:10)
    at Busboy.<anonymous> (/home/merc/Development/compAs/server/node_modules/multer/lib/read-body.js:70:27)
    at Busboy.emit (node:events:514:28)
    at Busboy.emit (/home/merc/Development/compAs/server/node_modules/busboy/lib/main.js:37:33)
    at PartStream.<anonymous> (/home/merc/Development/compAs/server/node_modules/busboy/lib/types/multipart.js:214:13)
    at PartStream.emit (node:events:514:28)
    at HeaderParser.<anonymous> (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:50:16)
    at HeaderParser.emit (node:events:514:28)
    at HeaderParser._finish (/home/merc/Development/compAs/server/node_modules/dicer/lib/HeaderParser.js:68:8)
    at SBMH.<anonymous> (/home/merc/Development/compAs/server/node_modules/dicer/lib/HeaderParser.js:40:12)
    at SBMH.emit (node:events:514:28)
    at SBMH._sbmh_feed (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:159:14)
    at SBMH.push (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:56:14)
    at HeaderParser.push (/home/merc/Development/compAs/server/node_modules/dicer/lib/HeaderParser.js:46:19)
    at Dicer._oninfo (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:196:25)
    at SBMH.<anonymous> (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:126:10)
    at SBMH.emit (node:events:514:28)
    at SBMH._sbmh_feed (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:188:10)
    at SBMH.push (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:56:14)
    at Dicer._write (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:108:17)
    at writeOrBuffer (node:internal/streams/writable:556:12)
    at _write (node:internal/streams/writable:490:10)
    at Writable.write (node:internal/streams/writable:494:10)
    at Multipart.write (/home/merc/Development/compAs/server/node_modules/busboy/lib/types/multipart.js:291:24)
    at Busboy._write (/home/merc/Development/compAs/server/node_modules/busboy/lib/main.js:80:16)
    at writeOrBuffer (node:internal/streams/writable:556:12)
    at _write (node:internal/streams/writable:490:10)
    at Writable.write (node:internal/streams/writable:494:10)
    at IncomingMessage.ondata (node:internal/streams/readable:985:22)
    at IncomingMessage.emit (node:events:514:28)
    at Readable.read (node:internal/streams/readable:758:10)
    at flow (node:internal/streams/readable:1248:53)
    at resume_ (node:internal/streams/readable:1227:3)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) PROMISE: Promise {
  <rejected> TypeError: The "path" argument must be of type string or an instance of Buffer or URL. Received null
      at TempWriteStream.WriteStream (node:internal/fs/streams:340:5)
      at new TempWriteStream (/home/merc/Development/compAs/server/node_modules/fs-temp/lib/write-stream.js:6:15)
      at Object.createWriteStream (/home/merc/Development/compAs/server/node_modules/fs-temp/lib/temp.js:121:10)
      at Busboy.<anonymous> (/home/merc/Development/compAs/server/node_modules/multer/lib/read-body.js:70:27)
      at Busboy.emit (node:events:514:28)
      at Busboy.emit (/home/merc/Development/compAs/server/node_modules/busboy/lib/main.js:37:33)
      at PartStream.<anonymous> (/home/merc/Development/compAs/server/node_modules/busboy/lib/types/multipart.js:214:13)
      at PartStream.emit (node:events:514:28)
      at HeaderParser.<anonymous> (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:50:16)
      at HeaderParser.emit (node:events:514:28)
      at HeaderParser._finish (/home/merc/Development/compAs/server/node_modules/dicer/lib/HeaderParser.js:68:8)
      at SBMH.<anonymous> (/home/merc/Development/compAs/server/node_modules/dicer/lib/HeaderParser.js:40:12)
      at SBMH.emit (node:events:514:28)
      at SBMH._sbmh_feed (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:159:14)
      at SBMH.push (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:56:14)
      at HeaderParser.push (/home/merc/Development/compAs/server/node_modules/dicer/lib/HeaderParser.js:46:19)
      at Dicer._oninfo (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:196:25)
      at SBMH.<anonymous> (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:126:10)
      at SBMH.emit (node:events:514:28)
      at SBMH._sbmh_feed (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:188:10)
      at SBMH.push (/home/merc/Development/compAs/server/node_modules/streamsearch/lib/sbmh.js:56:14)
      at Dicer._write (/home/merc/Development/compAs/server/node_modules/dicer/lib/Dicer.js:108:17)
      at writeOrBuffer (node:internal/streams/writable:556:12)
      at _write (node:internal/streams/writable:490:10)
      at Writable.write (node:internal/streams/writable:494:10)
      at Multipart.write (/home/merc/Development/compAs/server/node_modules/busboy/lib/types/multipart.js:291:24)
      at Busboy._write (/home/merc/Development/compAs/server/node_modules/busboy/lib/main.js:80:16)
      at writeOrBuffer (node:internal/streams/writable:556:12)
      at _write (node:internal/streams/writable:490:10)
      at Writable.write (node:internal/streams/writable:494:10)
      at IncomingMessage.ondata (node:internal/streams/readable:985:22)
      at IncomingMessage.emit (node:events:514:28)
      at Readable.read (node:internal/streams/readable:758:10)
      at flow (node:internal/streams/readable:1248:53)
      at resume_ (node:internal/streams/readable:1227:3)
      at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
    code: 'ERR_INVALID_ARG_TYPE'
  }
}

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

Wait wait I am using 2.0.0-rc.1 -- could the dependency be fixed in rc2?

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

(Wait... I didn't know there was an rc4?!?)

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

Maybe. [email protected] depends on [email protected]. Are you able to update to [email protected] and test again?

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

btw, @mercmobily how is your app importing multer@2+ since it's an ESM?

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

@mercmobily npm i [email protected] should install it.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

Oh sweet, I don't need to do last-minute patching anymore...!
What is the plan for V2? Are you guys going to release this RC?
Or are you going to convert everything back to CJS before release?

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

Two big questions. I just now started trying to help. So I have some learning to do and don’t know what the plan is yet for multer. But I have confidence there will be one.

I wish we could get everyone who’s actively using multer on an email list and start communicating the changes, re: ESM, in hopes to get feedback to drive a direction. I wonder how many people would respond to a survey? We could host a survey page on the express site 🤓

Perhaps a safer approach would be to update v1 dependencies to their latest versions and go ahead and release v2 as it stands. That would hopefully give users flexibility to evolve their codebases on their time table.

Glad you found a better solution for your situation. It’s funny how it turns out that keeping software soft can be quite difficult.

from multer.

ljharb avatar ljharb commented on April 28, 2024

At the moment, an ESM-only package empirically and drastically limits adoption.

Once that feature is released, unflagged, in every supported node version, and a few years have passed, that may change - but package authors should not be holding their breath for that.

from multer.

mercmobily avatar mercmobily commented on April 28, 2024

I guess we will talk about it in 2034...

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

Yes. I’m willing and able to help. @LinusU I can follow your lead here. My schedule is flexible.

Perhaps we can create a separate issue to use for hashing out the plan?

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024

Yeah whatever works best for @LinusU I think. I just wanted to chime in to make sure this issue didn't stall out if I could help anything.

from multer.

wesleytodd avatar wesleytodd commented on April 28, 2024

Ah ok, sorry for the misunderstanding! We can close this then, sorry for re-opening. Seriously though it would be great for you to help us figure out what next steps need to happen for this repo. Even if that is as a user working closely with us to ensure that the 2.x beta's are working so we can fix forward!

from multer.

joeyguerra avatar joeyguerra commented on April 28, 2024

I’m glad you created this issue. Among many things, it shows that at least 1 person is using multer and express for something valuable and I think that’s amazing. I’m glad we came up with a better solution to the situation together. I don’t know about y’all, but I suffer from major imposter syndrome when trying to use code I didn’t write and these types of online interactions help me make progress.

Side note: the internet and programming is supposed to be fun, first and foremost. Here’s hoping to bring back the fun.🤩

from multer.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.