Node.js Compat: Implement the nodejs_compat_v2 compat flag

[jasnell]

The nodejs_compat_v2 flag supersedes the 'nodejs_compat' flag. When enabled...

1. Node.js built-ins are available for import/require
2. Unlike the original nodejs_compat flag, Node.js imports do not require the 'node:' specifier prefix. Internally, the implementation will detect a Node.js raw specifier and convert it into the appropriate prefixed specifier, e.g. 'fs' becomes 'node:fs'
3. The Buffer, process, and global are exposed on the global
4. A user worker bundle can still provide its own implementations of all node: modules which will take precedence over the built-ins
5. The new process.getBuiltinModule(...) API is implemented. See process: add process.getBuiltinModule(id) nodejs/node#52762

A worker can replace the implementation of node:process if they choose, which may mean that getBuiltinModule(...) is not available.

Refs: #2133

[IgorMinar]

I can't judge the implementation soundness, but I was able to play with this and can confirm that with the nodejs_compat_v2 flag Buffer and process are now exposed, however global is not. Could you please add that?

I was also able to do some basic experiments with process.getBuiltinModule which also seems to work as expected. I did not test if overwriting a built-in using a user-provided module results in no change for the getBuiltinModule - this would be a good thing to add to an automated test.

Can you please add a test or sample for this functionality?

Otherwise this change looks good to me and will unblock our work on hybrid polyfills (unjs/unenv#181). Thank you for a speedy turnaround.

To recap the todos:

• expose global
• add tests

FYI here is my test code:

import path from 'path';
import npath from 'node:path';

export default {
  async fetch(req, env) {
    return new Response("Hello World\n" +
        (('Buffer' in globalThis) ? Buffer : "xBufferx") + '\n' +
        (('process' in globalThis) ? JSON.stringify(Object.keys(process)) : 'xprocessx') + '\n' +
        (JSON.stringify(Object.keys(process.getBuiltinModule('path')))) + '\n' +
        (JSON.stringify(Object.keys(path))) + '\n' +
        (JSON.stringify(Object.keys(npath))) + '\n'+
        (('global' in globalThis) ? global : "xglobalx") + '\n'
    );
  }
};

[jasnell]

@fhanau @harrishancock @dom96 ... this change is needed by the frameworks/wrangler folks to move forward on evolving the node.js compat story.

@irvinebroque @mikenomitch ... heads up from the product perspective this changes the way node.js compat is enabled. There's the existing nodejs_compat flag for the original version, and then a new nodejs_compat_v2 flag to enable this variation. Whee!

[IgorMinar]

@irvinebroque @mikenomitch ... heads up from the product perspective this changes the way node.js compat is enabled. There's the existing nodejs_compat flag for the original version, and then a new nodejs_compat_v2 flag to enable this variation. Whee!

Is the plan to make nodejs_compat_v2 become equivalent with nodejs_compat after a certain compat date? That would be my preference as I want to avoid a proliferation of compat flags. I see nodejs_compat_v2 just as a temporary vehicle to test the changes we plan to make to nodejs_compat which will be enabled at a set compat date.

[jasnell]

Is the plan to make nodejs_compat_v2 become equivalent with nodejs_compat after a certain compat date? That would be my preference as I want to avoid a proliferation of compat flags. I see nodejs_compat_v2 just as a temporary vehicle to test the changes we plan to make to nodejs_compat which will be enabled at a set compat date.

Unfortunately I don't think so. Given the way compat dates work and the guarantees they carry, we can't really combine these at any point and we had made the decision long ago that Node.js compat would not be on-by-default ever. If we move forward with adding this, it'll mean a second node.js compat mode going forward indefinitely. This is something we should discuss tho to see if there's a reasonable path forward.

[IgorMinar]

@jasnell sorry if I was not clear, I'm not suggesting that nodejs_compat_v2 becomes activated on by default after some compat date. What I'm suggesting instead is that after some compat date nodejs_compat and nodejs_compat_v2 mean the same thing (and nodejs_compat_v2 becomes unnecessary). This would reduce the number of flags we need to document and explain to developers as well as number of branches in our codebase.

[jasnell]

Even then, I'm not sure we can combine nodejs_compat and nodejs_compat_v2 unfortunately. This PR introduces a few changes that could be technically breaking even in the nodejs_compat case currently. We'll need to evaluate carefully.

[IgorMinar]

yes, let's evaluate and be careful, but I'd personally rather take the risk of breaking things and having to pivot, than taking the easy route and having to live with two confusing flags forever.

[mikenomitch]

Just catching up now, but I generally agree with @IgorMinar's intuition on this. node_compat vs nodejs_compat is confusing enough already, so adding a new permanent flag worries me a lot - Do global compatability dates not give us a way to break the functionality of nodejs_compat in a safe way?

And given that "node:" will still work, is the main breaking change the new globals that are added?

[jasnell]

... is the main breaking change the new globals that are added?

I'll say "potentially" breaking change are the new globals and the fact that statements like import('buffer') will suddenly start working where they fail now. As we've seen with other recent changes, folks like to do feature-sniffing by trying an action and revering to some fall back logic when it doesn't work. Having it so that the non-prefixed specifiers suddenly start working could cause unexpected side effects that we need to be careful of.

[fhanau]

LGTM otherwise