Skip to content

Commit

Permalink
module: fix async resolution error within the sync findPackageJSON
Browse files Browse the repository at this point in the history
PR-URL: #56382
Reviewed-By: Jordan Harband <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
  • Loading branch information
JakobJingleheimer authored Dec 31, 2024
1 parent 4afb0e2 commit 19d58c5
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 8 deletions.
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).
> supersedes it, and a loader hook supersedes 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);
});
});

0 comments on commit 19d58c5

Please sign in to comment.