Open WasabiThumb opened 3 months ago
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.
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 😢
I've added a GitHub pages deployment, pressing the button should cause a console error. https://wasabithumb.github.io/reprex-539/
Any apdate ?
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?
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 :heart:
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).
I'll be happy to land a PR covered by tests.
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.
Use vite
to bundle an app and run the tests in the browser with playwright.
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 byreadable-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 thedefault
andprocess
fields. Here myvite.config.ts
:readable-stream
|^4.5.2
vite
|^5.4.2
vite-plugin-node-polyfills
|^0.22.0
rollup
|^4.21.0
esbuild
|^0.23.1