From 85305772225c61300faf1f42107a62b479e8a0b0 Mon Sep 17 00:00:00 2001 From: Wei Zhu Date: Sat, 12 Oct 2024 21:11:18 +1030 Subject: [PATCH] esm: fix inconsistency of importAssertion between resolve and load hook As the [documentation](https://nodejs.org/docs/latest/api/module.html#customization-hooks:~:text=The%20property%20context.importAssertions%20is%20replaced%20with%20context.importAttributes.%20Using%20the%20old%20name%20is%20still%20supported%20and%20will%20emit%20an%20experimental%20warning.) says, the `context.importAssertion` should be still supported and emit a warning. This is true for the load hook, but not correct for the context of resolve hook. This PR is tring to fix the inconsistency. --- lib/internal/modules/esm/hooks.js | 2 +- test/es-module/test-esm-import-assertion-warning.mjs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/internal/modules/esm/hooks.js b/lib/internal/modules/esm/hooks.js index 5984c707f16c4f..4571922ed5a0e9 100644 --- a/lib/internal/modules/esm/hooks.js +++ b/lib/internal/modules/esm/hooks.js @@ -237,7 +237,7 @@ class Hooks { const nextResolve = nextHookFactory(chain[chain.length - 1], meta, { validateArgs, validateOutput }); - const resolution = await nextResolve(originalSpecifier, context); + const resolution = await nextResolve(originalSpecifier, defineImportAssertionAlias(context)); const { hookErrIdentifier } = meta; // Retrieve the value after all settled validateOutput(hookErrIdentifier, resolution); diff --git a/test/es-module/test-esm-import-assertion-warning.mjs b/test/es-module/test-esm-import-assertion-warning.mjs index a11b5164cebffc..0cdb21c7ee838b 100644 --- a/test/es-module/test-esm-import-assertion-warning.mjs +++ b/test/es-module/test-esm-import-assertion-warning.mjs @@ -7,6 +7,11 @@ await Promise.all([ `data:text/javascript,export ${encodeURIComponent(function resolve() { return { shortCircuit: true, url: 'data:application/json,1', importAssertions: { type: 'json' } }; })}`, + // Using importAssertions on the context object of the resolve hook should warn but still work. + `data:text/javascript,export ${encodeURIComponent(function resolve(s, c, n) { + const type = c.importAssertions.type; + return { shortCircuit: true, url: 'data:application/json,1', importAttributes: { type: 'json' } }; + })}`, // Setting importAssertions on the context object of the load hook should warn but still work. `data:text/javascript,export ${encodeURIComponent(function load(u, c, n) { c.importAssertions = { type: 'json' }; @@ -22,9 +27,9 @@ await Promise.all([ '--eval', ` import assert from 'node:assert'; import { register } from 'node:module'; - + register(${JSON.stringify(loaderURL)}); - + assert.deepStrictEqual( { ...await import('data:') }, { default: 1 }