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 3c04034

Browse filesBrowse files
JakobJingleheimerJakobJingleheimer
authored
Revert "esm: convert resolve hook to synchronous"
This reverts commit 90b634a. PR-URL: #43526 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
1 parent f5ce6b1 commit 3c04034
Copy full SHA for 3c04034
Expand file treeCollapse file tree

30 files changed

+127
-216
lines changed
Open diff view settings
Collapse file

‎doc/api/esm.md‎

Copy file name to clipboardExpand all lines: doc/api/esm.md
+8-21Lines changed: 8 additions & 21 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,6 @@ added:
324324
- v13.9.0
325325
- v12.16.2
326326
changes:
327-
- version: REPLACEME
328-
pr-url: https://github.com/nodejs/node/pull/43363
329-
description: Convert from asynchronous to synchronous.
330327
- version:
331328
- v16.2.0
332329
- v14.18.0
@@ -342,19 +339,15 @@ command flag enabled.
342339
* `specifier` {string} The module specifier to resolve relative to `parent`.
343340
* `parent` {string|URL} The absolute parent module URL to resolve from. If none
344341
is specified, the value of `import.meta.url` is used as the default.
345-
* Returns: {string}
342+
* Returns: {Promise}
346343
347344
Provides a module-relative resolution function scoped to each module, returning
348-
the URL string. In alignment with browser behavior, this now returns
349-
synchronously.
350-
351-
> **Caveat** This can result in synchronous file-system operations, which
352-
> can impact performance similarly to `require.resolve`.
345+
the URL string.
353346
354347
<!-- eslint-skip -->
355348
356349
```js
357-
const dependencyAsset = import.meta.resolve('component-lib/asset.css');
350+
const dependencyAsset = await import.meta.resolve('component-lib/asset.css');
358351
```
359352
360353
`import.meta.resolve` also accepts a second argument which is the parent module
@@ -363,11 +356,11 @@ from which to resolve from:
363356
<!-- eslint-skip -->
364357
365358
```js
366-
import.meta.resolve('./dep', import.meta.url);
359+
await import.meta.resolve('./dep', import.meta.url);
367360
```
368361
369-
This function is synchronous because the ES module resolver in Node.js is
370-
synchronous.
362+
This function is asynchronous because the ES module resolver in Node.js is
363+
allowed to be asynchronous.
371364
372365
## Interoperability with CommonJS
373366
@@ -738,9 +731,6 @@ prevent unintentional breaks in the chain.
738731
739732
<!-- YAML
740733
changes:
741-
- version: REPLACEME
742-
pr-url: https://github.com/nodejs/node/pull/43363
743-
description: Convert hook from asynchronous to synchronous.
744734
- version: REPLACEME
745735
pr-url: https://github.com/nodejs/node/pull/42623
746736
description: Add support for chaining resolve hooks. Each hook must either
@@ -774,9 +764,6 @@ changes:
774764
terminate the chain of `resolve` hooks. **Default:** `false`
775765
* `url` {string} The absolute URL to which this input resolves
776766
777-
> **Caveat** A resolve hook can contain synchronous file-system operations
778-
> (as `defaultResolveHook()` does), which can impact performance.
779-
780767
The `resolve` hook chain is responsible for resolving file URL for a given
781768
module specifier and parent URL, and optionally its format (such as `'module'`)
782769
as a hint to the `load` hook. If a format is specified, the `load` hook is
@@ -803,7 +790,7 @@ Node.js module specifier resolution behavior_ when calling `defaultResolve`, the
803790
`context.conditions` array originally passed into the `resolve` hook.
804791
805792
```js
806-
export function resolve(specifier, context, nextResolve) {
793+
export async function resolve(specifier, context, nextResolve) {
807794
const { parentURL = null } = context;
808795

809796
if (Math.random() > 0.5) { // Some condition.
@@ -1102,7 +1089,7 @@ const baseURL = pathToFileURL(`${cwd()}/`).href;
11021089
// CoffeeScript files end in .coffee, .litcoffee, or .coffee.md.
11031090
const extensionsRegex = /\.coffee$|\.litcoffee$|\.coffee\.md$/;
11041091
1105-
export function resolve(specifier, context, nextResolve) {
1092+
export async function resolve(specifier, context, nextResolve) {
11061093
if (extensionsRegex.test(specifier)) {
11071094
const { parentURL = baseURL } = context;
11081095
Collapse file

‎lib/internal/modules/esm/initialize_import_meta.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/initialize_import_meta.js
+12-14Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,21 @@
33
const { getOptionValue } = require('internal/options');
44
const experimentalImportMetaResolve =
55
getOptionValue('--experimental-import-meta-resolve');
6+
const {
7+
PromisePrototypeThen,
8+
PromiseReject,
9+
} = primordials;
610
const asyncESM = require('internal/process/esm_loader');
711

812
function createImportMetaResolve(defaultParentUrl) {
9-
return function resolve(specifier, parentUrl = defaultParentUrl) {
10-
let url;
11-
12-
try {
13-
({ url } = asyncESM.esmLoader.resolve(specifier, parentUrl));
14-
} catch (error) {
15-
if (error.code === 'ERR_UNSUPPORTED_DIR_IMPORT') {
16-
({ url } = error);
17-
} else {
18-
throw error;
19-
}
20-
}
21-
22-
return url;
13+
return async function resolve(specifier, parentUrl = defaultParentUrl) {
14+
return PromisePrototypeThen(
15+
asyncESM.esmLoader.resolve(specifier, parentUrl),
16+
({ url }) => url,
17+
(error) => (
18+
error.code === 'ERR_UNSUPPORTED_DIR_IMPORT' ?
19+
error.url : PromiseReject(error))
20+
);
2321
};
2422
}
2523

Collapse file

‎lib/internal/modules/esm/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/loader.js
+31-64Lines changed: 31 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ const {
1515
ObjectDefineProperty,
1616
ObjectSetPrototypeOf,
1717
PromiseAll,
18-
PromiseResolve,
19-
PromisePrototypeThen,
2018
ReflectApply,
2119
RegExpPrototypeExec,
2220
SafeArrayIterator,
@@ -111,14 +109,12 @@ let emittedSpecifierResolutionWarning = false;
111109
* position in the hook chain.
112110
* @param {string} meta.hookName The kind of hook the chain is (ex 'resolve')
113111
* @param {boolean} meta.shortCircuited Whether a hook signaled a short-circuit.
114-
* @param {object} validators A wrapper function
112+
* @param {(hookErrIdentifier, hookArgs) => void} validate A wrapper function
115113
* containing all validation of a custom loader hook's intermediary output. Any
116114
* validation within MUST throw.
117-
* @param {(hookErrIdentifier, hookArgs) => void} validators.validateArgs
118-
* @param {(hookErrIdentifier, output) => void} validators.validateOutput
119115
* @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
120116
*/
121-
function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
117+
function nextHookFactory(chain, meta, validate) {
122118
// First, prepare the current
123119
const { hookName } = meta;
124120
const {
@@ -141,7 +137,7 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
141137
// factory generates the next link in the chain.
142138
meta.hookIndex--;
143139

144-
nextNextHook = nextHookFactory(chain, meta, { validateArgs, validateOutput });
140+
nextNextHook = nextHookFactory(chain, meta, validate);
145141
} else {
146142
// eslint-disable-next-line func-name-matching
147143
nextNextHook = function chainAdvancedTooFar() {
@@ -152,36 +148,21 @@ function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
152148
}
153149

154150
return ObjectDefineProperty(
155-
(...args) => {
151+
async (...args) => {
156152
// Update only when hook is invoked to avoid fingering the wrong filePath
157153
meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;
158154

159-
validateArgs(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);
160-
161-
const outputErrIdentifier = `${chain[generatedHookIndex].url} '${hookName}' hook's ${nextHookName}()`;
155+
validate(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);
162156

