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
12 changes: 8 additions & 4 deletions doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,17 @@ added: v23.2.0
* `base` {string|URL} The absolute location (`file:` URL string or FS path) of the
containing module. For CJS, use `__filename` (not `__dirname`!); for ESM, use
`import.meta.url`. You do not need to pass it if `specifier` is an `absolute specifier`.
* Returns: {string|undefined} A path if the `package.json` is found. When `startLocation`
* Returns: {string|undefined} A path if the `package.json` is found. When `specifier`
is a package, the package's root `package.json`; when a relative or unresolved, the closest
`package.json` to the `startLocation`.
`package.json` to the `specifier`.

> **Caveat**: Do not use this to try to determine module format. There are many things effecting
> **Caveat**: Do not use this to try to determine module format. There are many things affecting
> that determination; the `type` field of package.json is the _least_ definitive (ex file extension
> superceeds it, and a loader hook superceeds that).
> supercedes it, and a loader hook supercedes that).

> **Caveat**: This currently leverages only the built-in default resolver; if
> [`resolve` customization hooks][resolve hook] are registered, they will not affect the resolution.
> This may change in the future.

```text
/path/to/project
Expand Down
13 changes: 9 additions & 4 deletions lib/internal/modules/package_json_reader.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,8 @@ function getPackageJSONURL(specifier, base) {
throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
}

const pjsonImportAttributes = { __proto__: null, type: 'json' };
let cascadedLoader;
/** @type {import('./esm/resolve.js').defaultResolve} */
let defaultResolve;
/**
* @param {URL['href'] | string | URL} specifier The location for which to get the "root" package.json
* @param {URL['href'] | string | URL} [base] The location of the current module (ex file://tmp/foo.js).
Expand Down Expand Up @@ -296,10 +296,15 @@ function findPackageJSON(specifier, base = 'data:') {
}

let resolvedTarget;
cascadedLoader ??= require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve;

try {
resolvedTarget = cascadedLoader.resolve(specifier, `${parentURL}`, pjsonImportAttributes).url;
// TODO(@JakobJingleheimer): Detect whether findPackageJSON is being used within a loader
// (possibly piggyback on `allowImportMetaResolve`)
// - When inside, use the default resolve
// - (I think it's impossible to use the chain because of re-entry & a deadlock from atomics).
// - When outside, use cascadedLoader.resolveSync (not implemented yet, but the pieces exist).
resolvedTarget = defaultResolve(specifier, { parentURL: `${parentURL}` }).url;
} catch (err) {
if (err.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
resolvedTarget = err.url;
Expand Down
40 changes: 40 additions & 0 deletions test/parallel/test-find-package-json.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,44 @@ describe('findPackageJSON', () => { // Throws when no arguments are provided
});
}));
});

it('should work within a loader', async () => {
const specifierBase = './packages/root-types-field';
const target = fixtures.fileURL(specifierBase, 'index.js');
const foundPjsonPath = path.toNamespacedPath(fixtures.path(specifierBase, 'package.json'));
const { code, stderr, stdout } = await common.spawnPromisified(process.execPath, [
'--no-warnings',
'--loader',
[
'data:text/javascript,',
'import fs from "node:fs";',
'import module from "node:module";',
encodeURIComponent(`fs.writeSync(1, module.findPackageJSON(${JSON.stringify(target)}));`),
'export const resolve = async (s, c, n) => n(s);',
].join(''),
'--eval',
'import "node:os";', // Can be anything that triggers the resolve hook chain
]);

assert.strictEqual(stderr, '');
assert.ok(stdout.includes(foundPjsonPath), stdout);
assert.strictEqual(code, 0);
});

it('should work with an async resolve hook registered', async () => {
const specifierBase = './packages/root-types-field';
const target = fixtures.fileURL(specifierBase, 'index.js');
const foundPjsonPath = path.toNamespacedPath(fixtures.path(specifierBase, 'package.json'));
const { code, stderr, stdout } = await common.spawnPromisified(process.execPath, [
'--no-warnings',
'--loader',
'data:text/javascript,export const resolve = async (s, c, n) => n(s);',
'--print',
`require("node:module").findPackageJSON(${JSON.stringify(target)})`,
]);

assert.strictEqual(stderr, '');
assert.ok(stdout.includes(foundPjsonPath), stdout);
assert.strictEqual(code, 0);
});
});
Loading