fix: prefix stream/web import with node:

[benmccann]

The purpose of this PR is:

Making it more obvious that stream/web is a Node built-in. Since we upgraded to node-fetch 3.0 final our deploys are failing on Netlify (netlify/zip-it-and-ship-it#743). I'm hoping this would be part of a solution

This is what had to change:

Add node: prefix

This is what like reviewers to know:

This is a supported syntax for importing core modules. I believe the node: prefix is only available on Node 16. However, since stream/web is only available on Node 16 as well, I don't think that should be an issue. It does skip the require cache, which might cause some small performance loss - if the user's application is importing stream/web elsewhere I believe this will cause a new instance to be imported

I might wait to merge this until seeing what the reviewers of the detection package think about the idea as well (dependents/node-precinct#88). However, I thought I'd at least raise it here to see whether you'd be amenable to the idea.

---

• I prefixed the PR-title with docs: , fix(area): , feat(area): or breaking(area):
• I updated ./CHANGELOG.md with a link to this PR or Issue
• I updated the README.md
• I Added unit test(s)

---

[LinusU]

Neat 👍

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (main@9022f8f). Click here to learn what that means.
The diff coverage is n/a.

@@ Coverage Diff @@ ## main #122 +/- ## ======================================== Coverage ? 100.00% ======================================== Files ? 4 Lines ? 425 Branches ? 69 ======================================== Hits ? 425 Misses ? 0 Partials ? 0 

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9022f8f...07227e2. Read the comment docs.

[jimmywarting]

node: prefix was added in v12.20 but only for esm syntax, require() got support for it later in v14.18

we still support v12.20

[jimmywarting]

a better detection (on your end) would be to discard everything after a / so detecting built in modules such as stream/whatever is compared as just stream

[benmccann]

node: prefix was added in v12.20 but only for esm syntax, require() got support for it later in v14.18

per the issue description, stream/web is only available on Node 16, so it's completely safe to use node: here

[jimmywarting]

per the issue description, stream/web is only available on Node 16, so it's completely safe to use node: here

touché

[benmccann]

Thanks! Any chance we can get a new release with this included?

[jimmywarting]

it didn't bring much new to the table that it was so much worth making a release out of. but i guess we could make a new patch release

i have found out that NodeJS now have added a Readable.toWeb and a FileHandle.readableWebStream() method that i would like to use to read local files. But FileHandle.readableWebStream don't take any start/stop options... so i would have to use Readable.toWeb instead.

[benmccann]

A new patch release would be great. No project that's using fetch 3.0 final can deploy to Netlify, so it's a somewhat serious issue even if the fix here is very minor.

[jimmywarting]

okey

[benmccann]

great, thanks!

Netlify has merged the fix on their end (netlify/zip-it-and-ship-it#773), so all we need is a release here and then folks should be able to use this library on Netlify again

[jimmywarting]

it's on my agenda, i where just thinking if maybe i could get #125 merged first. I would like for someone to cast just an eye on it first. After that i will make a new release