Fix require() to return mutable exports for Node.js compat

[anonrig]

Fixes #5844 (with the help of our friend Claude)

ES module namespaces have read-only properties by specification, but CommonJS require() is expected to return objects with mutable properties.

[jasnell]

This is likely breaking and I'm not yet convinced this is the correct fix. Specifically, we have the EXPORT_DEFAULT option and corresponding compat flag that is supposed to fix the issue. The problem is that the original implementation of require() returned the module namespace and not the default export. The compat flag is supposed to update that so that it returns the default export, which is the correct behavior. When that compat flag is not set, it's supposed to maintain the original behavior of returning the module namespace. Before applying any other fixes like this here, we need to investigate if/why the EXPORT_DEFAULT option is not being correctly set.

[anonrig]

@jasnell I investigated and the EXPORT_DEFAULT mechanism is working correctly - the issue is much more in depth than it initially appeared.

• EXPORT_DEFAULT is only set for synthetic modules (not for ESM modules)
• Node built-in modules are ESM modules, not synthetic. They're compiled from TS to JS, so maybeSynthetic == kj::none and EXPORT_DEFAULT is intentionally not set for them.
• Even if we set EXPORT_DEFAULT for node:timers/promises, the default export of that module will be the namespace object (which is read-only). So we'd still get the same exact error.

I don't see any other way of solving this than the current approach.

[jasnell]

As mentioned, the proposed approach is likely breaking. This will require more consideration on what the correct approach is. I'm working on streams stuff at the moment but will be able to come back to this either later today or tomorrow.

[jasnell]

Ok, yeah I remember now about the limitations on the existing compat flag. What needs to happen then is introducing another compat flag that essentially says: for any module type, when require() is used, return the default export if it exists, and if it does not, fallback to the behavior you have here. However, if this new compat flag is turned off, the current behavior should be preserved. The general idea is that require should always return the default export, regardless of the module type.

[vicb]

/cc @petebacondarwin FYI (wondering if Devin did not write code relying on that)

[vicb]

When #5844 uses require, that's before the ESBuild bundling phase.

So there should be import in the end.

Note that the issue has import based code working in Node and failing in workerd.

So I think that's something we make sure works and should test.

I won't have time to check tonight but can probably find time to check by Monday.

Another option is to merge that but as experimental until we can double check it actually cover all the use cases and only remove exp / set a default date then.