Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

node:test test should be element of builtinModules #47465

Closed
loynoir opened this issue Apr 7, 2023 · 4 comments
Closed

node:test test should be element of builtinModules #47465

loynoir opened this issue Apr 7, 2023 · 4 comments

Comments

@loynoir
Copy link

loynoir commented Apr 7, 2023

Version

v18.15.0

Platform

archlinux

Subsystem

No response

What steps will reproduce the bug?

No response

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

$ node
> void await import('node:test')
> (await import('module')).builtinModules.includes('test')
true

What do you see instead?

$ node
> void await import('node:test')
> (await import('module')).builtinModules.includes('test')
false

Additional information

$ node
> (await import('is-builtin-module')).default('node:test')
false

https://github.com/sindresorhus/is-builtin-module/blob/2b22980d890cd656419106e2d2cca5696453bd7f/index.js#L2

https://github.com/sindresorhus/builtin-modules/blob/0d234f3ad3965c57d8cc5b9ca198c505fe032452/index.js#L2

@loynoir loynoir changed the title node:test test should be element of builtinModules node:test test should be element of builtinModules() Apr 7, 2023
@loynoir loynoir changed the title node:test test should be element of builtinModules() node:test test should be element of builtinModules Apr 7, 2023
@MoLow
Copy link
Member

MoLow commented Apr 7, 2023

I am not sure this is a bug, since 893995d enforces node:test can only be imported with a schema.
@cjihrig would you expect builtinModules to contain node:test?

@loynoir
Copy link
Author

loynoir commented Apr 7, 2023

My outdated thoughts

Emm, after node: protocol introduced, and node:X and X is now kind of different, does it make sense to add .builtinModuleIds, and deprecate .builtinModules?

.builtinModuleIds

  • it is something like ["X", "node:X", "X/promises", "node:X/promises"]

  • in ESM, it refers to AST level import statement importId

  • in CJS, it refers to AST level require expression arg[0]

  • Compared with .builtinModules which use concept module, a layer over raw AST concept, leads to different instinct between node dev and userland dev


Or, I thinks providing isBuiltinId(importId_or_requireId: string) would be very nice, which I think this should be something in node instead of userland.

https://nodejs.org/api/module.html#moduleisbuiltinmodulename

@loynoir
Copy link
Author

loynoir commented Apr 7, 2023

Problem solved.

is-builtin-module should be deprecated, in favor of node native nodeModule.isBuiltin

> nodeModule.isBuiltin('node:test')
true

@loynoir loynoir closed this as completed Apr 7, 2023
@cjihrig
Copy link
Contributor

cjihrig commented Apr 7, 2023

@MoLow yes, this is a dupe of #42785

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants