search

nodejs / readable-stream

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

fix global undefined on service worker #493

Closed smeng9 closed 1 year ago

smeng9 commented 1 year ago

Fixes https://github.com/nodejs/readable-stream/issues/487

mcollina commented 1 year ago

Do you think it would be possible to add a unit test under https://github.com/nodejs/readable-stream/tree/main/test/browser?

smeng9 commented 1 year ago

What kind of tests are we expected to run for this case? Do you mean add a runner-worker.mjs for service worker?

mcollina commented 1 year ago

It should run a service worker and get back some data.

smeng9 commented 1 year ago

Hi @mcollina the worker setup seems is way more complicated than I thought.

Also this pull request should only target v3.6.0 https://github.com/nodejs/readable-stream/blob/bed7ffa274f5b9e6d0d5c22369e6fe825ded03d2/lib/_stream_readable.js where v4 seems does not have such issue. I cannot find a branch for v3.

Unfortunately, some of our dependencies still uses v3.

mcollina commented 1 year ago

Can you target the v3.x line with your PR then? The build scripts were redone between them.

smeng9 commented 1 year ago

Hi @mcollina I cannot find the 3.x branch here https://github.com/nodejs/readable-stream/branches/stale?page=1 to target my commit with. Can you help to create a 3.x branch in this repo? Thanks!

Then I can fork it and create a merge.