FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

esm: fix chain advances when loader calls next<HookName> multiple times · nodejs/node@10bcad5 · GitHub

/ node Public

Commit 10bcad5

Browse files
JakobJingleheimer
authored and
targos
committed
esm: fix chain advances when loader calls next<HookName> multiple times
PR-URL: #43303 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent 50d64ed commit 10bcad5

6 files changed

Lines changed: 198 additions & 109 deletions

File tree

‎lib/internal/errors.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1372,7 +1372,7 @@ E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe', Error);
13721372
E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks', Error);
13731373
E(
13741374
'ERR_LOADER_CHAIN_INCOMPLETE',
1375-
'The "%s" hook from %s did not call the next hook in its chain and did not' +
1375+
'"%s" did not call the next hook in its chain and did not' +
13761376
' explicitly signal a short circuit. If this is intentional, include' +
13771377
' `shortCircuit: true` in the hook\'s return.',
13781378
Error

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

Lines changed: 115 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,22 @@ const {
1212
FunctionPrototypeCall,
1313
ObjectAssign,
1414
ObjectCreate,
15+
ObjectDefineProperty,
1516
ObjectSetPrototypeOf,
1617
PromiseAll,
18+
ReflectApply,
1719
RegExpPrototypeExec,
1820
SafeArrayIterator,
1921
SafeWeakMap,
22+
StringPrototypeSlice,
23+
StringPrototypeToUpperCase,
2024
globalThis,
2125
} = primordials;
2226
const { MessageChannel } = require('internal/worker/io');
2327

2428
const {
2529
ERR_LOADER_CHAIN_INCOMPLETE,
30+
ERR_INTERNAL_ASSERTION,
2631
ERR_INVALID_ARG_TYPE,
2732
ERR_INVALID_ARG_VALUE,
2833
ERR_INVALID_RETURN_PROPERTY_VALUE,
@@ -89,6 +94,81 @@ const { getOptionValue } = require('internal/options');
8994

9095
let emittedSpecifierResolutionWarning = false;
9196

97+
/**
98+
* A utility function to iterate through a hook chain, track advancement in the
99+
* chain, and generate and supply the `next<HookName>` argument to the custom
100+
* hook.
101+
* @param {KeyedHook[]} chain The whole hook chain.
102+
* @param {object} meta Properties that change as the current hook advances
103+
* along the chain.
104+
* @param {boolean} meta.chainFinished Whether the end of the chain has been
105+
* reached AND invoked.
106+
* @param {string} meta.hookErrIdentifier A user-facing identifier to help
107+
* pinpoint where an error occurred. Ex "file:///foo.mjs 'resolve'".
108+
* @param {number} meta.hookIndex A non-negative integer tracking the current
109+
* position in the hook chain.
110+
* @param {string} meta.hookName The kind of hook the chain is (ex 'resolve')
111+
* @param {boolean} meta.shortCircuited Whether a hook signaled a short-circuit.
112+
* @param {(hookErrIdentifier, hookArgs) => void} validate A wrapper function
113+
* containing all validation of a custom loader hook's intermediary output. Any
114+
* validation within MUST throw.
115+
* @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
116+
*/
117+
function nextHookFactory(chain, meta, validate) {
118+
// First, prepare the current
119+
const { hookName } = meta;
120+
const {
121+
fn: hook,
122+
url: hookFilePath,
123+
} = chain[meta.hookIndex];
124+
125+
// ex 'nextResolve'
126+
const nextHookName = `next${
127+
StringPrototypeToUpperCase(hookName[0]) +
128+
StringPrototypeSlice(hookName, 1)
129+
}`;
130+
131+
// When hookIndex is 0, it's reached the default, which does not call next()
132+
// so feed it a noop that blows up if called, so the problem is obvious.
133+
const generatedHookIndex = meta.hookIndex;
134+
let nextNextHook;
135+
if (meta.hookIndex > 0) {
136+
// Now, prepare the next: decrement the pointer so the next call to the
137+
// factory generates the next link in the chain.
138+
meta.hookIndex--;
139+
140+
nextNextHook = nextHookFactory(chain, meta, validate);
141+
} else {
142+
// eslint-disable-next-line func-name-matching
143+
nextNextHook = function chainAdvancedTooFar() {
144+
throw new ERR_INTERNAL_ASSERTION(
145+
`ESM custom loader '${hookName}' advanced beyond the end of the chain.`
146+
);
147+
};
148+
}
149+
150+
return ObjectDefineProperty(
151+
async (...args) => {
152+
// Update only when hook is invoked to avoid fingering the wrong filePath
153+
meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;
154+
155+
validate(`${meta.hookErrIdentifier} hook's ${nextHookName}()`, args);
156+
157+
// Set when next<HookName> is actually called, not just generated.
158+
if (generatedHookIndex === 0) { meta.chainFinished = true; }
159+
160+
ArrayPrototypePush(args, nextNextHook);
161+
const output = await ReflectApply(hook, undefined, args);
162+
163+
if (output?.shortCircuit === true) { meta.shortCircuited = true; }
164+
return output;
165+
166+
},
167+
'name',
168+
{ __proto__: null, value: nextHookName },
169+
);
170+
}
171+
92172
/**
93173
* An ESMLoader instance is used as the main entry point for loading ES modules.
94174
* Currently, this is a singleton -- there is only one used for loading
@@ -471,32 +551,21 @@ class ESMLoader {
471551
* @returns {{ format: ModuleFormat, source: ModuleSource }}
472552
*/
473553
async load(url, context = {}) {
474-
const loaders = this.#loaders;
475-
let hookIndex = loaders.length - 1;
476-
let {
477-
fn: loader,
478-
url: loaderFilePath,
479-
} = loaders[hookIndex];
480-
let chainFinished = hookIndex === 0;
481-
let shortCircuited = false;
482-
483-
const nextLoad = async (nextUrl, ctx = context) => {
484-
--hookIndex; // `nextLoad` has been called, so decrement our pointer.
485-
486-
({
487-
fn: loader,
488-
url: loaderFilePath,
489-
} = loaders[hookIndex]);
490-
491-
if (hookIndex === 0) { chainFinished = true; }
492-
493-
const hookErrIdentifier = `${loaderFilePath} "load"`;
554+
const chain = this.#loaders;
555+
const meta = {
556+
chainFinished: null,
557+
hookErrIdentifier: '',
558+
hookIndex: chain.length - 1,
559+
hookName: 'load',
560+
shortCircuited: false,
561+
};
494562

563+
const validate = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
495564
if (typeof nextUrl !== 'string') {
496565
// non-strings can be coerced to a url string
497566
// validateString() throws a less-specific error
498567
throw new ERR_INVALID_ARG_TYPE(
499-
`${hookErrIdentifier} nextLoad(url)`,
568+
`${hookErrIdentifier} url`,
500569
'a url string',
501570
nextUrl,
502571
);
@@ -508,29 +577,20 @@ class ESMLoader {
508577
new URL(nextUrl);
509578
} catch {
510579
throw new ERR_INVALID_ARG_VALUE(
511-
`${hookErrIdentifier} nextLoad(url)`,
580+
`${hookErrIdentifier} url`,
512581
nextUrl,
513582
'should be a url string',
514583
);
515584
}
516585
}
517586

518-
validateObject(ctx, `${hookErrIdentifier} nextLoad(, context)`);
519-
520-
const output = await loader(nextUrl, ctx, nextLoad);
521-
522-
if (output?.shortCircuit === true) { shortCircuited = true; }
523-
524-
return output;
587+
validateObject(ctx, `${hookErrIdentifier} context`);
525588
};
526589

527-
const loaded = await loader(
528-
url,
529-
context,
530-
nextLoad,
531-
);
590+
const nextLoad = nextHookFactory(chain, meta, validate);
532591

533-
const hookErrIdentifier = `${loaderFilePath} load`;
592+
const loaded = await nextLoad(url, context);
593+
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
534594

535595
if (typeof loaded !== 'object') { // [2]
536596
throw new ERR_INVALID_RETURN_VALUE(
@@ -540,10 +600,10 @@ class ESMLoader {
540600
);
541601
}
542602

543-
if (loaded?.shortCircuit === true) { shortCircuited = true; }
603+
if (loaded?.shortCircuit === true) { meta.shortCircuited = true; }
544604

545-
if (!chainFinished && !shortCircuited) {
546-
throw new ERR_LOADER_CHAIN_INCOMPLETE('load', loaderFilePath);
605+
if (!meta.chainFinished && !meta.shortCircuited) {
606+
throw new ERR_LOADER_CHAIN_INCOMPLETE(hookErrIdentifier);
547607
}
548608

549609
const {
@@ -736,55 +796,34 @@ class ESMLoader {
736796
parentURL,
737797
);
738798
}
739-
const resolvers = this.#resolvers;
740-
741-
let hookIndex = resolvers.length - 1;
742-
let {
743-
fn: resolver,
744-
url: resolverFilePath,
745-
} = resolvers[hookIndex];
746-
let chainFinished = hookIndex === 0;
747-
let shortCircuited = false;
799+
const chain = this.#resolvers;
800+
const meta = {
801+
chainFinished: null,
802+
hookErrIdentifier: '',
803+
hookIndex: chain.length - 1,
804+
hookName: 'resolve',
805+
shortCircuited: false,
806+
};
748807

749808
const context = {
750809
conditions: DEFAULT_CONDITIONS,
751810
importAssertions,
752811
parentURL,
753812
};
754-
755-
const nextResolve = async (suppliedSpecifier, ctx = context) => {
756-
--hookIndex; // `nextResolve` has been called, so decrement our pointer.
757-
758-
({
759-
fn: resolver,
760-
url: resolverFilePath,
761-
} = resolvers[hookIndex]);
762-
763-
if (hookIndex === 0) { chainFinished = true; }
764-
765-
const hookErrIdentifier = `${resolverFilePath} "resolve"`;
813+
const validate = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
766814

767815
validateString(
768816
suppliedSpecifier,
769-
`${hookErrIdentifier} nextResolve(specifier)`,
817+
`${hookErrIdentifier} specifier`,
770818
); // non-strings can be coerced to a url string
771819

772-
validateObject(ctx, `${hookErrIdentifier} nextResolve(, context)`);
773-
774-
const output = await resolver(suppliedSpecifier, ctx, nextResolve);
775-
776-
if (output?.shortCircuit === true) { shortCircuited = true; }
777-
778-
return output;
820+
validateObject(ctx, `${hookErrIdentifier} context`);
779821
};
780822

781-
const resolution = await resolver(
782-
originalSpecifier,
783-
context,
784-
nextResolve,
785-
);
823+
const nextResolve = nextHookFactory(chain, meta, validate);
786824

787-
const hookErrIdentifier = `${resolverFilePath} resolve`;
825+
const resolution = await nextResolve(originalSpecifier, context);
826+
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
788827

789828
if (typeof resolution !== 'object') { // [2]
790829
throw new ERR_INVALID_RETURN_VALUE(
@@ -794,10 +833,10 @@ class ESMLoader {
794833
);
795834
}
796835

797-
if (resolution?.shortCircuit === true) { shortCircuited = true; }
836+
if (resolution?.shortCircuit === true) { meta.shortCircuited = true; }
798837

799-
if (!chainFinished && !shortCircuited) {
800-
throw new ERR_LOADER_CHAIN_INCOMPLETE('resolve', resolverFilePath);
838+
if (!meta.chainFinished && !meta.shortCircuited) {
839+
throw new ERR_LOADER_CHAIN_INCOMPLETE(hookErrIdentifier);
801840
}
802841

803842
const {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1081,7 +1081,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
10811081
}
10821082
}
10831083

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

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL