diff --git a/cli/tests/testdata/npm/compare_globals/main.out b/cli/tests/testdata/npm/compare_globals/main.out index 8b3b62bc1b..e60a39ba63 100644 --- a/cli/tests/testdata/npm/compare_globals/main.out +++ b/cli/tests/testdata/npm/compare_globals/main.out @@ -5,3 +5,6 @@ Download http://localhost:4545/npm/registry/@types/node/node-18.8.2.tgz Check file:///[WILDCARD]/npm/compare_globals/main.ts true [] +5 +undefined +undefined diff --git a/cli/tests/testdata/npm/compare_globals/main.ts b/cli/tests/testdata/npm/compare_globals/main.ts index 5710d0bd59..0468404a88 100644 --- a/cli/tests/testdata/npm/compare_globals/main.ts +++ b/cli/tests/testdata/npm/compare_globals/main.ts @@ -12,3 +12,16 @@ type _TestHasNodeJsGlobal = NodeJS.Architecture; const controller = new AbortController(); controller.abort("reason"); // in the NodeJS declaration it doesn't have a reason + +// Super edge case where some Node code deletes a global where the +// Node code has its own global and the Deno code has the same global, +// but it's different. Basically if some Node code deletes +// one of these globals then we don't want it to suddenly inherit +// the Deno global. +globals.withNodeGlobalThis((nodeGlobalThis: any) => { + (globalThis as any).setTimeout = 5; + console.log(setTimeout); + delete nodeGlobalThis["setTimeout"]; + console.log(nodeGlobalThis["setTimeout"]); // should be undefined + console.log(globalThis["setTimeout"]); // should be undefined +}); diff --git a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts index ee03712dd4..3f3eeb92af 100644 --- a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts +++ b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.d.ts @@ -11,3 +11,5 @@ type AssertTrue = never; type _TestHasProcessGlobal = AssertTrue< typeof globalThis extends { process: any } ? true : false >; + +export function withNodeGlobalThis(action: (global: typeof globalThis) => void): void; diff --git a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js index 50d2d3d2a3..daac83c664 100644 --- a/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js +++ b/cli/tests/testdata/npm/registry/@denotest/globals/1.0.0/index.js @@ -1,3 +1,7 @@ exports.globalThis = globalThis; exports.global = global; exports.process = process; + +exports.withNodeGlobalThis = function (action) { + action(globalThis); +}; diff --git a/ext/node/01_node.js b/ext/node/01_node.js index de27c5180b..bbae660834 100644 --- a/ext/node/01_node.js +++ b/ext/node/01_node.js @@ -12,8 +12,12 @@ const { ObjectDefineProperty, Proxy, ReflectDefineProperty, + ReflectDeleteProperty, + ReflectGet, ReflectGetOwnPropertyDescriptor, + ReflectHas, ReflectOwnKeys, + ReflectSet, Set, SetPrototypeHas, } = primordials; @@ -27,62 +31,54 @@ function assert(cond) { let initialized = false; const nodeGlobals = {}; const nodeGlobalThis = new Proxy(globalThis, { - get(_target, prop, _receiver) { - if (prop in nodeGlobals) { - return nodeGlobals[prop]; + get(target, prop) { + if (ReflectHas(nodeGlobals, prop)) { + return ReflectGet(nodeGlobals, prop); } else { - return globalThis[prop]; + return ReflectGet(target, prop); } }, - set(_target, prop, value) { - if (prop in nodeGlobals) { - nodeGlobals[prop] = value; + set(target, prop, value) { + if (ReflectHas(nodeGlobals, prop)) { + return ReflectSet(nodeGlobals, prop, value); } else { - globalThis[prop] = value; + return ReflectSet(target, prop, value); } - return true; }, - deleteProperty(_target, prop) { - let success = false; - if (prop in nodeGlobals) { - delete nodeGlobals[prop]; - success = true; - } - if (prop in globalThis) { - delete globalThis[prop]; - success = true; - } - return success; + has(target, prop) { + return ReflectHas(nodeGlobals, prop) || ReflectHas(target, prop); }, - ownKeys(_target) { - const globalThisKeys = ReflectOwnKeys(globalThis); + deleteProperty(target, prop) { + const nodeDeleted = ReflectDeleteProperty(nodeGlobals, prop); + const targetDeleted = ReflectDeleteProperty(target, prop); + return nodeDeleted || targetDeleted; + }, + ownKeys(target) { + const targetKeys = ReflectOwnKeys(target); const nodeGlobalsKeys = ReflectOwnKeys(nodeGlobals); const nodeGlobalsKeySet = new Set(nodeGlobalsKeys); return [ ...ArrayPrototypeFilter( - globalThisKeys, + targetKeys, (k) => !SetPrototypeHas(nodeGlobalsKeySet, k), ), ...nodeGlobalsKeys, ]; }, - defineProperty(_target, prop, desc) { - if (prop in nodeGlobals) { + defineProperty(target, prop, desc) { + if (ReflectHas(nodeGlobals, prop)) { return ReflectDefineProperty(nodeGlobals, prop, desc); } else { - return ReflectDefineProperty(globalThis, prop, desc); + return ReflectDefineProperty(target, prop, desc); } }, - getOwnPropertyDescriptor(_target, prop) { - if (prop in nodeGlobals) { + getOwnPropertyDescriptor(target, prop) { + if (ReflectHas(nodeGlobals, prop)) { return ReflectGetOwnPropertyDescriptor(nodeGlobals, prop); } else { - return ReflectGetOwnPropertyDescriptor(globalThis, prop); + return ReflectGetOwnPropertyDescriptor(target, prop); } }, - has(_target, prop) { - return prop in nodeGlobals || prop in globalThis; - }, }); const nativeModuleExports = ObjectCreate(null);