If we create a new file we might need to add it to the startup snapshot (see #45849). Alternatively is there a way to stick this in an existing file?

The reason I put this in its own file is so that the url property actually led to a valid file that represented the base "loader".

It can live in literally any file that can be treated as a loader (exports load, resolve, etc.) This code is the relevant block:

const defaultLoader = 'internal/modules/esm/default_loader';
this.addCustomLoader(`node:${defaultLoader}`, require(defaultLoader));

So that if we ever expose a list loaders or anything like that the url property is what you see above.

Let’s wait to land this after #49144.

Works for me 🎉

Can't use for…of :( gotta use a plain ol' for

💯 😢 😆

Why remove these? If you need them to start out empty, you can still leave the fields (resolve etc) and then also the code docs

So that the built-in hooks are not treated as any kind of "special" object; they now work just like any other loader – you can see them being added in the constructor. This ensures that if we attach any data to a loader instance that the default node loader gets it too – right now it's just url.

Ah, yes. I figured that :) I think you can still leave everything else here and just initialise the lists as empty 🙂

Ah yes true 😄 I can do that. Part of me considered that hard-coding the hook names in a bunch of places might be less clean… but it's more clear and that works for me 🎉

Did anything change here? It looks like it's now just less readable

Yes. The previous structure of chain:

type Fn = (arg0: any, context: any, next: Fn) => any;

interface ChainItem {
  fn: Fn
  url: string;
}

type Chain = ChainItem[];

The new structure of chain:

type Fn = (arg0: any, context: any, next: Fn) => any;

interface LoaderInstance {
  load?: Fn;
  resolve?: Fn;
  initialize?: Fn;
  url: string;
}

interface ChainItem {
  fn: Fn
  loader: LoaderInstance;
}

type Chain = ChainItem[];

Gotta use primordials for these sorts of things ArrayPrototypePush.

Derp thanks 😄

This is a performance loss. I think it would be better for there to be only rebuildChains and for it to accept a list of chain names to rebuild, like

#rebuildChains(...names) {
  if (!names.length) {
    names = new Array('initialize','load','resolve'); // [1]
  }

  const shouldRebuild = { __proto__: null };

  for (let n = names.length - 1; n > -1; n--) {
    const name = names[n];
    shouldRebuild[name]: true;
    this.#chains[name].length = 0; // [2] Might need a primordial?
  }

  for (
    let i = 0,
        l = this.#loaderInstances.length - 1;
    i < l;
    i++
  ) {
    const {
      initialize,
      load,
      resolve,
    } = this.#loaderInstances[i];

    if (shouldRebuild.initialize) { /* … */ }
    if (shouldRebuild.load) { /* … */ }
    if (shouldRebuild.resolve) { /* … */ }
  }
}

this.#rebuildChains('resolve'); // Rebuild only `resolve` chain
this.#rebuildChains('load','resolve'); // Rebuild `load` and `resolve`
this.#rebuildChains(); // Rebuild all chains

[1] I believe this (new Array(…) instead of […]) will invoke an optimisation in V8 to choose the most appropriate specialised C++ array from the start and also to pre-allocation only exactly the space for these 3 items. If it doesn't, no need for the otherwise uncommon use of new Array() instead of [].

[2] We don't need to re-create the array, only empty it. For that, setting its length to 0 is far better.

I think we can make this happen 😄 Probably a bit of a µ-optimization considering how often #rebuildChains is called and the size of N… but it's not a problem to make it a little bit faster. If the actual chain execution was affected I would be more concerned, considering that is called on every import/require.

Mm, it likely won't break the performance bank. But it's easy to avoid the de-op, so why not 🙂