fix(node-resolve): support node: protocol#1124
Conversation
Imports with the [`node:` protocol](https://nodejs.org/api/esm.html#node-imports) are currently undetected by this plugin. Switch to using [is-builtin-module](https://github.com/sindresorhus/is-builtin-module), which handles this protocol and also submodule imports like `fs/promises`. Functionality is otherwise identical. Resolves rollup#1120.
|
Apologies, but I changed the authorship on the commit and now CI needs a button press again. |
|
@guybedford I think this could use your input given that it's introducing resolution for a new (to rollup) protocol |
guybedford
left a comment
There was a problem hiding this comment.
Seems good. Note we should really just cut out the dependency and use something like:
function isBuiltinModule (name) {
if (name.startsWith('node:')) name = name.slice(5);
return require('module').builtinModules.includes(name);
}|
@isker what are your thoughts on that suggestion? |
|
What are @lukastaegert’s thoughts? This solution was his suggestion: #1120 (comment). |
|
I took a look at the Since this does add new capability, I'm going to merge this as a new |
Rollup Plugin Name:
node-resolveThis PR contains:
Are tests included?
Breaking Changes?
If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.
List any relevant issue numbers:
#1120
Description
Imports with the
node:protocol are currently undetected by this plugin.Switch to using is-builtin-module, which handles this protocol and also submodule imports like
fs/promises. Functionality is otherwise identical.I confirmed the added test fails without the other changes in this PR.