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

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

Closed
mhdawson opened this issue Sep 6, 2024 · 6 comments
Closed

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

mhdawson opened this issue Sep 6, 2024 · 6 comments
Assignees

Comments

@mhdawson
Copy link
Member

mhdawson commented Sep 6, 2024

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

  • ESM Hook deadlocks since 21.2.0 a3e09b3 #50948

nodejs/loaders

  • Proposal: Moving hooks on thread #205
  • Hooks thread direction #201

Invited

  • Loaders team: @nodejs/loaders

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

@mhdawson mhdawson self-assigned this Sep 6, 2024
@joyeecheung
Copy link
Member

Removed nodejs/node#53787 and #198, PRs are being reviewed on GitHub. Also I am traveling in a different timezone and can't attend meetings this late.

@JakobJingleheimer
Copy link
Contributor

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
Copy link
Member

joyeecheung commented Sep 9, 2024

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
Copy link
Contributor

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

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

@JakobJingleheimer
Copy link
Contributor

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

@joyeecheung
Copy link
Member

joyeecheung commented Sep 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants