Implement node:module createRequire API

[jasnell]

The createRequire() function is used in node.js to create a require() function that can be used from inside an ESM module. This is useful largely for porting existing CommonJS code to ESM as it allows the ESM module to keep using require() to load other modules.

This is being implemented in the runtime rather than as a polyfill because it needs to integrate tightly with the module registry implementation.

The implementation is as close to Node.js' as we can currently make it.

Note that the return value of createRequire() is a function. In Node.js, this function has additional properties like require.resolve(...), require.main, require.extensions, and require.cache. We are not currently implementing these additional properties but we could in the future if there is a need for them. (require.cache is unlikely to ever be implemented since workers does not have a module cache the way Node.js does).

Because it was extremely trivial to do so, is closely related, and we already have to maintain a list of known node builtins,
this also implements the require('module').isBuiltin(...) API.

This was implemented because the frameworks team asked for it.

Refs: https://nodejs.org/docs/latest/api/module.html#modulecreaterequirefilename

[vicb]

If we have .isBuiltin exposed, it would be nice to also have .builtinModules

Otherwise we might end up with some iconsistencies (i.e. unenv providing .builtinModules and the the workerd implementation of .isBuiltin)

[pi0]

Re unenv: I think unenv should still return all known built-ins for builtinModules and also return true for isBuiltin if it is a known built in (same behavior as Node.js). Libraries depend on both to determine if an import is Node.js built-in or not and if we only return true for the native-subset of workerd, it can break their behavior or at least make inconsistencies. I understand this is something useful for workerd itself to support but probably not for mixed compatibility.

[vicb]

Re unenv: I think unenv should still return all known built-ins for builtinModules and also return true for isBuiltin

Good point.

[jasnell]

Re unenv: I think unenv should still return all known built-ins for builtinModules and also return true for isBuiltin if it is a known built in (same behavior as Node.js). Libraries depend on both to determine if an import is Node.js built-in or not and if we only return true for the native-subset of workerd, it can break their behavior or at least make inconsistencies. I understand this is something useful for workerd itself to support but probably not for mixed compatibility.

Not quite sure I'm following. Are you saying the runtime should not expose isBuiltin()?

[vicb]

Not quite sure I'm following. Are you saying the runtime should not expose isBuiltin()?

I think workerd should expose it but unenv will patch it to return true for the module it polyfills.
With unenv the workerd impl is available at process.getBuiltinModule('module').isBuiltin

Is that correct @pi0 ?

[jasnell]

@vicb ... I'm just a bit confused because module.isBuiltin(...) should return true for all of the known Node.js built-in specifiers, not just the ones that are built into the runtime. For instance, in this implementation isBuiltin('fs') would return true even tho it is not implemented in the runtime.

[vicb]

in this implementation isBuiltin('fs') would return true even tho it is not implemented in the runtime.

Now I'm confused too 🤔

[vicb]

import module from "node:module";

for (const m of modules()) {
  if (!module.isBuiltin(m)) {
    console.error(m);
  }
}

function modules() {
  // return the output of module.builtInModules as of Node.js v22.6.0 minus "internal/..." packages
}

-> % node test.mjs
test
-> % deno run test.mjs 
test
-> % bun test.mjs
test

Noe that test is (kind of) expected per the Node.js doc

[vicb]

Would this comment

If we have .isBuiltin exposed, it would be nice to also have .builtinModules

be easy to add to the PR?

[jasnell]

... Noe that test is (kind of) expected per the Node.js doc

It's not clear what you're saying in this comment.

If we have .isBuiltin exposed, it would be nice to also have .builtinModules

It would be fairly trivial yes.

[vicb]

... Noe that test is (kind of) expected per the Node.js doc

It's not clear what you're saying in this comment.

The docs says "Note: the list doesn't contain prefix-only modules like node:test."

I interpreted that as test being "special", implying that isBuiltin(test) === false is expected - "kind of" because I'm unsure if my interpretation is correct.

[jasnell]

Yes, isBuiltin('test') should be false... which is what this PR implements:

[jasnell]

require('module').builtinModules added.

[jasnell]

Marking this ready for review

[vicb]

A few comments:

Node 22.6 had 2 extras builtin modules: test/mock_loader and wasi

I got my previous cross runtime wrong as the reference was deno instead of node. By correctly using the node ref, there are missing modules in runtimes:

-> % bun test.mjs 
test/mock_loader

-> % deno run test.mjs 
_http_client
_http_common
_http_incoming
_http_server
_stream_wrap
_tls_common
_tls_wrap
inspector/promises
test/mock_loader
trace_events

Which means that .isBuiltin() behavior is not consistent across runtimes. At least deno returns what it actually implements.

test/mock_loader might not have made it to the list (see this bug).

I think adding wasi to current PR and clearly document the behavior of workerd which is isBuiltin() === implemented is Node is good to me

[vicb]

LGTM with a few minor comments

[vicb]

Thanks for the updates James

[jasnell]

Running an internal CI run before landing.