search

nodejs / readable-stream

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

v4 fails to load in @web/test-runner #513

Closed tpluscode closed 1 year ago

tpluscode commented 1 year ago

I started receiving this error when trying to run my tests:

Could not resolve import "stream"

I traced this to a dependency which now uses v4 of readable-stream. In the module lib/ours/index.js there is a const Stream = require('stream')

Why import the built-in?

mcollina commented 1 year ago

so that instanceof Stream would work. But maybe we should soft-detect if that's available and fall back to our implementation.

mcollina commented 1 year ago

Would you like to send a PR?

tpluscode commented 1 year ago

Happy to, although I'm not sure what the fix would entail. How would you "soft-detect" in a way which avoid the import?

kanongil commented 1 year ago

I expect you can surround the require in try/catch:

try {
    var Stream = require('stre' + 'am');
}
catch (err) {}

This will make the Stream value undefined both if require is undefined and if it fails to find the 'stream' module.

Additionally, I changed the import to a "computed" value so that tooling will leave it alone.

tpluscode commented 1 year ago

To be honest, I'm at a loss here. I think I will close the issue for now as it likely exposes some other problem with rollup or @web/test-runner which not necessarily caused by readable-stream