Skip to content

module: fix async resolution error within the sync findPackageJSON #56382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
7 changes: 7 additions & 0 deletions lib/internal/modules/package_json_reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,13 @@ function findPackageJSON(specifier, base = 'data:') {
cascadedLoader ??= require('internal/modules/esm/loader').getOrInitializeCascadedLoader();

try {
// The problem is here: when an async resolve hook is registered, resolve returns a promise
// I think 2 things are needed:
// 1. detect whether findPackageJSON is being used within a loader
// (possibly piggyback on `allowImportMetaResolve`)
// 2a. When inside, use the default resolve
// (I think it's impossible to use the chain because of re-entry & a deadlock from atomics).
// 2b. When outside, use cascadedLoader.resolveSync (not implemented yet, but the pieces exist).
resolvedTarget = cascadedLoader.resolve(specifier, `${parentURL}`, pjsonImportAttributes).url;
} catch (err) {
if (err.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-find-package-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
const fixtures = require('../common/fixtures');
const tmpdir = require('../common/tmpdir');
const assert = require('node:assert');
const { spawnSync } = require('node:child_process');
const fs = require('node:fs');
const { findPackageJSON } = require('node:module');
const path = require('node:path');
Expand Down Expand Up @@ -149,4 +150,49 @@
});
}));
});

