Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESM --loader cause duplicate app load #49082

Closed
mamaniv opened this issue Aug 9, 2023 · 4 comments
Closed

ESM --loader cause duplicate app load #49082

mamaniv opened this issue Aug 9, 2023 · 4 comments
Labels
loaders Issues and PRs related to ES module loaders

Comments

@mamaniv
Copy link

mamaniv commented Aug 9, 2023

Version

v20.4.0

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

No response

What steps will reproduce the bug?

We use --loader to manipulate ESM sourcecode before loading and when using node v20.x, it cause duplicate application loads.

Please take a look at the following code, which is a minimal reproducer for the issue.

Files:

// esm-app.mjs
console.log('APP START')
setTimeout(() => {
    // stay alive
}, 1000000);
// esm-loader.mjs
console.log('LOADER');
export async function load(importStr, context, nextLoad) {
    setImmediate(async () => {
        const moduleInstance = await import(importStr);
    });
    return nextLoad(importStr);
}

Now let's compare the runs for node 20 vs node 18:

PS C:\Dev\Webapps\loader-test> node --version
v18.13.0
PS C:\Dev\Webapps\loader-test> node --loader ./esm-loader.mjs esm-app.mjs
(node:17060) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
LOADER
APP START
C:\Dev\Webapps\loader-test> node --version
v20.4.0
PS C:\Dev\Webapps\loader-test> node --loader ./esm-loader.mjs esm-app.mjs
(node:52952) ExperimentalWarning: Custom ESM Loaders is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
LOADER
APP START
APP START

Note that we do import again any loaded module during the esm-loader.load callback, but we excepted to get a cached module, like we get for any other import call. As you can see, that's how it worked in previous node versions and with the node.js example, we ran the esm-app.mjs code only once.

Is it a bug in node 20 or is there something we can do to prevent this duplicate import?

Thanks!

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

Always

What is the expected behavior? Why is that the expected behavior?

See above runs

What do you see instead?

See above runs

Additional information

No response

@Flarna Flarna added the loaders Issues and PRs related to ES module loaders label Aug 10, 2023
@aduh95
Copy link
Contributor

aduh95 commented Aug 10, 2023

Is it a bug in node 20 or is there something we can do to prevent this duplicate import?

It's not a bug, it's a feature. As of 20.0.0, loaders are now executed on a separate thread, meaning importing module and setting globals no longer affects the main application thread. By doing import(importStr) in the load hook, you are loading that module a second time on the loader thread.

Maybe you could share a bit more on what you are trying to achieve so we can discuss how to support your use-case?

@mamaniv
Copy link
Author

mamaniv commented Aug 13, 2023

Our application is a kind of a profiler and we need the ability to get all the ES Modules right after the loading.
With commonjs, we use the Module._load function to get all the loaded module and we're trying the do the same with ES modules.
With the previous node versions, we could achieve this goal using --loader and then to manually read the file and instrument it, but then with node v20 we got this issue.

If you have a better solution, maybe a more elegant way to register to ES module load event, it will be much easier for us.

Thanks!

@hbarshak
Copy link

seems to be related or even duplicate to #47880

@aduh95
Copy link
Contributor

aduh95 commented Dec 6, 2023

You can use the load hook to register load events, it even has limited support for CJS now which means you might even forego the monkey patching of the CJS loader.

I'm going to close this now if that's OK.

@aduh95 aduh95 closed this as completed Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

No branches or pull requests

4 participants