163157
// Set when next<HookName> is actually called, not just generated.
164158
if (generatedHookIndex === 0) { meta.chainFinished = true; }
165159

166160
ArrayPrototypePush(args, nextNextHook);
167-
const output = ReflectApply(hook, undefined, args);
168-
169-
validateOutput(outputErrIdentifier, output);
161+
const output = await ReflectApply(hook, undefined, args);
170162

171-
function checkShortCircuited(output) {
172-
if (output?.shortCircuit === true) { meta.shortCircuited = true; }
173-
174-
return output;
175-
}
176-
177-
if (meta.isChainAsync) {
178-
return PromisePrototypeThen(
179-
PromiseResolve(output),
180-
checkShortCircuited,
181-
);
182-
}
163+
if (output?.shortCircuit === true) { meta.shortCircuited = true; }
164+
return output;
183165

184-
return checkShortCircuited(output);
185166
},
186167
'name',
187168
{ __proto__: null, value: nextHookName },
@@ -440,11 +421,8 @@ class ESMLoader {
440421
);
441422
}
442423

443-
const { format, url } = this.resolve(
444-
specifier,
445-
parentURL,
446-
importAssertionsForResolve,
447-
);
424+
const { format, url } =
425+
await this.resolve(specifier, parentURL, importAssertionsForResolve);
448426

449427
let job = this.moduleMap.get(url, importAssertions.type);
450428

