Add support for requiring node internal modules with "node:" prefix#134
Add support for requiring node internal modules with "node:" prefix#134Lily0603 wants to merge 1 commit into
Conversation
| const builtin = name.startsWith('node:'); | ||
| if (builtin) { | ||
| const builtinName = name.slice(5); | ||
| if (builtinModules.includes(builtinName)) return internalRequire(name); |
There was a problem hiding this comment.
I have strong concerns about exposing raw result of interal require. The results are cached and potentialy be shared between different modules which defeats metavm isolations purposes
There was a problem hiding this comment.
The internalRequire was added before my changes.
Also node: guarantees that exactly the built-in module is loaded, and not something from the cache or the file system.
Built-in modules can be identified using the node: prefix, in which case it bypasses the require cache. For instance, require('node:http') will always return the built in HTTP module, even if there is require.cache entry by that name
| assert.strictEqual(typeof ms.exports, 'object'); | ||
| assert.strictEqual(typeof ms.exports.fs.promises, 'object'); |
There was a problem hiding this comment.
It might be worth doing more than just checking that the exports is an object.
To ensure the correct requiring built-in modules from node namespace, we could assert the presence of a few "Stable" API functions, rather than relying on shallow type checks
2 participants
npm t)npm run fix)Support
node:prefix for built-in modules #95