Crash of ESM loader

[coderaiser]

Version

v20.20

Platform

mac os

Subsystem

No response

What steps will reproduce the bug?

I'm working on tool that collects coverage https://github.com/coderaiser/escover
It runs provided script and adds loaders using NODE_OPTIONS. Node v20.2 is crashed, node v18, node v16 works good.

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

No response

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

No response

What do you see instead?

coderaiser@localcmd:~/putout/packages/putout$ redrun test
> TERM_PROGRAM=0 TERMINAL_EMULATOR=0 escover tape 'test/*.{js,mjs}' '{bin,lib}/**/*.spec.{js,mjs}'
node:internal/modules/esm/hooks:515
        if (response.message.status === 'exit') { return; }
                     ^

TypeError: Cannot read properties of undefined (reading 'message')
    at #waitForWorker (node:internal/modules/esm/hooks:515:22)
    at HooksProxy.makeAsyncRequest (node:internal/modules/esm/hooks:525:24)
    at #getModuleJob (node:internal/modules/esm/loader:366:44)
    at CustomizedModuleLoader.getModuleJob (node:internal/modules/esm/loader:371:42)
    at CustomizedModuleLoader.import (node:internal/modules/esm/loader:265:12)
    at node:internal/modules/run_main:56:28
    at loadESM (node:internal/process/esm_loader:36:13)
    at runMainESM (node:internal/modules/run_main:53:21)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:79:5)
    at node:internal/main/run_main_module:23:47

Node.js v20.2.0

Additional information

No response

[aduh95]

Could you provide steps to reproduce? Ideally without involving any npm package or downloading code from the internet, but anything would be appreciated.

EDIT: I'm able to reproduce using out/Release/node --loader data:text/javascript,await%20Promise.reject\(function\(\){}\) test/fixtures/empty.js

[coderaiser]

Thank you! Looks like I understand what the problem is: I write the tool to load a couple loaders, and it's globalPreload returns nothing.

Now I have another thing I cannot understand. I'm working on library to mock ESM imports, and it is used as loader and works good in node v16 and node v18. I use global variables to access map and set of modules to re-import and cannot understand how to deal with node v20 loaders, since they use workers.

Ideally I want to keep mocking for a test file:

test('cat: should call readFile', async (t) => {
    const readFile = stub();
    
    mockImport('fs/promises', {
        readFile,
    });
    
    const cat = await reImport('./cat.js');
    await cat();
    
    stopAll();
    
    t.calledWith(readFile, ['./README.md', 'utf8']);
    t.end();
});

And this reImport should influence loader to reImport file and transform it.

[aduh95]

Global variables won't work, you'll need to use globalPreload to communicate between the main thread and the loader thread. We used to have a example on how to do it, unfortunately it was removed in #46402, @nodejs/loaders we should come up with a new example ASAP.

[giltayar]

Maybe this can help as an example? https://github.com/giltayar/esm-loaders-talk/tree/main/07-clear-cache/off-thread

It's a loader that implements "cleaning the cache" and has commuication between loader-api.js and loader.js.

(this is sample code and has no error handling and such)

[coderaiser]

Could you please clarify can I pass function using globalPreload protocol?

[JakobJingleheimer]

Could you please clarify can I pass function using globalPreload protocol?

globalPreload is a function (specifically, a loader hook). However, we're considering replacing it with a simpler utility (the current globalPreload is very confusing).

Not to discourage ingenuity, but have you considered testdouble? Mocking ESM is exactly what it does (very well).

[coderaiser]

globalPreload is a function (specifically, a loader hook). However, we're considering replacing it with a simpler utility (the current globalPreload is very confusing).

I mean communication using port, can I send function with help of port.postMessage and receive it in loader using port.onmessage?

Also as I understand I cannot save port as global variable, and must use Proxy to sync mocks over test file and loader. This is really over complicated, but I hope it gives performance benefits as we use loader in separate worker.

Not to discourage ingenuity, but have you considered testdouble? Mocking ESM is exactly what it does (very well).

Never herd about it. I used mock-require a lot in my code bases, also mock-import has type definitions and linter support.

[giltayar]

@coderaiser you can't send functions via the port. Only serializable stuff can be sent.

[coderaiser]

@giltayar in this case mocking functions of ESM modules starting from node v20 will be absolutely impossible? Since I need a way to create function inside test, and then loaded module should call this function.

[targos]

Both the test file and the loaded module are evaluated in the main thread. The function doesn't need to pass through the loader thread.

[coderaiser]

@targos here is how mock-import works now:

• I have test file which says to loader to mock-import;
• I tell mock-import to mock implementation of a module;
• Then I re-import module (with a suffix ?counter=i where i increased every time, to get a fresh version without cache hit), mock-import loader sees that module should be processed so it transforms the code changing imports with variable declarations const glob = global.__mockImportCache.get('./glob.js');
• I use global variables for this purpose;

How should I rewrite this logic to have ability to mock implementations of a modules?

I don't want to tell that mocking is best practice, but sometimes you don't want to run real implementation which do something to network connections, or works with file system, and cannot to re-write all your codebase.

[aduh95]

Maybe this can help as an example? https://github.com/giltayar/esm-loaders-talk/tree/main/07-clear-cache/off-thread

@giltayar what we would need is to have a mocking implementation in our test suite, having it outside the test suite is not as useful as it could break at anytime without us noticing.

[cjihrig]

FWIW I've been working on this for our test runner's mocking API.

[coderaiser]

Thank you guys, looks like I understand the thing about using port, the most confusing part was that globalPreload return value is module code, and what is before is loader code.

Anyways I don't need to pass mocking details to loader, it only need to know the name of mocked module to make transformation.

Here is how it looks like.

[coderaiser]

FWIW I've been working on this for our test runner's mocking API.

@cjihrig could you please share what API test runner will have?

[cjihrig]

@coderaiser you can see my work in progress here: https://github.com/cjihrig/node/commits/mock. Sorry, there are no docs yet.

[simlu]

The amount of hacks needed these days to maintain a testing framework is just mind boggling. Test framework design is very much an afterthought in node unfortunately... So, for the next unfortunate person stumbling onto this thread, here is another example of... some really dark magic =)

https://github.com/simlu/robo-config-plugin/blob/035c13e463d539f9ec56e9e9ad1ab2eadc67a753/test/projects/assorted/%40npm-closedsource/test/hot.js

[JakobJingleheimer]

Test framework design is very much an afterthought in node unfortunately

This is simply not true. I had a conversation literally yesterday where we specifically discussed support and implications for this. Also, multiple members of our team are themselves maintainers of testing frameworks 😉

[simlu]

@JakobJingleheimer Man, I'd love to join in on these conversations! Especially the inability to free memory / cache in ESM feels like huge regression coming from CJS and has caused me so many headaches. But maybe I just have to make some fundamental design changes coming from CJS 😬

[JakobJingleheimer]

Our team meets every other Tuesday at 20:00 CET. I'm away for the next one (01 Aug); I'm not sure if that one will happen. An issue is automatically generated in the node/loaders repo for each meeting, where we coordinate. Please jump in—we'd be happy to have you 🙂

You mentioned a memory issue—feel free to toss that onto the discussion topics!

A quick glimpse at that issue says the person is looking for access to ESM's ModuleMap. I would guess direct access is something we would likely not expose, but we may provide some kind of readonly clone/snapshot if there was need of it. If that is indeed what you need, let us know.

[GeoffreyBooth]

If you’re referring to ESM’s lack of an equivalent to require.cache, it’s a known issue and not really under our control; V8 doesn’t allow removing or replacing ES modules once they’re loaded. However there are workarounds; look into how Vite implements ESM hot module reload. The same approach, where Vite creates a wrapper around the module and then the wrapper can be used to replace its contents, might work for you.

[simlu]

Thank you for your replies here, much appreciated!

@JakobJingleheimer I've subscribed to the repo and will keep an eye out for the next meeting! What we are really looking for is an easy way to hot reload in esm, without causing memory leaks.

@GeoffreyBooth Yeah, it's an unfortunate situation. I spend way too much time already trying various workarounds and the current one seems to be the best. I've written down what we are currently doing here

[JakobJingleheimer]

it’s a known issue and not really under our control; V8 doesn’t allow removing or replacing ES modules once they’re loaded

Ah, right, yes

What we are really looking for is an easy way to hot reload in esm, without causing memory leaks.

It's been discussed a few times, and AFAIK, this is not possible without access to V8's module graph. You can get something decent together with MessageChannel via globalPreload (soon to be initialize).

[simlu]

You can get something decent together with MessageChannel via globalPreload (soon to be initialize).

Do you have an exanple of that? Or is that basically what we are already doing here?

[JakobJingleheimer]

Or is that basically what we are already doing here?

Generally yes for globalPreload / postMessage.

This article does a good job of covering this topic: https://dev.to/giltayar/mock-all-you-want-supporting-es-modules-in-the-testdouble-js-mocking-library-3gh1. He's updated testdouble to use MessageChannel, so you could glean from that.

The initialize (globalPreload replacement) has landed by the way.

p.s. Sorry for the delay—I was out of town.