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

esm: restore `next<HookName>`'s `context` as optional arg · nodejs/node@953fefe · GitHub

/ node Public

Commit 953fefe

Browse files
JakobJingleheimer
authored and
targos
committed
esm: restore next<HookName>'s context as optional arg
PR-URL: #43553 Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 2251242 commit 953fefe

6 files changed

Lines changed: 107 additions & 38 deletions

File tree

‎doc/api/esm.md‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -815,7 +815,7 @@ export async function resolve(specifier, context, nextResolve) {
815815

816816
// Defer to the next hook in the chain, which would be the
817817
// Node.js default resolve if this is the last user-specified loader.
818-
return nextResolve(specifier, context);
818+
return nextResolve(specifier);
819819
}
820820
```
821821
@@ -910,7 +910,7 @@ export async function load(url, context, nextLoad) {
910910
}
911911

912912
// Defer to the next hook in the chain.
913-
return nextLoad(url, context);
913+
return nextLoad(url);
914914
}
915915
```
916916
@@ -1026,7 +1026,7 @@ export function resolve(specifier, context, nextResolve) {
10261026
}
10271027
10281028
// Let Node.js handle all other specifiers.
1029-
return nextResolve(specifier, context);
1029+
return nextResolve(specifier);
10301030
}
10311031
10321032
export function load(url, context, nextLoad) {
@@ -1049,7 +1049,7 @@ export function load(url, context, nextLoad) {
10491049
}
10501050
10511051
// Let Node.js handle all other URLs.
1052-
return nextLoad(url, context);
1052+
return nextLoad(url);
10531053
}
10541054
```
10551055

@@ -1102,7 +1102,7 @@ export async function resolve(specifier, context, nextResolve) {
11021102
}
11031103
11041104
// Let Node.js handle all other specifiers.
1105-
return nextResolve(specifier, context);
1105+
return nextResolve(specifier);
11061106
}
11071107
11081108
export async function load(url, context, nextLoad) {
@@ -1143,7 +1143,7 @@ export async function load(url, context, nextLoad) {
11431143
}
11441144
11451145
// Let Node.js handle all other URLs.
1146-
return nextLoad(url, context);
1146+
return nextLoad(url);
11471147
}
11481148
11491149
async function getPackageType(url) {

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

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ let emittedSpecifierResolutionWarning = false;
114114
* validation within MUST throw.
115115
* @returns {function next<HookName>(...hookArgs)} The next hook in the chain.
116116
*/
117-
function nextHookFactory(chain, meta, validate) {
117+
function nextHookFactory(chain, meta, { validateArgs, validateOutput }) {
118118
// First, prepare the current
119119
const { hookName } = meta;
120120
const {
@@ -137,7 +137,7 @@ function nextHookFactory(chain, meta, validate) {
137137
// factory generates the next link in the chain.
138138
meta.hookIndex--;
139139

140-
nextNextHook = nextHookFactory(chain, meta, validate);
140+
nextNextHook = nextHookFactory(chain, meta, { validateArgs, validateOutput });
141141
} else {
142142
// eslint-disable-next-line func-name-matching
143143
nextNextHook = function chainAdvancedTooFar() {
@@ -152,14 +152,28 @@ function nextHookFactory(chain, meta, validate) {
152152
// Update only when hook is invoked to avoid fingering the wrong filePath
153153
meta.hookErrIdentifier = `${hookFilePath} '${hookName}'`;
154154

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

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

162+
// `context` is an optional argument that only needs to be passed when changed
163+
switch (args.length) {
164+
case 1: // It was omitted, so supply the cached value
165+
ArrayPrototypePush(args, meta.context);
166+
break;
167+
case 2: // Overrides were supplied, so update cached value
168+
ObjectAssign(meta.context, args[1]);
169+
break;
170+
}
171+
160172
ArrayPrototypePush(args, nextNextHook);
161173
const output = await ReflectApply(hook, undefined, args);
162174

175+
validateOutput(outputErrIdentifier, output);
176+
163177
if (output?.shortCircuit === true) { meta.shortCircuited = true; }
164178
return output;
165179

@@ -554,13 +568,14 @@ class ESMLoader {
554568
const chain = this.#loaders;
555569
const meta = {
556570
chainFinished: null,
571+
context,
557572
hookErrIdentifier: '',
558573
hookIndex: chain.length - 1,
559574
hookName: 'load',
560575
shortCircuited: false,
561576
};
562577

563-
const validate = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
578+
const validateArgs = (hookErrIdentifier, { 0: nextUrl, 1: ctx }) => {
564579
if (typeof nextUrl !== 'string') {
565580
// non-strings can be coerced to a url string
566581
// validateString() throws a less-specific error
@@ -584,21 +599,24 @@ class ESMLoader {
584599
}
585600
}
586601

587-
validateObject(ctx, `${hookErrIdentifier} context`);
602+
if (ctx) validateObject(ctx, `${hookErrIdentifier} context`);
603+
};
604+
const validateOutput = (hookErrIdentifier, output) => {
605+
if (typeof output !== 'object' || output === null) { // [2]
606+
throw new ERR_INVALID_RETURN_VALUE(
607+
'an object',
608+
hookErrIdentifier,
609+
output,
610+
);
611+
}
588612
};
589613

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

592616
const loaded = await nextLoad(url, context);
593617
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
594618

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

603621
if (loaded?.shortCircuit === true) { meta.shortCircuited = true; }
604622

@@ -797,41 +815,44 @@ class ESMLoader {
797815
);
798816
}
799817
const chain = this.#resolvers;
818+
const context = {
819+
conditions: DEFAULT_CONDITIONS,
820+
importAssertions,
821+
parentURL,
822+
};
800823
const meta = {
801824
chainFinished: null,
825+
context,
802826
hookErrIdentifier: '',
803827
hookIndex: chain.length - 1,
804828
hookName: 'resolve',
805829
shortCircuited: false,
806830
};
807831

808-
const context = {
809-
conditions: DEFAULT_CONDITIONS,
810-
importAssertions,
811-
parentURL,
812-
};
813-
const validate = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
814-
832+
const validateArgs = (hookErrIdentifier, { 0: suppliedSpecifier, 1: ctx }) => {
815833
validateString(
816834
suppliedSpecifier,
817835
`${hookErrIdentifier} specifier`,
818836
); // non-strings can be coerced to a url string
819837

820-
validateObject(ctx, `${hookErrIdentifier} context`);
838+
if (ctx) validateObject(ctx, `${hookErrIdentifier} context`);
839+
};
840+
const validateOutput = (hookErrIdentifier, output) => {
841+
if (typeof output !== 'object' || output === null) { // [2]
842+
throw new ERR_INVALID_RETURN_VALUE(
843+
'an object',
844+
hookErrIdentifier,
845+
output,
846+
);
847+
}
821848
};
822849

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

825852
const resolution = await nextResolve(originalSpecifier, context);
826853
const { hookErrIdentifier } = meta; // Retrieve the value after all settled
827854

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

836857
if (resolution?.shortCircuit === true) { meta.shortCircuited = true; }
837858

‎test/es-module/test-esm-loader-chaining.mjs‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ const commonArgs = [
229229
assert.strictEqual(status, 0);
230230
}
231231

232-
{ // Verify chain does break and throws appropriately
232+
{ // Verify resolve chain does break and throws appropriately
233233
const { status, stderr, stdout } = spawnSync(
234234
process.execPath,
235235
[
@@ -273,7 +273,7 @@ const commonArgs = [
273273
assert.strictEqual(status, 1);
274274
}
275275

276-
{ // Verify chain does break and throws appropriately
276+
{ // Verify load chain does break and throws appropriately
277277
const { status, stderr, stdout } = spawnSync(
278278
process.execPath,
279279
[
@@ -314,6 +314,27 @@ const commonArgs = [
314314
assert.match(stderr, /'resolve' hook's nextResolve\(\) specifier/);
315315
}
316316

317+
{ // Verify error thrown when resolve hook is invalid
318+
const { status, stderr } = spawnSync(
319+
process.execPath,
320+
[
321+
'--loader',
322+
fixtures.fileURL('es-module-loaders', 'loader-resolve-passthru.mjs'),
323+
'--loader',
324+
fixtures.fileURL('es-module-loaders', 'loader-resolve-null-return.mjs'),
325+
...commonArgs,
326+
],
327+
{ encoding: 'utf8' },
328+
);
329+
330+
assert.strictEqual(status, 1);
331+
assert.match(stderr, /ERR_INVALID_RETURN_VALUE/);
332+
assert.match(stderr, /loader-resolve-null-return\.mjs/);
333+
assert.match(stderr, /'resolve' hook's nextResolve\(\)/);
334+
assert.match(stderr, /an object/);
335+
assert.match(stderr, /got null/);
336+
}
337+
317338
{ // Verify error thrown when invalid `context` argument passed to `nextResolve`
318339
const { status, stderr } = spawnSync(
319340
process.execPath,
@@ -333,6 +354,27 @@ const commonArgs = [
333354
assert.strictEqual(status, 1);
334355
}
335356

357+
{ // Verify error thrown when load hook is invalid
358+
const { status, stderr } = spawnSync(
359+
process.execPath,
360+
[
361+
'--loader',
362+
fixtures.fileURL('es-module-loaders', 'loader-load-passthru.mjs'),
363+
'--loader',
364+
fixtures.fileURL('es-module-loaders', 'loader-load-null-return.mjs'),
365+
...commonArgs,
366+
],
367+
{ encoding: 'utf8' },
368+
);
369+
370+
assert.strictEqual(status, 1);
371+
assert.match(stderr, /ERR_INVALID_RETURN_VALUE/);
372+
assert.match(stderr, /loader-load-null-return\.mjs/);
373+
assert.match(stderr, /'load' hook's nextLoad\(\)/);
374+
assert.match(stderr, /an object/);
375+
assert.match(stderr, /got null/);
376+
}
377+
336378
{ // Verify error thrown when invalid `url` argument passed to `nextLoad`
337379
const { status, stderr } = spawnSync(
338380
process.execPath,

‎test/fixtures/es-module-loaders/loader-load-null-return.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export async function load(specifier, context, next) {
2+
return null;
3+
}

‎test/fixtures/es-module-loaders/loader-resolve-42.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@ export async function resolve(specifier, context, next) {
22
console.log('resolve 42'); // This log is deliberate
33
console.log('next<HookName>:', next.name); // This log is deliberate
44

5-
return next('file:///42.mjs', context);
5+
return next('file:///42.mjs');
66
}

‎test/fixtures/es-module-loaders/loader-resolve-null-return.mjs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export async function resolve(specifier, context, next) {
2+
return null;
3+
}

0 commit comments

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