-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
This module only works with node 14 or LOWER #1245
Comments
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 |
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? |
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. Sorry if I can't be of much more help. |
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!
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. |
I didn't think of this. I will ask. |
Being ESM (both v2 here and in |
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. |
@mercmobily not synchronously it can't - only with importing CJS in node Just Works, period - so it's the easiest possible thing. |
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. |
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!] |
"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) |
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) |
Check There is 6M weekly downloads for v11 that is "no longer maintained and we will not accept any backport requests"! |
I am not sure this is a good example. After a quick awk/openoffice, you can see that version 11 has 8,025,208 I didn't count older versions -- if somebody is using version 10 or lower, they wouldn't be upgrading either way. |
@mercmobily https://majors.nullvoxpopuli.com/q?packages=got shows the problem more clearly. |
@mercmobily can you please describe the scenario? I want to write a failing test and submit a fix for it. |
@mercmobily I've been reviewing the code and the rc 2 version. RE: the 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)
}) |
It took me a while to make it happen... If you look at the patch I am manually running, it boils down to I don't think only tests use it -- this line in the stack below shows:
Full stack:
|
Wait wait I am using 2.0.0-rc.1 -- could the dependency be fixed in rc2? |
(Wait... I didn't know there was an rc4?!?) |
Maybe. |
btw, @mercmobily how is your app importing |
My app is also ESM -- I updated everything basically just for multer about
a year ago.
How do I get rc4?
…On Sun, 31 Mar 2024, 5:20 am Joey Guerra, ***@***.***> wrote:
btw, @mercmobily <https://github.com/mercmobily> how is your app
importing ***@***.***+ since it's an ESM?
—
Reply to this email directly, view it on GitHub
<#1245 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQHWXXMNMYG24SLIWSMN4LY256HXAVCNFSM6AAAAABD5XASXGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRYGU2DAMJQGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@mercmobily |
Oh sweet, I don't need to do last-minute patching anymore...! |
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. |
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. |
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. |
I guess we will talk about it in 2034... |
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 |
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? |
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. |
I am sorry, there was a communication issue here. Plus:
This was NOT a disgruntled complaint! It was really my forecast. What I should have written is:
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. Sorry for the noise and for the misunderstanding. But please rest assured that I am not "that" kind of user/developer...! |
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! |
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.🤩 |
Seriously though, we are using on a huge production project, and it's worked really well for years. Coming up to 7 now. |
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 |
Hi guys! To all those who are looking for multer for new versions of Node.js, I can recommend the package @ts-stack/multer (I am its author). This fork is based on multer v2.0.0-rc.4, it updates almost all dependencies to new versions. By the way, Required Node.js >= v20.0.6, writen in TypeScript. |
I love the work done on this module, but:
Now... I just had to add this to a production machine, to "fix" the issue with fs-temp:
If the module is no longer maintained, please say so and ask for somebody to take it over.
The text was updated successfully, but these errors were encountered: