fix(dev): exclude node:-prefixed built-ins from the browser build#5222
fix(dev): exclude node:-prefixed built-ins from the browser build#5222
node:-prefixed built-ins from the browser build#5222Conversation
|
node:-prefixed built-ins from the browser build
sergiodxa
left a comment
There was a problem hiding this comment.
Will this also allows import Buffer from "buffer" to import buffer from the node_modules? Or is this just to consider node:* as server-only imports?
fd7453e to
50b3070
Compare
| let nodeBuiltins = Array.from( | ||
| new Set([ | ||
| ...builtinModules, | ||
| // account for `node:`-prefixed built-ins like `"node:fs"` | ||
| ...builtinModules.map((x) => `node:${x}`), | ||
| ]) | ||
| ); |
There was a problem hiding this comment.
i'm sort of surprised that builtinModules doesn't already account for the node:* prefix tbh 🤷
There was a problem hiding this comment.
It does, but not sure since what version that's included tbh
There was a problem hiding this comment.
This was also introduced in v16.17 (see nodejs/node#43396), so we should account for values that can or can not have node:-prefix
A possible solution would be to map over all values, remove the node:-prefix, put that array in a Set so we have unique values, loop over the Set to add the node:-prefix again
|
This is blocked on polyfills not handling for |
8b49d66 to
5c83a37
Compare
|
@MichaelDeBoey thanks for looking into it! I thought about this some more and we shouldn't be using polyfills for Node in the browser in the first place. I have a branch locally that treeshakes out all Node built-ins (but keeps them if they are actually 3rd party libs like |
5c83a37 to
5b1437e
Compare
|
Superceded by #5773 |
Closes: #4544
TODO
fakeBuiltinscheck introduced in better message when people import fake built-ins? #190 ?