[Feature request] `is-core-module`

[SuperchupuDev]

Can be easily replaced by importing isBuiltin from node:module

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

[SukkaW]

@SuperchupuDev I've considered this before. But is-core-module also supports getting built-in modules of a specific Node.js version by passing the second argument. This behavior doesn't exist in isBuiltin and could potentially be breaking. I haven't found a sound solution here yet.

[SukkaW]

Also, the toxic dependency of the is-core-module is hasown, which brings its friend function-bind:

https://npmgraph.js.org/?q=is-core-module

And nolyfill already provides @nolyfill/hasown for hasown's replacement.

[dominikg]

i'd consider is-core-module itself toxic too, unless you need the second arg there is no reason for all of this over the platform provided isBuiltin. Resolve doesn't use it https://github.com/browserify/resolve/blob/fd788d94d037e32d4f4be948e2f7e15f6981f004/lib/sync.js#L106

If you replaced it with a function that throws if the second arg is used, people can decide for themselves.

[SukkaW]

i'd consider is-core-module itself toxic too, unless you need the second arg there is no reason for all of this over the platform provided isBuiltin. Resolve doesn't use it https://github.com/browserify/resolve/blob/fd788d94d037e32d4f4be948e2f7e15f6981f004/lib/sync.js#L106

@dominikg

I agree with you that is-core-module is indeed toxic. But the @nolyfill/is-core-module has to completely override every is-core-module (if we do conditional override, people will end up having both @nolyfill/is-core-module and is-core-module inside their node_modules). The resolve doesn't use the second argument, but other dependents might.

[dominikg]

there are very few uses of it with the second argument set on github, and all but one of them set it to a very high version to get 'latest' info

https://github.com/search?q=%2F%28%3F-i%29%28%3F%3AisCore%7CisCoreModule%7CisBuiltinModule%29%5C%28%5B%5E%5C%29%5D%2B%5C%2C%5B%5E%5C%29%5D%2B%5C%29%5C%29%2F+AND+%28language%3AJavaScript+OR+language%3ATypeScript+%29&type=code&p=2

(names found via https://github.com/search?q=%2F%5B%27%22%5Dis-core-module%5B%27%22%5D%2F+AND+%28language%3AJavaScript+OR+language%3ATypeScript+%29&type=code&p=2 )

i think it would be pretty safe to even just ignore the second arg instead of throwing an exception.

[dominikg]

oh, you could also make this a 2 for 1 by going after https://www.npmjs.com/package/is-builtin-module too

[SukkaW]

oh, you could also make this a 2 for 1 by going after https://www.npmjs.com/package/is-builtin-module too

@dominikg IMHO we should not override is-builtin-module:

• It leaves very little footprint in the node_modules
  • Installation size 7.7 KiB with 2 packages: https://pkg-size.dev/is-builtin-module
• It is modern
  • It is even an ESM-only package
  • It doesn't depend on any polyfill
• It even promotes module.isBuiltin API in its README.

[dominikg]

7.7kb is still a lot more than the native function.

would it be possible to cut a new major to bump min node or have nolyfill18 so that you can reduce it to a single call?

[SukkaW]

Would it be possible to cut a new major to bump min node or have nolyfill18 so that you can reduce it to a single call?

The discussion about bumping the minimum supported Node.js version is here: https://github.com/SukkaW/nolyfill/issues/63

Also, I noticed that Node.js implements module.isBuiltin in JS under the hood when implementing @nolyfill/is-core-module, so technically module.isBuiltin does not have less function invocation.