Skip to content

Commit

Permalink
esm: allow resolve to return import assertions
Browse files Browse the repository at this point in the history
PR-URL: #46153
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
  • Loading branch information
GeoffreyBooth authored and targos committed Nov 10, 2023
1 parent 5ffc90a commit 45de8d1
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 43 deletions.
30 changes: 16 additions & 14 deletions doc/api/esm.md
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,8 @@ changes:
* `specifier` {string}
* `context` {Object}
* `conditions` {string\[]} Export conditions of the relevant `package.json`
* `importAssertions` {Object}
* `importAssertions` {Object} An object whose key-value pairs represent the
assertions for the module to import
* `parentURL` {string|undefined} The module importing this one, or undefined
if this is the Node.js entry point
* `nextResolve` {Function} The subsequent `resolve` hook in the chain, or the
Expand All @@ -762,23 +763,24 @@ 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 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.
The `conditions` property in `context` is an array of conditions for
[package exports conditions][Conditional Exports] that apply to this resolution
Expand Down
43 changes: 29 additions & 14 deletions lib/internal/modules/esm/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class Hooks {
};

// Enable an optimization in ESMLoader.getModuleJob
hasCustomLoadHooks = false;
hasCustomResolveOrLoadHooks = false;

// Cache URLs we've already validated to avoid repeated validation
#validatedUrls = new SafeSet();
Expand Down Expand Up @@ -125,6 +125,7 @@ class Hooks {
);
}
if (resolve) {
this.hasCustomResolveOrLoadHooks = true;
ArrayPrototypePush(
this.#hooks.resolve,
{
Expand All @@ -134,7 +135,7 @@ class Hooks {
);
}
if (load) {
this.hasCustomLoadHooks = true;
this.hasCustomResolveOrLoadHooks = true;
ArrayPrototypePush(
this.#hooks.load,
{
Expand Down Expand Up @@ -317,21 +318,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 @@ -358,9 +348,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
14 changes: 9 additions & 5 deletions lib/internal/modules/esm/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,25 +171,28 @@ class ESMLoader {

// We can skip cloning if there are no user-provided loaders because
// the Node.js default resolve hook does not use import assertions.
if (this.#hooks?.hasCustomLoadHooks) {
if (this.#hooks?.hasCustomResolveOrLoadHooks) {
// This method of cloning only works so long as import assertions cannot contain objects as values,
// which they currently cannot per spec.
importAssertionsForResolve = {
__proto__: null,
...importAssertions,
};
}

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 @@ -234,6 +237,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
27 changes: 17 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,24 @@
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))) {
// Clean new import assertions object to ensure that this test isn't passing due to mutation.
result.importAssertions = {
...(result.importAssertions ?? context.importAssertions),
type: 'json',
};
}
return next(url);

return result;
}

0 comments on commit 45de8d1

Please sign in to comment.