it('should work within a loader', () => {
const importMetaUrl = `${fixtures.fileURL('whatever.ext')}`;
const specifierBase = './packages/root-types-field';
const foundPjsonPath = path.toNamespacedPath(fixtures.path(specifierBase, 'package.json'));
const { status: code, stderr, stdout } = spawnSync(process.execPath, [
'--no-warnings',
'--loader',
[
'data:text/javascript,',
'import module from "node:module";',
`module.findPackageJSON('${specifierBase}','${importMetaUrl}');`,
'export const resolve = async (s, c, n) => n(s);',
].join(''),
'--eval',
'import module from "node:module";', // can be anything that triggers the resolve hook chain

Check failure on line 168 in test/parallel/test-find-package-json.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Comments should not begin with a lowercase character
], { encoding: 'utf-8' });

// Error you're trying to fix:
// TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance
// of URL. Received undefined
assert.strictEqual(stderr, '');

Check failure on line 174 in test/parallel/test-find-package-json.js

View workflow job for this annotation

GitHub Actions / test-macOS (macos-13)

--- stderr --- --- stdout --- ::debug::starting to run findPackageJSON ::debug::starting to run should throw when no arguments are provided ▶ findPackageJSON ✔ should throw when no arguments are provided (1.582495ms) ✔ should throw when parentLocation is invalid (0.595537ms) ✔ should accept a file URL (string), like from `import.meta.resolve()` (1.714139ms) ✔ should accept a file URL instance (0.385762ms) ::debug::completed running should throw when no arguments are provided ::debug::starting to run should throw when parentLocation is invalid ::debug::completed running should throw when parentLocation is invalid ::debug::starting to run should accept a file URL (string), like from `import.meta.resolve()` ::debug::completed running should accept a file URL (string), like from `import.meta.resolve()` ::debug::starting to run should accept a file URL instance ::debug::completed running should accept a file URL instance ::debug::starting to run should be able to crawl up (CJS) ✔ should be able to crawl up (CJS) (2.693416ms) ✔ should be able to crawl up (ESM) (2.709033ms) ✔ can require via package.json (3.468298ms) ✔ should be able to resolve both root and closest package.json (6.599981ms) ::debug::completed running should be able to crawl up (CJS) ::debug::starting to run should be able to crawl up (ESM) ::debug::completed running should be able to crawl up (ESM) ::debug::starting to run can require via package.json ::debug::completed running can require via package.json ::debug::starting to run should be able to resolve both root and closest package.json ::debug::completed running should be able to resolve both root and closest package.json ::debug::starting to run should work within a loader ✖ should work within a loader (103.675031ms) AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: + actual - expected + '\n' + + 'node:internal/modules/run_main:104\n' + + ' triggerUncaughtException(\n' + + ' ^\n' + + 'TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined\n' + + ' at fileURLToPath (node:internal/url:1507:11)\n' + + ' at Function.findPackageJSON (node:internal/modules/package_json_reader:318:43)\n' + + ` at data:text/javascript,import module from "node:module";module.findPackageJSON('./packages/root-types-field','file:///Users/runner/work/node/node/test/fixtures/whatever.ext');export const resolve = async (s, c, n) => n(s);:1:41\n` + + ' at ModuleJob.run (node:internal/modules/esm/module_job:272:25)\n' + + ' at process.processTicksAndRejections (node:internal/process/task_queues:105:5)\n' + + ' at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:580:26)\n' + + ' at async Hooks.register (node:internal/modules/esm/hooks:152:7)\n' + + ' at async initializeHooks (node:internal/modules/esm/utils:318:5)\n' + + ' at async customizedModuleWorker (node:internal/modules/esm/worker:109:13) {\n' + + " code: 'ERR_INVALID_ARG_TYPE'\n" + + '}\n' + + '\n' + + 'Node.js v24.0.0-pre\n' - '' at TestContext.<anonymous> (/Users/runner/work/node/node/test/parallel/test-find-package-json.js:174:12) at Test.runInAsyncScope (node:async_hooks:211:14) at Test.run (node:internal/test_runner/test:931:25) at Suite.processPendingSubtests (node:internal/test_runner/test:629:18) at Test.postRun (node:internal/test_runner/test:1042:19) at Test.run (node:internal/test_runner/test:970:12) at async Suite.processPendingSubtests (node:internal/test_runner/test:629:7) { generatedMessage: true, code: 'ERR_ASSERTION', actual: `\nnode:internal/modules/run_main:104\n triggerUncaughtException(\n ^\nTypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined\n at fileURLToPath (node:internal/url:1507:11)\n at Function.findPackageJSON (node:int

Check failure on line 174 in test/parallel/test-find-package-json.js

View workflow job for this annotation

GitHub Actions / test-linux

--- stderr --- --- stdout --- ::debug::starting to run findPackageJSON ::debug::starting to run should throw when no arguments are provided ▶ findPackageJSON ✔ should throw when no arguments are provided (1.474179ms) ✔ should throw when parentLocation is invalid (0.501281ms) ✔ should accept a file URL (string), like from `import.meta.resolve()` (0.655451ms) ✔ should accept a file URL instance (0.327194ms) ::debug::completed running should throw when no arguments are provided ::debug::starting to run should throw when parentLocation is invalid ::debug::completed running should throw when parentLocation is invalid ::debug::starting to run should accept a file URL (string), like from `import.meta.resolve()` ::debug::completed running should accept a file URL (string), like from `import.meta.resolve()` ::debug::starting to run should accept a file URL instance ::debug::completed running should accept a file URL instance ::debug::starting to run should be able to crawl up (CJS) ✔ should be able to crawl up (CJS) (0.978026ms) ✔ should be able to crawl up (ESM) (1.855264ms) ✔ can require via package.json (0.712569ms) ✔ should be able to resolve both root and closest package.json (5.701676ms) ::debug::completed running should be able to crawl up (CJS) ::debug::starting to run should be able to crawl up (ESM) ::debug::completed running should be able to crawl up (ESM) ::debug::starting to run can require via package.json ::debug::completed running can require via package.json ::debug::starting to run should be able to resolve both root and closest package.json ::debug::completed running should be able to resolve both root and closest package.json ::debug::starting to run should work within a loader ✖ should work within a loader (72.230541ms) AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: + actual - expected + '\n' + + 'node:internal/modules/run_main:104\n' + + ' triggerUncaughtException(\n' + + ' ^\n' + + 'TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined\n' + + ' at fileURLToPath (node:internal/url:1507:11)\n' + + ' at Function.findPackageJSON (node:internal/modules/package_json_reader:318:43)\n' + + ` at data:text/javascript,import module from "node:module";module.findPackageJSON('./packages/root-types-field','file:///home/runner/work/node/node/test/fixtures/whatever.ext');export const resolve = async (s, c, n) => n(s);:1:41\n` + + ' at ModuleJob.run (node:internal/modules/esm/module_job:272:25)\n' + + ' at process.processTicksAndRejections (node:internal/process/task_queues:105:5)\n' + + ' at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:580:26)\n' + + ' at async Hooks.register (node:internal/modules/esm/hooks:152:7)\n' + + ' at async initializeHooks (node:internal/modules/esm/utils:318:5)\n' + + ' at async customizedModuleWorker (node:internal/modules/esm/worker:109:13) {\n' + + " code: 'ERR_INVALID_ARG_TYPE'\n" + + '}\n' + + '\n' + + 'Node.js v24.0.0-pre\n' - '' at TestContext.<anonymous> (/home/runner/work/node/node/test/parallel/test-find-package-json.js:174:12) at Test.runInAsyncScope (node:async_hooks:211:14) at Test.run (node:internal/test_runner/test:931:25) at Suite.processPendingSubtests (node:internal/test_runner/test:629:18) at Test.postRun (node:internal/test_runner/test:1042:19) at Test.run (node:internal/test_runner/test:970:12) at async Suite.processPendingSubtests (node:internal/test_runner/test:629:7) { generatedMessage: true, code: 'ERR_ASSERTION', actual: `\nnode:internal/modules/run_main:104\n triggerUncaughtException(\n ^\nTypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined\n at fileURLToPath (node:internal/url:1507:11)\n at Function.findPackageJSON (node:intern

Check failure on line 174 in test/parallel/test-find-package-json.js

View workflow job for this annotation

GitHub Actions / test-macOS (macos-14)

--- stderr --- --- stdout --- ::debug::starting to run findPackageJSON ::debug::starting to run should throw when no arguments are provided ▶ findPackageJSON ✔ should throw when no arguments are provided (0.71825ms) ✔ should throw when parentLocation is invalid (0.252083ms) ✔ should accept a file URL (string), like from `import.meta.resolve()` (1.214333ms) ✔ should accept a file URL instance (0.140708ms) ::debug::completed running should throw when no arguments are provided ::debug::starting to run should throw when parentLocation is invalid ::debug::completed running should throw when parentLocation is invalid ::debug::starting to run should accept a file URL (string), like from `import.meta.resolve()` ::debug::completed running should accept a file URL (string), like from `import.meta.resolve()` ::debug::starting to run should accept a file URL instance ::debug::completed running should accept a file URL instance ::debug::starting to run should be able to crawl up (CJS) ✔ should be able to crawl up (CJS) (0.943ms) ✔ should be able to crawl up (ESM) (1.079042ms) ✔ can require via package.json (0.779583ms) ✔ should be able to resolve both root and closest package.json (3.010958ms) ::debug::completed running should be able to crawl up (CJS) ::debug::starting to run should be able to crawl up (ESM) ::debug::completed running should be able to crawl up (ESM) ::debug::starting to run can require via package.json ::debug::completed running can require via package.json ::debug::starting to run should be able to resolve both root and closest package.json ::debug::completed running should be able to resolve both root and closest package.json ::debug::starting to run should work within a loader ✖ should work within a loader (40.452958ms) AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: + actual - expected + '\n' + + 'node:internal/modules/run_main:104\n' + + ' triggerUncaughtException(\n' + + ' ^\n' + + 'TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined\n' + + ' at fileURLToPath (node:internal/url:1507:11)\n' + + ' at Function.findPackageJSON (node:internal/modules/package_json_reader:318:43)\n' + + ` at data:text/javascript,import module from "node:module";module.findPackageJSON('./packages/root-types-field','file:///Users/runner/work/node/node/test/fixtures/whatever.ext');export const resolve = async (s, c, n) => n(s);:1:41\n` + + ' at ModuleJob.run (node:internal/modules/esm/module_job:272:25)\n' + + ' at process.processTicksAndRejections (node:internal/process/task_queues:105:5)\n' + + ' at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:580:26)\n' + + ' at async Hooks.register (node:internal/modules/esm/hooks:152:7)\n' + + ' at async initializeHooks (node:internal/modules/esm/utils:318:5)\n' + + ' at async customizedModuleWorker (node:internal/modules/esm/worker:109:13) {\n' + + " code: 'ERR_INVALID_ARG_TYPE'\n" + + '}\n' + + '\n' + + 'Node.js v24.0.0-pre\n' - '' at TestContext.<anonymous> (/Users/runner/work/node/node/test/parallel/test-find-package-json.js:174:12) at Test.runInAsyncScope (node:async_hooks:211:14) at Test.run (node:internal/test_runner/test:931:25) at Suite.processPendingSubtests (node:internal/test_runner/test:629:18) at Test.postRun (node:internal/test_runner/test:1042:19) at Test.run (node:internal/test_runner/test:970:12) at async Suite.processPendingSubtests (node:internal/test_runner/test:629:7) { generatedMessage: true, code: 'ERR_ASSERTION', actual: `\nnode:internal/modules/run_main:104\n triggerUncaughtException(\n ^\nTypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance of URL. Received undefined\n at fileURLToPath (node:internal/url:1507:11)\n at Function.findPackageJSON (node:internal
assert.match(stdout, new RegExp(foundPjsonPath));
assert.strictEqual(code, 0);
});

it('should work with an async resolve hook registered', () => {
const importMetaUrl = `${fixtures.fileURL('whatever.ext')}`;
const specifierBase = './packages/root-types-field';
const foundPjsonPath = path.toNamespacedPath(fixtures.path(specifierBase, 'package.json'));
const { status: code, stderr, stdout } = spawnSync(process.execPath, [
'--no-warnings',
'--loader',
'data:text/javascript,export const resolve = async (s, c, n) => n(s);',
'--eval',
`console.log(require("node:module").findPackageJSON('${specifierBase}','${importMetaUrl}'));`

Check failure on line 188 in test/parallel/test-find-package-json.js

View workflow job for this annotation

GitHub Actions / lint-js-and-md

Missing trailing comma
], { encoding: 'utf-8' });

// Error you're trying to fix:
// TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string or an instance
// of URL. Received undefined
assert.strictEqual(stderr, '');
assert.match(stdout, new RegExp(foundPjsonPath));
assert.strictEqual(code, 0);
});
});
Loading