Plans for v2?

[recaph]

Hi Team,

Our scans have identified a few dependencies as operational risk due to being old/unmaintained. These are coming from busboy and concat-stream dependencies.

multer v2.0.0rc3 seems to be addressing these already (busboy 0.3.1 and removing concat-stream)

It would be great to know when v2 will finally be released.

[LinusU]

Right now I cannot commit to a specific date, but if you could test the latest release candidate and share your experience in #399 that would really help move this forward. As it is now I'm a bit afraid to release it since I haven't heard too much from people using the the v2 release candidates in production environments 😅

[recaph]

Actually we depend on a different project https://github.com/nestjs/nest/tree/master/packages/platform-express which depends on older version of this project.. I can ask them too but they may not agree to use a rc release..

[TeemuKoivisto]

@LinusU hey Linus, I just started debugging things and happened to stumble upon upgrading multer to "^2.0.0-rc.3". Not having both commonjs and ESM imports is really a no-go for many projects. I can't just upgrade everything to use ESM and it's awkward to require an ESM import. I'd suggest using a bundler and adding both, "main" and "module" blocks. With Rollup for example that could be:

import commonjs from '@rollup/plugin-commonjs'

import pkg from './package.json'

export default {
  input: 'src/index.js',
  output: [
    {
      file: pkg.main,
      format: 'cjs',
    },
    {
      file: pkg.module,
      format: 'es',
    },
  ],
  external: [...Object.keys(pkg.dependencies || {})],
  plugins: [commonjs()],
}

Breaking things downstream is just really annoying and I much prefer not having to debug unrelated things to my current task.

[LinusU]

I'd suggest using a bundler and adding both, "main" and "module" blocks.

The problem is that our dependencies are ESM-only, so the package cannot be CJS and import those modules (unless we somehow require them asynchronously, which could possible work 🤔. Although at this point it seems like the entire ecosystem will move to ESM sooner or later so 🤷 )

Breaking things downstream is just really annoying and I much prefer not having to debug unrelated things to my current task.

The entire API has changed anyways, since we have removed the concept of storage engines and are working with streams instead, so it will be a breaking change even without ESM...

[TeemuKoivisto]

The problem is that our dependencies are ESM-only, so the package cannot be CJS and import those modules (unless we somehow require them asynchronously, which could possible work 🤔. Although at this point it seems like the entire ecosystem will move to ESM sooner or later so 🤷 )

I just checked the deps. Half of them are still CommonJS, the other half are packages you maintain. You sure they can't be bundled also into CJS? And as a sidenote I'm not sure what is the purpose of a package like has-own-property. But I digress.

The entire API has changed anyways, since we have removed the concept of storage engines and are working with streams instead, so it will be a breaking change even without ESM...

Yep I understand. It's just there are many packages that are CJS-only so it'd would be nice I wouldn't have to start patching them up in the same setting as I upgrade a npm library. Then having to update the Docker image Node.js version. Deploy that. Add a note in the README to use only Node.js v16. Just lot of extra work for a single change.

[LinusU]

[...] the other half are packages you maintain.

I believe that we depend on file-type by Sindre which won't be made available for CJS.

And as a sidenote I'm not sure what is the purpose of a package like has-own-property.

Once we drop support for Node.js <16 this could be replaced with Object.hasOwn. But the purpose is to check if there is a property existing on an object that can have a null prototype.

It's just there are many packages that are CJS-only [...]

You can import CJS packages from ESM, it's just the other way around that doesn't work.

Then having to update the Docker image Node.js version. Deploy that. Add a note in the README to use only Node.js v16.

ESM works on Node.js 12, 14, and 16+ which is the currently supported Node.js versions upstream. Even if we make Multer CJS compatible it wouldn't change the versions of Node.js that we would support. Or do you have a need to support older versions?

[TeemuKoivisto]

I believe that we depend on file-type by Sindre which won't be made available for CJS.

I see. Well I guess this is just a bad time to update your packages then unless you prefer ESM. Which I do, but not when I have to deal with a legacy project that crashes on me.

Once we drop support for Node.js <16 this could be replaced with Object.hasOwn. But the purpose is to check if there is a property existing on an object that can have a null prototype.

Ok. Just saying it would seem easier to add an utility function instead of importing a whole package. From security and maintenance perspective. There is enough stuff in node_modules as it is.

ESM works on Node.js 12, 14, and 16+ which is the currently supported Node.js versions upstream. Even if we make Multer CJS compatible it wouldn't change the versions of Node.js that we would support. Or do you have a need to support older versions?

Not without a flag and something like top-level await is only for >=14.18 and >=16. Any case, I'll get this sorted out so don't worry. Just my 2 cents.

[LinusU]

I see. Well I guess this is just a bad time to update your packages then unless you prefer ESM. Which I do, but not when I have to deal with a legacy project that crashes on me.

Yeah, I feel you. Although since this is a major version already, I thought that it wouldn't affect any legacy projects?

I felt like file type detection was a very requested feature, and I didn't want to ship an old version that won't be receiving updates since I know that there will be issues opened with "why isn't file type X supported" 😅

Ok. Just saying it would seem easier to add an utility function instead of importing a whole package. From security and maintenance perspective. There is enough stuff in node_modules as it is.

Well technically your node_modules would be larger if every package would copy the function in instead of sharing it with others, but I get were you're coming from! In fact, I was actually looking into deprecating that package just the other day. I wish that Object.hasOwn would have landed earlier 😄

Not without a flag [...]

I don't believe that this is correct? As far as I understand it stable ESM is in all of the versions that this package support (^12.20.0 || ^14.13.1 || >=16.0.0)

[...] and something like top-level await is only for >=14.18 and >=16.

Yeah, although this package aren't using top-level await so I'm not if that is relevant here? 🤔

Any case, I'll get this sorted out so don't worry. Just my 2 cents.

I really appreciate the input and discussion!

[kibertoad]

Busboy 0.3.1 does come with its own issues, such as mscdex/busboy#250 and mscdex/busboy#251. We've recently started fixing them within our own fork (https://github.com/fastify/busboy), and also optimized performance along the way: fastify/busboy#32 and fastify/busboy#36

After we are done with our first public release, would you be interested in PR for Multer switching to the fork? Since we only support Node 10+, I would assume that would only affect the v2.

[LinusU]

@kibertoad awesome!

[kibertoad]

@LinusU #1056 is up
BTW, is there any timeline on releasing GA version of V2 api, and is there anything we can do to help it happen/

[mercmobily]

@LinusU Can you please confirm that Multer will NOT be available as a CommonJS module at all going forward?
(If that is the case, we won't be able to use it -- we will have to stay on V1...
(I frankly think there are a lot of people on the same boat!)