nodejs / loaders

ECMAScript Modules Loaders
MIT License
128 stars 17 forks source link

Node.js Loaders Team Meeting 2024-09-10 #226

Closed mhdawson closed 1 month ago

mhdawson commented 1 month ago

Time

UTC Tue 10-Sep-2024 18:00 (06:00 PM):

Timezone Date/Time
US / Pacific Tue 10-Sep-2024 11:00 (11:00 AM)
US / Mountain Tue 10-Sep-2024 12:00 (12:00 PM)
US / Central Tue 10-Sep-2024 13:00 (01:00 PM)
US / Eastern Tue 10-Sep-2024 14:00 (02:00 PM)
EU / Western Tue 10-Sep-2024 19:00 (07:00 PM)
EU / Central Tue 10-Sep-2024 20:00 (08:00 PM)
EU / Eastern Tue 10-Sep-2024 21:00 (09:00 PM)
Moscow Tue 10-Sep-2024 21:00 (09:00 PM)
Chennai Tue 10-Sep-2024 23:30 (11:30 PM)
Hangzhou Wed 11-Sep-2024 02:00 (02:00 AM)
Tokyo Wed 11-Sep-2024 03:00 (03:00 AM)
Sydney Wed 11-Sep-2024 04:00 (04:00 AM)

Or in your local time:

Links

Agenda

Extracted from loaders-agenda labelled issues and pull requests from the nodejs org prior to the meeting.

nodejs/node

nodejs/loaders

Invited

Observers/Guests

Notes

The agenda comes from issues labelled with loaders-agenda across all of the repositories in the nodejs org. Please label any additional issues that should be on the agenda before the meeting starts.

Joining the meeting

joyeecheung commented 1 month ago

Removed https://github.com/nodejs/node/issues/53787 and https://github.com/nodejs/loaders/pull/198, PRs are being reviewed on GitHub. Also I am traveling in a different timezone and can't attend meetings this late.

JakobJingleheimer commented 1 month ago

Let's skip this one then if both you and Geoffrey won't be there.

Last I recall, making hooks sync turned out to be a no-go because it would preclude plugins for eslint, vite, etc, (which are async) closing the door on a huge amount of functionality.

joyeecheung commented 1 month ago

Are you talking about adding new synchronous hooks or replacing asynchronous hooks with synchronous hooks? I think there's only been active work on the former.

Even on the latter I think we have came to the conclusion that users could work around it by just spinning their own workers to run async code and Atomics.wait(). That's what module.register() already does internally to hook into require() and import.meta.resolve() anyway, the code could've been written in user land JS and don't have to be in core. If needed you could even polyfill the asynchronous API in user land on top of a synchronous hooks API by copy pasting most of the internal loader worker code out of core.

Existing packages like @babel/register have already been running async code in a worker with a synchronous monkey-patched CJS hook on the main thread without the help of the asynchronous hooks too. I think the built-in asynchronous hooks are just helpers that come with their own curse (users not taking control of the loader worker makes it deadlock-prone, mutation on the main thread is very limited since you can't serialize functions), but don't really open any doors feature-wise. If moved in-thread, then it won't be effective for require or import.meta.resolve(), which means users would still need another synchronous in-thread API to fill that gap.

JakobJingleheimer commented 1 month ago

Wasn't there discussion about unifying cjs and esm hooks and making the esm ones sync?

https://github.com/nodejs/node/pull/54769 (bit of a daisy-chain, but this appears to lead to all that)

JakobJingleheimer commented 1 month ago

No-one has indicated they will attend, so cancelling this meeting instance.

joyeecheung commented 1 month ago

nodejs/node#54769 (bit of a daisy-chain, but this appears to lead to all that)

No, that just refactors the already existing off-thread synchronous path, so that we can reuse it in-thread synchronous hooks too. There is already an off-thread hook path for module.register() that's internally synchronous for require() and import.meta.resolve(), if you read some of changes there, that just adds "...and future synchronous hooks" to the comments. And when we add in-thread synchronous hooks, users can even polyfill module.register() with it by just copying the hooks loader code out of core, since that's just written in JS that could be ported to user land. So by then module.register() will just be a helper that could've been polyfilled by users and it's another topic whether it should be kept around.