Dynamic import in loader hooks crashes node

[sebamarynissen]

Version

16.12.0

Platform

Microsoft Windows NT 10.0.19041.0 x64

Subsystem

esm

What steps will reproduce the bug?

I am currently doing some experiments with loader hooks (see also www.npmjs.com/package/create-esm-loader and https://www.npmjs.com/package/node-esm-loader) and noticed something that was working in 16.11, but not anymore in 16.12.0 (which coincides with the changes in the loader api).

The setup is the following:

// # loader.js
const promise = import('./dep.js');

export async function resolve(specifier, ctx, defaultResolve) {
  await promise;
  return defaultResolve(specifier, ctx, defaultResolve);
}

// # dep.js
import './nested.js';

// # nested.js
console.log('Nested dep');

If you try to run a file using node --experimental-loader="./loader.js" file.js, it will crash the Node process with an exit code 13 on 16.12, whereas it runs fine on 16.11.

How often does it reproduce? Is there a required condition?

The bug can only be reproduced if you import a nested dependency nested.js.

What is the expected behavior?

The Node process should not crash, as it did in 16.11.

What do you see instead?

Node crashes.

Additional information

The problem is likely related to the fact the the nested dependency is loaded asynchronously and that the resolve() hook is interfering with loading it. Indeed, if you do

import './dep.js';
const promise = import('./dep.js');

export async function resolve(specifier, ctx, defaultResolve) {
  await promise;
  return defaultResolve(specifier, ctx, defaultResolve);
}

the Node process does not crash because it finished loading the nested dependency before trying to use the loader hooks.

[aduh95]

I don't think we can work around this, the same way using dynamic imports in loader hooks results in an infinite recursion, if to resolve './dep.js', you first need to wait for ./dep.js to be loaded, you end up in a dead lock. In any case, that'd be interesting to document this behavior.
@nodejs/loaders

[sebamarynissen]

I understand if this is something that can't be fixed, it's a rather exotic use case anyway. After discovering this, I was actually surprised that it even worked in <16.12. If you would ever need this kind of functionality, I guess it can be solved by TLA (which is what I went with eventually in create-esm-loader:

const config = await makeSureEverythingIsLoaded();

export async function resolve(specifier, ctx, defaultResolve) {
  return doSomethingWith(config, specifier, ctx, defaultResolve);
}

[bmeck]

This is a bug with it not actually waiting on the dynamic import. It should be able to be fixed if someone finds time to look into it.