Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 4f77920

Browse filesBrowse files
JakobJingleheimeraduh95
authored andcommitted
module: fix async resolution error within the sync findPackageJSON
PR-URL: #56382 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
1 parent 6afe363 commit 4f77920
Copy full SHA for 4f77920

File tree

Expand file treeCollapse file tree

3 files changed

+57
-8
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+57
-8
lines changed
Open diff view settings
Collapse file

‎doc/api/module.md‎

Copy file name to clipboardExpand all lines: doc/api/module.md
+8-4Lines changed: 8 additions & 4 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,17 @@ added: v23.2.0
8181
* `base` {string|URL} The absolute location (`file:` URL string or FS path) of the
8282
containing module. For CJS, use `__filename` (not `__dirname`!); for ESM, use
8383
`import.meta.url`. You do not need to pass it if `specifier` is an `absolute specifier`.
84-
* Returns: {string|undefined} A path if the `package.json` is found. When `startLocation`
84+
* Returns: {string|undefined} A path if the `package.json` is found. When `specifier`
8585
is a package, the package's root `package.json`; when a relative or unresolved, the closest
86-
`package.json` to the `startLocation`.
86+
`package.json` to the `specifier`.
8787
88-
> **Caveat**: Do not use this to try to determine module format. There are many things effecting
88+
> **Caveat**: Do not use this to try to determine module format. There are many things affecting
8989
> that determination; the `type` field of package.json is the _least_ definitive (ex file extension
90-
> superceeds it, and a loader hook superceeds that).
90+
> supersedes it, and a loader hook supersedes that).
91+
92+
> **Caveat**: This currently leverages only the built-in default resolver; if
93+
> [`resolve` customization hooks][resolve hook] are registered, they will not affect the resolution.
94+
> This may change in the future.
9195
9296
```text
9397
/path/to/project
Collapse file

‎lib/internal/modules/package_json_reader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/package_json_reader.js
+9-4Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,8 @@ function getPackageJSONURL(specifier, base) {
267267
throw new ERR_MODULE_NOT_FOUND(packageName, fileURLToPath(base), null);
268268
}
269269

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

298298
let resolvedTarget;
299-
cascadedLoader ??= require('internal/modules/esm/loader').getOrInitializeCascadedLoader();
299+
defaultResolve ??= require('internal/modules/esm/resolve').defaultResolve;
300300

301301
try {
302-
resolvedTarget = cascadedLoader.resolve(specifier, `${parentURL}`, pjsonImportAttributes).url;
302+
// TODO(@JakobJingleheimer): Detect whether findPackageJSON is being used within a loader
303+
// (possibly piggyback on `allowImportMetaResolve`)
304+
// - When inside, use the default resolve
305+
// - (I think it's impossible to use the chain because of re-entry & a deadlock from atomics).
306+
// - When outside, use cascadedLoader.resolveSync (not implemented yet, but the pieces exist).
307+
resolvedTarget = defaultResolve(specifier, { parentURL: `${parentURL}` }).url;
303308
} catch (err) {
304309
if (err.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
305310
resolvedTarget = err.url;
Collapse file

‎test/parallel/test-find-package-json.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-find-package-json.js
+40Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,4 +149,44 @@ describe('findPackageJSON', () => { // Throws when no arguments are provided
149149
});
150150
}));
151151
});
152+
153+
it('should work within a loader', async () => {
154+
const specifierBase = './packages/root-types-field';
155+
const target = fixtures.fileURL(specifierBase, 'index.js');
156+
const foundPjsonPath = path.toNamespacedPath(fixtures.path(specifierBase, 'package.json'));
157+
const { code, stderr, stdout } = await common.spawnPromisified(process.execPath, [
158+
'--no-warnings',
159+
'--loader',
160+
[
161+
'data:text/javascript,',
162+
'import fs from "node:fs";',
163+
'import module from "node:module";',
164+
encodeURIComponent(`fs.writeSync(1, module.findPackageJSON(${JSON.stringify(target)}));`),
165+
'export const resolve = async (s, c, n) => n(s);',
166+
].join(''),
167+
'--eval',
168+
'import "node:os";', // Can be anything that triggers the resolve hook chain
169+
]);
170+
171+
assert.strictEqual(stderr, '');
172+
assert.ok(stdout.includes(foundPjsonPath), stdout);
173+
assert.strictEqual(code, 0);
174+
});
175+
176+
it('should work with an async resolve hook registered', async () => {
177+
const specifierBase = './packages/root-types-field';
178+
const target = fixtures.fileURL(specifierBase, 'index.js');
179+
const foundPjsonPath = path.toNamespacedPath(fixtures.path(specifierBase, 'package.json'));
180+
const { code, stderr, stdout } = await common.spawnPromisified(process.execPath, [
181+
'--no-warnings',
182+
'--loader',
183+
'data:text/javascript,export const resolve = async (s, c, n) => n(s);',
184+
'--print',
185+
`require("node:module").findPackageJSON(${JSON.stringify(target)})`,
186+
]);
187+
188+
assert.strictEqual(stderr, '');
189+
assert.ok(stdout.includes(foundPjsonPath), stdout);
190+
assert.strictEqual(code, 0);
191+
});
152192
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.