@@ -579,11 +557,10 @@ class ESMLoader {
579557
hookErrIdentifier: '',
580558
hookIndex: chain.length - 1,
581559
hookName: 'load',
582-
isChainAsync: true,
583560
shortCircuited: false,
584561
};
585562

586-
const validateArgs = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
563+
const validate = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
587564
if (typeof nextUrl !== 'string') {
588565
// non-strings can be coerced to a url string
589566
// validateString() throws a less-specific error
@@ -609,22 +586,19 @@ class ESMLoader {
609586

610587
validateObject(ctx, `${hookErrIdentifier} context`);
611588
};
612-
const validateOutput = (hookErrIdentifier, output) => {
613-
if (typeof output !== 'object') { // [2]
614-
throw new ERR_INVALID_RETURN_VALUE(
615-
'an object',
616-
hookErrIdentifier,
617-
output,
618-
);
619-
}
620-
};
621589

622-
const nextLoad = nextHookFactory(chain, meta, { validateArgs, validateOutput });
590+
const nextLoad = nextHookFactory(chain, meta, validate);
623591

624592
const loaded = await nextLoad(url, context);
625593
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
626594

627-
validateOutput(hookErrIdentifier, loaded);
595+
if (typeof loaded !== 'object') { // [2]
596+
throw new ERR_INVALID_RETURN_VALUE(
597+
'an object',
598+
hookErrIdentifier,
599+
loaded,
600+
);
601+
}
628602

629603
if (loaded?.shortCircuit === true) { meta.shortCircuited = true; }
630604

@@ -804,7 +778,7 @@ class ESMLoader {
804778
* statement or expression.
805779
* @returns {{ format: string, url: URL['href'] }}
806780
*/
807-
resolve(
781+
async resolve(
808782
originalSpecifier,
809783
parentURL,
810784
importAssertions = ObjectCreate(null)
@@ -828,43 +802,36 @@ class ESMLoader {
828802
hookErrIdentifier: '',
829803
hookIndex: chain.length - 1,
830804
hookName: 'resolve',
831-
isChainAsync: false,
832805
shortCircuited: false,
833806
};
807+
834808
const context = {
835809
conditions: DEFAULT_CONDITIONS,
836810
importAssertions,
837811
parentURL,
838812
};
813+
const validate = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
839814

840-
const validateArgs = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
841815
validateString(
842816
suppliedSpecifier,
843817
`${hookErrIdentifier} specifier`,
844818
); // non-strings can be coerced to a url string
845819

846820
validateObject(ctx, `${hookErrIdentifier} context`);
847821
};
848-
const validateOutput = (hookErrIdentifier, output) => {
849-
if (
850-
typeof output !== 'object' || // [2]
851-
output === null ||
852-
(output.url == null && typeof output.then === 'function')
853-
) {
854-
throw new ERR_INVALID_RETURN_VALUE(
855-
'an object',
856-
hookErrIdentifier,
857-
output,
858-
);
859-
}
860-
};
861822

862-
const nextResolve = nextHookFactory(chain, meta, { validateArgs, validateOutput });
823+
const nextResolve = nextHookFactory(chain, meta, validate);
863824

864-
const resolution = nextResolve(originalSpecifier, context);
825+
const resolution = await nextResolve(originalSpecifier, context);
865826
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
866827

867-
validateOutput(hookErrIdentifier, resolution);
828+
if (typeof resolution !== 'object') { // [2]
829+
throw new ERR_INVALID_RETURN_VALUE(
830+
'an object',
831+
hookErrIdentifier,
832+
resolution,
833+
);
834+
}
868835

869836
if (resolution?.shortCircuit === true) { meta.shortCircuited = true; }
870837

Collapse file

‎lib/internal/modules/esm/resolve.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/esm/resolve.js
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
10811081
}
10821082
}
10831083

1084-
function defaultResolve(specifier, context = {}) {
1084+
async function defaultResolve(specifier, context = {}) {
10851085
let { parentURL, conditions } = context;
10861086
if (parentURL && policy?.manifest) {
10871087
const redirects = policy.manifest.getDependencyMapper(parentURL);
@@ -1227,11 +1227,11 @@ const {
12271227

12281228
if (policy) {
12291229
const $defaultResolve = defaultResolve;
1230-
module.exports.defaultResolve = function defaultResolve(
1230+
module.exports.defaultResolve = async function defaultResolve(
12311231
specifier,
12321232
context
12331233
) {
1234-
const ret = $defaultResolve(specifier, context);
1234+
const ret = await $defaultResolve(specifier, context, $defaultResolve);
12351235
// This is a preflight check to avoid data exfiltration by query params etc.
12361236
policy.manifest.mightAllow(ret.url, () =>
12371237
new ERR_MANIFEST_DEPENDENCY_MISSING(

0 commit comments

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