Skip to content

Commit a74d0bf

Browse files
anonriganonrig
committed
Fix require() to return mutable exports for Node.js compat
1 parent cfbf415 commit a74d0bf

16 files changed

+301
-8
lines changed

‎src/workerd/api/global-scope.c++‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -928,8 +928,15 @@ jsg::JsValue ServiceWorkerGlobalScope::getBuffer(jsg::Lock& js) {
928928
// that set the bufferValue, we let's check again.
929929
return p.getHandle(js);
930930
}
931-
auto def = module.get(js, "default"_kj);
932-
auto obj = KJ_ASSERT_NONNULL(def.tryCast<jsg::JsObject>());
931+
// When requireReturnsDefaultExport flag is enabled, resolveModule returns the
932+
// default export directly. Otherwise it returns the module namespace.
933+
jsg::JsObject obj = [&]() -> jsg::JsObject {
934+
if (module.has(js, "default"_kj)) {
935+
auto def = module.get(js, "default"_kj);
936+
return KJ_ASSERT_NONNULL(def.tryCast<jsg::JsObject>());
937+
}
938+
return module;
939+
}();
933940
auto buffer = obj.get(js, "Buffer"_kj);
934941
JSG_REQUIRE(buffer.isFunction(), TypeError, "Invalid node:buffer implementation");
935942
bufferValue = jsg::JsRef(js, buffer);
@@ -970,7 +977,14 @@ jsg::JsValue ServiceWorkerGlobalScope::getProcess(jsg::Lock& js) {
970977
// that set the processValue, we let's check again.
971978
return p.getHandle(js);
972979
}
973-
auto def = module.get(js, "default"_kj);
980+
// When requireReturnsDefaultExport flag is enabled, resolveInternalModule returns the
981+
// default export directly. Otherwise it returns the module namespace.
982+
jsg::JsValue def = [&]() -> jsg::JsValue {
983+
if (module.has(js, "default"_kj)) {
984+
return module.get(js, "default"_kj);
985+
}
986+
return module;
987+
}();
974988
JSG_REQUIRE(def.isObject(), TypeError, "Invalid node:process implementation");
975989
processValue = jsg::JsRef(js, def);
976990
return def;

‎src/workerd/api/node/tests/BUILD.bazel‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ wd_test(
228228
data = ["module-create-require-test.js"],
229229
)
230230

