Proposal: preImport loader hook

A preImport hook can take the place of the async use cases for having an async resolver hook by allowing any async work to be done upfront before triggering the further pipeline steps. For example, loading an import map could be an async operation prior to synchronously using that import map in resolution (which is exactly what the browser does).

Ultimately, the goal would be that by covering these needs, this should allow for a synchronous core resolver which would enable unification with browser resolution.

The initial hook PR would not deprecate the resolver yet, and could be released to get feedback on this first, before following up with a subsequent async resolver deprecation.

There is a basic draft API in nodejs/node#43245, with the following documentation:

preImport(specifier, context)
* `specifier` {string}
* `context` {Object}
  * `conditions` {string\[]} Resolution conditions of the current environment,
    as defined for the `package.json` imports and exports fields
  * `dynamic` {boolean} Whether this import is a dynamic `import()`
  * `importAssertions` {Object}
  * `parentURL` {string|undefined} The module importing this one, or undefined
    if this is the Node.js entry point

The preImport hook allows for tracking and asynchronous setup work for every
top-level import operation.
The preImport hook is called for each top-level import operation by the
module loader, both for the host-called imports (ie for the main entry) and for
dynamic import() imports. These are distinguished by the dynamic context.
All preImport hooks for all loaders are run asynchronously in parallel,
and block any further load operations (ie resolve and load) for the module graph
being imported until they all complete successfully.
Multiple import calls to the same import specifier will re-call the hook
multiple times. The first error thrown by the preImport hooks will be directly
returned to the specific import operation as the load failure.

export async function preImport (specifier, context) {
  if (context.topLevel)
    console.log(`Top-level load of ${specifier}`);
  else
    console.log(`Dynamic import of ${specifier} in ${context.parentURL}`);
}

This is an interesting idea! I have a couple questions:

All preImport hooks for all loaders are run asynchronously in parallel,
and block any further load operations (ie resolve and load) for the module graph
being imported until they all complete successfully.

Is this effectively a Promise.all() or Promise.race()? Also, why? I'm a little confused by why they should run in parallel.
In your example, you have if (context.topLevel), but topLevel is not in context currently or in your proposed additions. What does it mean? I presume top-level await, but that's what dynamic intends to identify, no?

I'm a little concerned by the pre prefixes: I already find globalPreload confusing (it's effectively a “setup” step). I think if we introduce this new hook, we should re-visit the globalPreload name (especially because it does not behave like the other hooks).

P.S. Sorry for the delayed reply; I'm only just seeing this now.

Is this effectively a Promise.all() or Promise.race()? Also, why? I'm a little confused by why they should run in parallel.

It's a Promise.all, the reason being that is the most performant design when we don't want to unnecessarily serialize to slow down imports. If two loaders both want to do fs / network / other async operations when a new import is initiated, there's no reason they need to block eachother.

In your example, you have if (context.topLevel), but topLevel is not in context currently or in your proposed additions. What does it mean? I presume top-level await, but that's what dynamic intends to identify, no?

Ahh thanks for spotting the issue in the example, that should be context.dynamic yes. I've gone back and fourth on the wording to get something clear. A top-level import is basically an import which will run the ECMA-262 top-level Evaluation function - https://tc39.es/ecma262/#sec-moduleevaluation. Not all modules go through a whole graph execution operation (eg most dependencies). topLevel is perhaps more descriptive than dynamic, but both likely need clarification. I have a better definition in the current readme update I'll add to the PR now.

I'm a little concerned by the pre prefixes: I already find globalPreload confusing (it's effectively a “setup” step). I think if we introduce this new hook, we should re-visit the globalPreload name (especially because it does not behave like the other hooks).

My preference would have been to just call it the import hook, but export function import() isn't supported in ECMA-262 unfortunately. I'm open to alternative names, eg beforeImport or importHook or prepareImport etc.

Yes, both globalPreload and preImport are effectively non-chainable hooks which can run without blocking other hooks. They fall under a different parallel hook model. In the loader documentation it could be worth eg explicitly documenting the "hook type" of each hook something like Hook Type: Sync Chained or Hook Type: Async Parallel, and there may also be other variants in future.

Closing in favor of #89

Navigation Menu

Repository navigation

Proposal: preImport loader hook #83

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Proposal: preImport loader hook #83

Description

Activity

JakobJingleheimer commented on Jun 19, 2022

guybedford commented on Jun 19, 2022

GeoffreyBooth commented on Jul 1, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions