Open joyeecheung opened 2 weeks ago
Review requested:
This is obviously exactly what I asked for (😄) and gets it "done" the quickest, but I'm curious if there are any other opinions about this in general.
Is it possible that import.now
or "weak/optional" imports come to fruition, such that those are more usable and processed by downstream tools? I'd hate to accidentally open the door to "breaking" node polyfills (node-polyfill-webpack-plugin, vite-plugin-node-polyfills, etc) and so on, just because it's not require or import, but some other expression.
I'd hate to accidentally open the door to "breaking" node polyfills (node-polyfill-webpack-plugin, vite-plugin-node-polyfills
Not quite sure if I am following this but I think process.getBuiltin(id)
should be polyfill-able by these kind of bundler plugins - just patch the its polyfilled process object to load its own polyfilled libraries by id? (In general plugins like this already polyfill the process object). For code that uses the polyfill/gets transformed to use the polyfills, if the polyfill doesn't support this yet, using this would not be too different than "using a new API that the polyfill doesn't support yet" which happens all the time.
Yeah, I guess it's no different than detecting calls to require
and error/warn on ones which cannot be statically determined. It's just not quite as injectable as require
.
Just poking holes in my own idea.
We can also suggest in the documentation that users should only consider using this for code paths that can only be run in Node.js (the code example is basically already suggesting that but we can make it clearer). If they want the code depending on the built-ins to be polyfill-able in the browser, they should still use require or import.
There is a TC39 proposal to get original built-in functions, through a new getIntrinsic
global (https://github.com/tc39/proposal-get-intrinsic). getBuiltin
is similar enough that could cause confusion — what do you think about something like process.getBuiltinModule
to be more clear about what it does?
Just to prove this out to its full extent (and to procrastinate other work I didn't want to do), I polyfilled this API using --require
and was able to get TypeScript's CLI, API, and test suite all working: https://github.com/microsoft/TypeScript/pull/58419
Updated the PR a bit:
process.getBuiltinModule()
as requested in https://github.com/nodejs/node/pull/52762#issuecomment-2088093800module.isBuiltin
and reject otherwise (so this now throws on getBuiltinModule('test')
etc) as requested in https://github.com/nodejs/node/pull/52762#discussion_r1585606673Changed it to process.getBuiltinModule() as requested in process: add process.getBuiltinModule(id)
Now it mismatches isBuiltin
. Personally I don't see the confusion between getIntrinsic
and getBuiltin
; I think getBuiltin
is fine, and there's more potential for confusion with this not corresponding with isBuiltin
.
Alternatively we can add isBuiltinModule
as an alias for isBuiltin
.
Now it mismatches
isBuiltin
. Personally I don't see the confusion betweengetIntrinsic
andgetBuiltin
; I thinkgetBuiltin
is fine, and there's more potential for confusion with this not corresponding withisBuiltin
.Alternatively we can add
isBuiltinModule
as an alias forisBuiltin
.
isBuiltin
is scoped in node:module
. It is not a function available from the global scope, like global.process.getBuiltinModule
.
isBuiltin
is scoped innode:module
. It is not a function available from the global scope, likeglobal.process.getBuiltinModule
.
Fair enough. I still think we should create an alias on node:module
isBuiltinModule
, but that can come later.
process: add process.getBuiltinModule(id)
process.getBuiltinModule(id)
provides a way to load built-in modules in a globally available function. ES Modules that need to support other environments can use it to conditionally load a Node.js built-in when it is run in Node.js, without having to deal with the resolution error that can be thrown byimport
in a non-Node.js environment or having to use dynamicimport()
which either turns the module into an asynchronous module, or turns a synchronous API into an asynchronous one.If
id
specifies a built-in module available in the current Node.js process,process.getBuiltinModule(id)
method returns the corresponding built-in module. Ifid
does not correspond to any built-in module,undefined
is returned.process.getBuiltinModule(id)
accept built-in module IDs that are recognized bymodule.isBuiltin(id)
. Some built-in modules must be loaded with thenode:
prefix.The built-in modules returned by
process.getBuiltinModule(id)
are always the original modules - that is, it's not affected byrequire.cache
.Fixes: https://github.com/nodejs/node/issues/52599