Incompatibility with Rollup/Vite process polyfill

[WasabiThumb]

Similar to #450, though in this case a polyfill is present. It looks like the process polyfill is an ES module with a default export that isn't being properly unwrapped by readable-stream.

I've set a breakpoint on the line that is causing TypeError: e.nextTick is not a function:

I can confirm that a nextTick implementation is available at both the default and process fields. Here my vite.config.ts:

import {defineConfig} from "vite";
import {nodePolyfills} from "vite-plugin-node-polyfills";

export default defineConfig({
    mode: 'development',
    build: {
        sourcemap: true
    },
    plugins: [
        nodePolyfills() // also tried: { globals: { process: true } }
    ]
});

Package	Version
readable-stream	^4.5.2
vite	^5.4.2
vite-plugin-node-polyfills	^0.22.0
rollup	^4.21.0
esbuild	^0.23.1

[mcollina]

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

[WasabiThumb]

Thanks for reporting!

Can you provide steps to reproduce? We often need a reproducible example, e.g. some code that allows someone else to recreate your problem by just copying and pasting it. If it involves more than a couple of different file, create a new repository on GitHub and add a link to that.

Here you are! ^_^
https://github.com/WasabiThumb/reprex-539 (non-boilerplate located here)
I've noticed that simply changing line 2 on src/main.ts to import stream rather than readable-stream makes the demo work flawlessly, but it's a shame since I thought usage of stream was discouraged 😢

[WasabiThumb]

I've added a GitHub pages deployment, pressing the button should cause a console error.
https://wasabithumb.github.io/reprex-539/

[Gitssalah]

Any apdate ?

[MattiasBuelens]

This package seems to require process in a rather odd way since #497. Notice the trailing slash:

const process = require('process/')

Perhaps vite-plugin-node-polyfills doesn't handle this case very well?

[WasabiThumb]

This package seems to require process in a rather odd way since #497. Notice the trailing slash:

const process = require('process/')

Perhaps vite-plugin-node-polyfills doesn't handle this case very well?

I noticed #497 since it is somewhat related. I recall editing the package to replace process/ with process to no effect. It should be easy to reproduce with the example. That does make some sense, if it was a resolution issue I would expect Vite to raise a compile-time error. It seems to resolve properly, but provides the whole ES module rather than just the default export. That also makes sense, since that is the default behavior for require, but something is typically supposed to handle this. Either a wrapper function like __webpack_require__, or TSC adding .default when converting import to require; someone can correct me here.

On the TSC note I realized that my example was missing allowSyntheticDefaultImports. However, I added it and got the exact same issue. It can't hurt to have, so I'm pushing it to the example repo. I've also updated the vite.config.ts to only use base: '/reprex-539' when targeting GitHub Pages, since I'm here making a commit anyways ❤️

[WasabiThumb]

In the absence of any new insight, I've made a brute-force fix on this branch. For what it's worth, all tests pass and it works perfectly in the reprex. To try it out:

npm install --save-dev WasabiThumb/readable-stream#bug/vite-esm -or-commit-or-tag

(cc. @Gitssalah)
If this passes the bar, I could make a PR (though I imagine that this is far too opaque).

[mcollina]

I'll be happy to land a PR covered by tests.

[WasabiThumb]

I'll be happy to land a PR covered by tests.

Created #543. I should also note that the PR would not introduce any new tests, simply that it does not actively cause a regression. I'm not sure what a test for this fix would look like while also being in-scope for the project.

[mcollina]

Use vite to bundle an app and run the tests in the browser with playwright.