fix(ext/node): Module.wrap cleanup for npm:v8-code-cache (#29725)

This removes the additional arguments on the function returned by
`Module.wrap`.

Allow libraries that overwrite this behavior (like `v8-code-cache`) to
work correctly.

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>

ext/node/polyfills/01_require.js

@@ -65,8 +65,6 @@ const {
   TypeError,
 } = primordials;
 
-import { nodeGlobals } from "ext:deno_node/00_globals.js";
-
 import _httpAgent from "node:_http_agent";
 import _httpCommon from "node:_http_common";
 import _httpOutgoing from "node:_http_outgoing";
@@ -948,8 +946,8 @@ Module.prototype.require = function (id) {
 // wrapper function we run the users code in. The only observable difference is
 // that in Deno `arguments.callee` is not null.
 Module.wrapper = [
-  "(function (exports, require, module, __filename, __dirname, Buffer, clearImmediate, clearInterval, clearTimeout, global, process, setImmediate, setInterval, setTimeout) { (function (exports, require, module, __filename, __dirname) {",
-  "\n}).call(this, exports, require, module, __filename, __dirname); })",
+  `(function (exports, require, module, __filename, __dirname) { var { Buffer, clearImmediate, clearInterval, clearTimeout, global, process, setImmediate, setInterval, setTimeout } = Deno[Deno.internal].nodeGlobals; (() => {`,
+  "\n})(); })",
 ];
 Module.wrap = function (script) {
   script = script.replace(/^#!.*?\n/, "");
@@ -1027,18 +1025,6 @@ Module.prototype._compile = function (content, filename, format) {
     op_require_break_on_next_statement();
   }
 
-  const {
-    Buffer,
-    clearImmediate,
-    clearInterval,
-    clearTimeout,
-    global,
-    process,
-    setImmediate,
-    setInterval,
-    setTimeout,
-  } = nodeGlobals;
-
   const result = compiledWrapper.call(
     thisValue,
     exports,
@@ -1046,15 +1032,6 @@ Module.prototype._compile = function (content, filename, format) {
     this,
     filename,
     dirname,
-    Buffer,
-    clearImmediate,
-    clearInterval,
-    clearTimeout,
-    global,
-    process,
-    setImmediate,
-    setInterval,
-    setTimeout,
   );
   if (requireDepth === 0) {
     statCache = null;
@@ -1135,7 +1112,7 @@ Module._extensions[".node"] = function (module, filename) {
   module.exports = op_napi_open(
     filename,
     globalThis,
-    nodeGlobals.Buffer,
+    buffer.Buffer,
     reportError,
   );
 };

runtime/js/99_main.js

@@ -94,6 +94,7 @@ import {
 } from "ext:runtime/98_global_scope_worker.js";
 import { SymbolDispose, SymbolMetadata } from "ext:deno_web/00_infra.js";
 import { bootstrap as bootstrapOtel } from "ext:deno_telemetry/telemetry.ts";
+import { nodeGlobals } from "ext:deno_node/00_globals.js";
 
 // deno-lint-ignore prefer-primordials
 if (Symbol.metadata) {
@@ -730,7 +731,7 @@ function removeImportedOps() {
 // FIXME(bartlomieju): temporarily add whole `Deno.core` to
 // `Deno[Deno.internal]` namespace. It should be removed and only necessary
 // methods should be left there.
-ObjectAssign(internals, { core });
+ObjectAssign(internals, { core, nodeGlobals: { ...nodeGlobals } });
 const internalSymbol = Symbol("Deno.internal");
 const finalDenoNs = {
   internal: internalSymbol,

tests/unit_node/module_test.ts

@@ -109,3 +109,65 @@ Deno.test("[node/module findSourceMap] is a function", () => {
 Deno.test("[node/module register] is a function", () => {
   assertEquals(register("foo"), undefined);
 });
+
+Deno.test("[node/module] overriding Module._compile is possible and Node globals work", () => {
+  // @ts-ignore Not documented but available
+  const originalCompile = Module.prototype._compile;
+
+  // @ts-ignore Not documented but available
+  Module.prototype._compile = function (content: string, filename: string) {
+    // deno-lint-ignore no-this-alias
+    const mod = this;
+    function require(id: string) {
+      return mod.require(id);
+    }
+    require.resolve = function (request: string) {
+      // @ts-ignore Not documented but available
+      return Module._resolveFilename(request, mod);
+    };
+    require.main = process.mainModule;
+
+    // Enable support to add extra extension types
+    // @ts-ignore Not documented but available
+    require.extensions = Module._extensions;
+    // @ts-ignore Not documented but available
+    require.cache = Module._cache;
+
+    const dirname = path.dirname(filename);
+
+    const wrapper = Module.wrap(content);
+    // @ts-ignore can't index by a symbol
+    const [f, err] = Deno[Deno.internal].core.evalContext(
+      wrapper,
+      `file:///${filename}`,
+      [true],
+    );
+    assertEquals(err, null);
+    const args = [
+      mod.exports,
+      require,
+      mod,
+      filename,
+      dirname,
+      process,
+    ];
+    f.apply(mod.exports, args);
+    return mod;
+  };
+
+  const src = `module.exports = {
+  clearImmediate: typeof clearImmediate,
+  clearTimeout: typeof clearTimeout, 
+  setImmediate: typeof setImmediate, 
+  global: typeof global
+};`;
+  // @ts-ignore Not documented but available
+  const ret = Module.prototype._compile(src, "file.js");
+  const exports = ret.exports;
+  assertEquals(exports.clearImmediate, "function");
+  assertEquals(exports.clearTimeout, "function");
+  assertEquals(exports.setImmediate, "function");
+  assertEquals(exports.global, "object");
+  // @ts-ignore Not documented but available
+  Module.prototype._compile = originalCompile;
+});