Skip to content

Commit

Permalink
esm: allow resolve to return import assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
GeoffreyBooth committed Jan 10, 2023
1 parent e8db11c commit 349d054
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 43 deletions.
37 changes: 21 additions & 16 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -754,8 +754,9 @@ changes:
* `specifier` {string}
* `context` {Object}
* `conditions` {string\[]} Export conditions of the relevant `package.json`
* `importAssertions` {Object}
* `parentURL` {string|undefined} The module importing this one, or undefined
* `importAssertions` {Object} The object after the `assert` in an `import`
statement, or the value of the `assert` property in the second argument of
an `import()` expression; or an empty object
if this is the Node.js entry point
* `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the
Node.js default `resolve` hook after the last user-supplied `resolve` hook
Expand All @@ -765,23 +766,26 @@ changes:
* `format` {string|null|undefined} A hint to the load hook (it might be
ignored)
`'builtin' | 'commonjs' | 'json' | 'module' | 'wasm'`
* `importAssertions` {Object|undefined} The import assertions to use when
caching the module (optional; if excluded the input will be used)
* `shortCircuit` {undefined|boolean} A signal that this hook intends to
terminate the chain of `resolve` hooks. **Default:** `false`
* `url` {string} The absolute URL to which this input resolves
The `resolve` hook chain is responsible for resolving file URL for a given
module specifier and parent URL, and optionally its format (such as `'module'`)
as a hint to the `load` hook. If a format is specified, the `load` hook is
ultimately responsible for providing the final `format` value (and it is free to
ignore the hint provided by `resolve`); if `resolve` provides a `format`, a
custom `load` hook is required even if only to pass the value to the Node.js
default `load` hook.
The module specifier is the string in an `import` statement or
`import()` expression.
The parent URL is the URL of the module that imported this one, or `undefined`
if this is the main entry point for the application.
The `resolve` hook chain is responsible for telling Node.js where to find and
how to cache a given `import` statement or expression. It can optionally return
its format (such as `'module'`) as a hint to the `load` hook. If a format is
specified, the `load` hook is ultimately responsible for providing the final
`format` value (and it is free to ignore the hint provided by `resolve`); if
`resolve` provides a `format`, a custom `load` hook is required even if only to
pass the value to the Node.js default `load` hook.
Import type assertions are part of the cache key for saving loaded modules into
the Node.js internal module cache. The `resolve` hook is responsible for
returning an `importAssertions` object if the module should be cached with
different assertions than were present in the source code (for example, if no
assertions were present but the module should be cached with assertions
`{ type: 'json' }`).
The `conditions` property in `context` is an array of conditions for
[package exports conditions][Conditional Exports] that apply to this resolution
Expand Down Expand Up @@ -857,7 +861,8 @@ changes:
* `format` {string}
* `shortCircuit` {undefined|boolean} A signal that this hook intends to
terminate the chain of `resolve` hooks. **Default:** `false`
* `source` {string|ArrayBuffer|TypedArray} The source for Node.js to evaluate
* `source` {string|ArrayBuffer|TypedArray} The source for Node.js to
evaluate
The `load` hook provides a way to define a custom method of determining how
a URL should be interpreted, retrieved, and parsed. It is also in charge of
Expand Down
40 changes: 28 additions & 12 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ class Hooks {
} = pluckHooks(exports);

if (globalPreload) {
this.hasCustomLoadHooks = true;
ArrayPrototypePush(
this.#hooks.globalPreload,
{
Expand All @@ -125,6 +126,7 @@ class Hooks {
);
}
if (resolve) {
this.hasCustomLoadHooks = true;
ArrayPrototypePush(
this.#hooks.resolve,
{
Expand Down Expand Up @@ -318,21 +320,10 @@ class Hooks {

const {
format,
importAssertions: resolvedImportAssertions,
url,
} = resolution;

if (
format != null &&
typeof format !== 'string' // [2]
) {
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'a string',
hookErrIdentifier,
'format',
format,
);
}

if (typeof url !== 'string') {
// non-strings can be coerced to a URL string
// validateString() throws a less-specific error
Expand All @@ -359,9 +350,34 @@ class Hooks {
}
}

if (
resolvedImportAssertions != null &&
typeof resolvedImportAssertions !== 'object'
) {
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'an object',
hookErrIdentifier,
'importAssertions',
resolvedImportAssertions,
);
}

if (
format != null &&
typeof format !== 'string' // [2]
) {
throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
'a string',
hookErrIdentifier,
'format',
format,
);
}

return {
__proto__: null,
format,
importAssertions: resolvedImportAssertions,
url,
};
}
Expand Down
12 changes: 7 additions & 5 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ class ESMLoader {
* module import.
* @returns {Promise<ModuleJob>} The (possibly pending) module job
*/
async getModuleJob(specifier, parentURL, importAssertions) {
async getModuleJob(specifier, parentURL, importAssertions = { __proto__: null }) {
let importAssertionsForResolve;

// We can skip cloning if there are no user-provided loaders because
Expand All @@ -167,18 +167,19 @@ class ESMLoader {
};
}

const { format, url } =
await this.resolve(specifier, parentURL, importAssertionsForResolve);
const resolveResult = await this.resolve(specifier, parentURL, importAssertionsForResolve);
const { url, format } = resolveResult;
const resolvedImportAssertions = resolveResult.importAssertions ?? importAssertions;

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

// CommonJS will set functions for lazy job evaluation.
if (typeof job === 'function') {
this.moduleMap.set(url, undefined, job = job());
}

if (job === undefined) {
job = this.#createModuleJob(url, importAssertions, parentURL, format);
job = this.#createModuleJob(url, resolvedImportAssertions, parentURL, format);
}

return job;
Expand Down Expand Up @@ -223,6 +224,7 @@ class ESMLoader {
if (process.env.WATCH_REPORT_DEPENDENCIES && process.send) {
process.send({ 'watch:import': [url] });
}

const ModuleJob = require('internal/modules/esm/module_job');
const job = new ModuleJob(
this,
Expand Down
26 changes: 16 additions & 10 deletions test/fixtures/es-module-loaders/assertionless-json-import.mjs
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
const DATA_URL_PATTERN = /^data:application\/json(?:[^,]*?)(;base64)?,([\s\S]*)$/;
const JSON_URL_PATTERN = /\.json(\?[^#]*)?(#.*)?$/;
const JSON_URL_PATTERN = /^[^?]+\.json(\?[^#]*)?(#.*)?$/;

export async function resolve(specifier, context, next) {
const noAssertionSpecified = context.importAssertions.type == null;

export function resolve(url, context, next) {
// Mutation from resolve hook should be discarded.
context.importAssertions.type = 'whatever';
return next(url);
}

export function load(url, context, next) {
if (context.importAssertions.type == null &&
(DATA_URL_PATTERN.test(url) || JSON_URL_PATTERN.test(url))) {
const { importAssertions } = context;
importAssertions.type = 'json';
// This fixture assumes that no other resolve hooks in the chain will error on invalid import assertions
// (as defaultResolve doesn't).
const result = await next(specifier, context);

if (noAssertionSpecified &&
(DATA_URL_PATTERN.test(result.url) || JSON_URL_PATTERN.test(result.url))) {
result.importAssertions = {
...(result.importAssertions ?? context.importAssertions),
type: 'json',
};
}
return next(url);

return result;
}

0 comments on commit 349d054

Please sign in to comment.