231+
wd_test(
232+
src = "module-require-mutable-exports-test.wd-test",
233+
args = ["--experimental"],
234+
data = ["module-require-mutable-exports-test.js"],
235+
)
236+
231237
wd_test(
232238
src = "process-exit-test.wd-test",
233239
args = ["--experimental"],

‎src/workerd/api/node/tests/module-create-require-test.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,28 @@ export const doTheTest = {
1414
const baz = require('baz');
1515
const qux = require('worker/qux');
1616

17-
strictEqual(foo.default, 1);
17+
// When require_returns_default_export flag is enabled, require() returns the
18+
// default export directly. Otherwise it returns the namespace object.
19+
if (Cloudflare.compatibilityFlags.require_returns_default_export) {
20+
strictEqual(foo, 1);
21+
} else {
22+
strictEqual(foo.default, 1);
23+
}
1824
strictEqual(bar, 2);
1925
strictEqual(baz, 3);
2026
strictEqual(qux, '4');
2127

2228
const assert = await import('node:assert');
2329
const required = require('node:assert');
24-
strictEqual(assert, required);
30+
31+
// When require_returns_default_export flag is enabled, require() returns the
32+
// default export directly (assert.default === required).
33+
// When the flag is disabled, require() returns the namespace (assert === required).
34+
if (Cloudflare.compatibilityFlags.require_returns_default_export) {
35+
strictEqual(assert.default, required);
36+
} else {
37+
strictEqual(assert, required);
38+
}
2539

2640
throws(() => require('invalid'), {
2741
message: 'Top-level await in module is not permitted at this time.',

‎src/workerd/api/node/tests/module-require-mutable-exports-test.js‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Copyright (c) 2017-2026 Cloudflare, Inc.
2+
// Licensed under the Apache 2.0 license found in the LICENSE file or at:
3+
// https://opensource.org/licenses/Apache-2.0
4+
import { createRequire } from 'node:module';
5+
import { strictEqual, ok, doesNotThrow } from 'node:assert';
6+
7+
const require = createRequire('/');
8+
9+
export const testTimersPromisesMutable = {
10+
test() {
11+
const timersPromises = require('node:timers/promises');
12+
const originalSetImmediate = timersPromises.setImmediate;
13+
ok(typeof originalSetImmediate === 'function');
14+
15+
const patchedSetImmediate = async function patchedSetImmediate() {
16+
return 'patched';
17+
};
18+
19+
doesNotThrow(() => {
20+
timersPromises.setImmediate = patchedSetImmediate;
21+
});
22+
23+
strictEqual(timersPromises.setImmediate, patchedSetImmediate);
24+
timersPromises.setImmediate = originalSetImmediate;
25+
strictEqual(timersPromises.setImmediate, originalSetImmediate);
26+
},
27+
};
28+
29+
export const testTimersMutable = {
30+
test() {
31+
const timers = require('node:timers');
32+
const originalSetTimeout = timers.setTimeout;
33+
ok(typeof originalSetTimeout === 'function');
34+
35+
const patchedSetTimeout = function patchedSetTimeout() {
36+
return 'patched';
37+
};
38+
39+
doesNotThrow(() => {
40+
timers.setTimeout = patchedSetTimeout;
41+
});
42+
43+
strictEqual(timers.setTimeout, patchedSetTimeout);
44+
timers.setTimeout = originalSetTimeout;
45+
},
46+
};
47+
48+
export const testBufferMutable = {
49+
test() {
50+
const buffer = require('node:buffer');
51+
const originalBuffer = buffer.Buffer;
52+
ok(typeof originalBuffer === 'function');
53+
54+
const patchedBuffer = function PatchedBuffer() {
55+
return 'patched';
56+
};
57+
58+
doesNotThrow(() => {
59+
buffer.Buffer = patchedBuffer;
60+
});
61+
62+
strictEqual(buffer.Buffer, patchedBuffer);
63+
buffer.Buffer = originalBuffer;
64+
},
65+
};
66+
67+
export const testUtilMutable = {
68+
test() {
69+
const util = require('node:util');
70+
const originalPromisify = util.promisify;
71+
ok(typeof originalPromisify === 'function');
72+
73+
const patchedPromisify = function patchedPromisify() {
74+
return 'patched';
75+
};
76+
77+
doesNotThrow(() => {
78+
util.promisify = patchedPromisify;
79+
});
80+
81+
strictEqual(util.promisify, patchedPromisify);
82+
util.promisify = originalPromisify;
83+
},
84+
};
85+
86+
export const testRequireCachesMutableObject = {
87+
test() {
88+
const timersPromises1 = require('node:timers/promises');
89+
const timersPromises2 = require('node:timers/promises');
90+
91+
strictEqual(timersPromises1, timersPromises2);
92+
93+
const patchedSetImmediate = async function patched() {
94+
return 'patched';
95+
};
96+
const original = timersPromises1.setImmediate;
97+
98+
timersPromises1.setImmediate = patchedSetImmediate;
99+
strictEqual(timersPromises2.setImmediate, patchedSetImmediate);
100+
timersPromises1.setImmediate = original;
101+
},
102+
};
103+
104+
// When require_returns_default_export is enabled, require() should return the
105+
// default export directly (which is the object with all the functions),
106+
// not the namespace wrapper with both `default` and named exports.
107+
export const testRequireReturnsDefaultExport = {
108+
test() {
109+
const timers = require('node:timers');
110+
// With require_returns_default_export enabled, timers should be the
111+
// default export object directly, not the namespace wrapper.
112+
// The default export IS the object with setTimeout, setInterval, etc.
113+
ok(typeof timers.setTimeout === 'function');
114+
ok(typeof timers.setInterval === 'function');
115+
ok(typeof timers.clearTimeout === 'function');
116+
ok(typeof timers.clearInterval === 'function');
117+
// The namespace wrapper would have a 'default' property, but when
118+
// we return the default export directly, there's no 'default' property
119+
// on the returned object (unless the default export itself has one).
120+
strictEqual(timers.default, undefined);
121+
},
122+
};

‎src/workerd/api/node/tests/module-require-mutable-exports-test.wd-test‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
using Workerd = import "/workerd/workerd.capnp";
2+
3+
const unitTests :Workerd.Config = (
4+
services = [
5+
( name = "module-require-mutable-exports-test",
6+
worker = (
7+
modules = [
8+
(name = "worker", esModule = embed "module-require-mutable-exports-test.js")
9+
],
10+
compatibilityFlags = ["nodejs_compat", "require_returns_default_export"],
11+
)
12+
),
13+
],
14+
);

‎src/workerd/io/compatibility-date.capnp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,4 +1316,14 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef {
13161316
# Node.js-compatible versions from node:timers. setTimeout and setInterval return
13171317
# Timeout objects with methods like refresh(), ref(), unref(), and hasRef().
13181318
# This flag requires nodejs_compat or nodejs_compat_v2 to be enabled.
1319+
1320+
requireReturnsDefaultExport @154 :Bool
1321+
$compatEnableFlag("require_returns_default_export")
1322+
$compatDisableFlag("require_returns_namespace")
1323+
$compatEnableDate("2026-01-22");
1324+
# When enabled, require() will return the default export of a module if it exists.
1325+
# If the default export does not exist, it falls back to returning the mutable
1326+
# module namespace object. This matches the behavior that Node.js uses for
1327+
# require(esm) where the default export is returned when available.
1328+
# This flag is useful for frameworks like Next.js that expect to patch module exports.
13191329
}

‎src/workerd/io/worker.c++‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,9 @@ Worker::Isolate::Isolate(kj::Own<Api> apiParam,
11071107
if (features.getEnableNodeJsProcessV2()) {
11081108
lock->setNodeJsProcessV2Enabled();
11091109
}
1110+
if (features.getRequireReturnsDefaultExport()) {
1111+
lock->setRequireReturnsDefaultExportEnabled();
1112+
}
11101113
if (features.getThrowOnUnrecognizedImportAssertion()) {
11111114
lock->setThrowOnUnrecognizedImportAssertion();
11121115
}

‎src/workerd/jsg/jsg.c++‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,10 @@ void Lock::setNodeJsProcessV2Enabled() {
219219
IsolateBase::from(v8Isolate).setNodeJsProcessV2Enabled({}, true);
220220
}
221221

222+
void Lock::setRequireReturnsDefaultExportEnabled() {
223+
IsolateBase::from(v8Isolate).setRequireReturnsDefaultExportEnabled({}, true);
224+
}
225+
222226
void Lock::setThrowOnUnrecognizedImportAssertion() {
223227
IsolateBase::from(v8Isolate).setThrowOnUnrecognizedImportAssertion();
224228
}

‎src/workerd/jsg/jsg.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2647,6 +2647,7 @@ class Lock {
26472647

26482648
void setNodeJsCompatEnabled();
26492649
void setNodeJsProcessV2Enabled();
2650+
void setRequireReturnsDefaultExportEnabled();
26502651
void setThrowOnUnrecognizedImportAssertion();
26512652
bool getThrowOnUnrecognizedImportAssertion() const;
26522653
void setToStringTag();

‎src/workerd/jsg/jsvalue.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,11 @@ class JsObject final: public JsBase<v8::Object, JsObject> {
465465

466466
int hashCode() const;
467467

468+
// Returns true if this object is an ES module namespace object.
469+
bool isModuleNamespaceObject() const {
470+
return inner->IsModuleNamespaceObject();
471+
}
472+
468473
kj::String getConstructorName() KJ_WARN_UNUSED_RESULT;
469474
JsArray getPropertyNames(Lock& js,
470475
KeyCollectionFilter keyFilter,

0 commit comments

Comments
 (0)