nodejs / readable-stream

Node-core streams for userland
https://nodejs.org/api/stream.html
Other
1.03k stars 227 forks source link

webpack 5 issue: buffer fallback import crashes webpack compilation #448

Closed devkral closed 2 years ago

devkral commented 3 years ago

Without fallback entry (require("buffer") or false) I get with webpack 5 the error:

ERROR in ./node_modules/safe-buffer/index.js 3:13-30 Module not found: Error: Can't resolve 'buffer' in '/.../node_modules/safe-buffer'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default. This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:

It would be nice if the import could be rewritten to not crash the webpack compilation.

mcollina commented 3 years ago

I think might be better added to https://www.npmjs.com/package/safe-buffer.

mcollina commented 3 years ago

cc @feross

feross commented 3 years ago

We need Node.js to continue using the built-in buffer which is provided by require('buffer') so I can't simple change that require to require('buffer/') without a massive performance hit.

One possible solution: Leave the require('buffer') as-is so Node.js uses the built-in Buffer, but add https://www.npmjs.com/package/buffer as a dependency of safe-buffer so that webpack will find a package named buffer in node_modules.

Can someone confirm if this approach works? If so, happy to apply the fix.

Vinnl commented 3 years ago

@feross Yes, I would expect that fix to work - currently, a workaround that works for us is to add buffer as a direct dependency, so ideally it would get pulled in automatically by safe-buffer. I think that would also resolve this issue: https://github.com/feross/safe-buffer/issues/18 (Although the original issue wasn't described too clearly, after its closure they discuss running into this issue too and being able to resolve it by adding buffer as a dependency.)

Edit: submitted a PR: https://github.com/feross/safe-buffer/pull/33

jimmywarting commented 3 years ago

should just stop depending on safe-buffer and switch to using buffer directly instead. safe-buffer isn't needed anymore

mcollina commented 2 years ago

Will be fixed in v4.0.0

jaxoncreed commented 2 years ago

I might be missing something, but I'm still having problems with v4.0.0. On a clean install using react-create-app (it uses webpack v5), I get the following errors:

ERROR in ./node_modules/readable-stream/lib/internal/streams/duplexify.js 5:21-38

Module not found: Error: Can't resolve 'buffer' in '/Users/jackson/O/scratch/test-ldo-react/node_modules/readable-stream/lib/internal/streams'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
    - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "buffer": false }

ERROR in ./node_modules/readable-stream/lib/ours/util.js 3:21-38

Module not found: Error: Can't resolve 'buffer' in '/Users/jackson/O/scratch/test-ldo-react/node_modules/readable-stream/lib/ours'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
    - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "buffer": false }

ERROR in ./node_modules/string_decoder/node_modules/safe-buffer/index.js 4:13-30

Module not found: Error: Can't resolve 'buffer' in '/Users/jackson/O/scratch/test-ldo-react/node_modules/string_decoder/node_modules/safe-buffer'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
    - add a fallback 'resolve.fallback: { "buffer": require.resolve("buffer/") }'
    - install 'buffer'
If you don't want to include a polyfill, you can use an empty module like this:
    resolve.fallback: { "buffer": false }

It looks like buffer is still required in those files: https://github.com/nodejs/readable-stream/blob/main/lib/internal/streams/duplexify.js#L5 https://github.com/nodejs/readable-stream/blob/main/lib/ours/util.js#L3 https://github.com/feross/safe-buffer/blob/master/index.js#L4

mcollina commented 2 years ago

Have you followed the docs in https://github.com/nodejs/readable-stream#usage-in-browsers? Could you open a fresh issue in case?

jaxoncreed commented 2 years ago

Ah, the goal was to require no webpack modifications. I guess I misunderstood the purpose of this library.

Turns out simply installing buffer (npm i buffer) allows you to use this library without any webpack changes