From f21d48bc0ac09fbb60d524ff5401f97036f125a8 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 27 Mar 2023 21:54:22 +0200 Subject: [PATCH] fix(ext/node): add missing _preloadModules hook (#18447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This internal node hook is used by libraries such as `ts-node` when used as a require hook `node -r ts-node/register`. That combination is often used with test frameworks like `mocha` or `jasmine`. We had a reference to `Module._preloadModules` in our code, but the implementation was missing. While fixing this I also noticed that the `fakeParent` module that we create internally always threw because of the `pathDirname` check on the module id in the constructor of `Mdoule`. So this code path was probably broken for a while. ```txt ✖ ERROR: Error: Empty filepath. at pathDirname (ext:deno_node/01_require.js:245:11) at new Module (ext:deno_node/01_require.js:446:15) at Function.Module._resolveFilename (ext:deno_node/01_require.js:754:28) at Function.resolve (ext:deno_node/01_require.js:1015:19) ``` --- cli/tests/unit_node/module_test.ts | 14 +++++++++ .../unit_node/testdata/add_global_property.js | 1 + ext/node/polyfills/01_require.js | 30 +++++++++++++++---- 3 files changed, 40 insertions(+), 5 deletions(-) create mode 100644 cli/tests/unit_node/module_test.ts create mode 100644 cli/tests/unit_node/testdata/add_global_property.js diff --git a/cli/tests/unit_node/module_test.ts b/cli/tests/unit_node/module_test.ts new file mode 100644 index 0000000000..d071ed2d18 --- /dev/null +++ b/cli/tests/unit_node/module_test.ts @@ -0,0 +1,14 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +import { Module } from "node:module"; +import { assertStrictEquals } from "../../../test_util/std/testing/asserts.ts"; + +Deno.test("[node/module _preloadModules] has internal require hook", () => { + // Check if it's there + // deno-lint-ignore no-explicit-any + (Module as any)._preloadModules([ + "./cli/tests/unit_node/testdata/add_global_property.js", + ]); + // deno-lint-ignore no-explicit-any + assertStrictEquals((globalThis as any).foo, "Hello"); +}); diff --git a/cli/tests/unit_node/testdata/add_global_property.js b/cli/tests/unit_node/testdata/add_global_property.js new file mode 100644 index 0000000000..814d17d0d1 --- /dev/null +++ b/cli/tests/unit_node/testdata/add_global_property.js @@ -0,0 +1 @@ +globalThis.foo = "Hello"; diff --git a/ext/node/polyfills/01_require.js b/ext/node/polyfills/01_require.js index 68398df690..42ead05e32 100644 --- a/ext/node/polyfills/01_require.js +++ b/ext/node/polyfills/01_require.js @@ -241,8 +241,10 @@ setupBuiltinModules(); const cjsParseCache = new SafeWeakMap(); function pathDirname(filepath) { - if (filepath == null || filepath === "") { + if (filepath == null) { throw new Error("Empty filepath."); + } else if (filepath === "") { + return "."; } return ops.op_require_path_dirname(filepath); } @@ -605,10 +607,10 @@ Module._nodeModulePaths = function (fromPath) { Module._resolveLookupPaths = function (request, parent) { const paths = []; - if (ops.op_require_is_request_relative(request) && parent?.filename) { + if (ops.op_require_is_request_relative(request)) { ArrayPrototypePush( paths, - ops.op_require_path_dirname(parent.filename), + parent?.filename ? ops.op_require_path_dirname(parent.filename) : ".", ); return paths; } @@ -752,7 +754,6 @@ Module._resolveFilename = function ( paths = options.paths; } else { const fakeParent = new Module("", null); - paths = []; for (let i = 0; i < options.paths.length; i++) { @@ -828,6 +829,25 @@ Module._resolveFilename = function ( throw err; }; +/** + * Internal CommonJS API to always require modules before requiring the actual + * one when calling `require("my-module")`. This is used by require hooks such + * as `ts-node/register`. + * @param {string[]} requests List of modules to preload + */ +Module._preloadModules = function (requests) { + if (!ArrayIsArray(requests) || requests.length === 0) { + return; + } + + const parent = new Module("internal/preload", null); + // All requested files must be resolved against cwd + parent.paths = Module._nodeModulePaths(process.cwd()); + for (let i = 0; i < requests.length; i++) { + parent.require(requests[i]); + } +}; + Module.prototype.load = function (filename) { if (this.loaded) { throw Error("Module already loaded"); @@ -954,7 +974,7 @@ Module._extensions[".js"] = function (module, filename) { const content = ops.op_require_read_file(filename); if (StringPrototypeEndsWith(filename, ".js")) { - const pkg = ops.op_require_read_closest_package_json(filename); + const pkg = ops.op_require_read_package_scope(filename); if (pkg && pkg.exists && pkg.typ == "module") { let message = `Trying to import ESM module: ${filename}`;