Skip to content

Commit 9d8d15d

Browse files
2024-06-18 meeting notes (#202)
* 2024-06-18 meeting notes * Update doc/meetings/2024-06-18.md --------- Co-authored-by: Chengzhong Wu <[email protected]>
1 parent 56689b7 commit 9d8d15d

File tree

1 file changed

+88
-0
lines changed

1 file changed

+88
-0
lines changed

doc/meetings/2024-06-18.md

+88
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
# Node.js Loaders Team Meeting 2024-06-18
2+
3+
## Links
4+
5+
* **Recording**: https://www.youtube.com/watch?v=KhbERbApnTc
6+
* **GitHub Issue**: https://github.com/nodejs/loaders/issues/200
7+
8+
## Present
9+
10+
* James Sumners @jsumners-nr
11+
* Geoffrey Booth @GeoffreyBooth
12+
* Guy Bedford @guybedford
13+
* Joyee Cheung @joyeecheung
14+
* Jakob Smith @JacobJingleheimer
15+
* Chengzhong Wu @legendecas
16+
17+
## Agenda
18+
19+
### nodejs/loaders
20+
21+
* Hooks thread direction [#201](https://github.com/nodejs/loaders/issues/201)
22+
23+
* James: didn’t understand the motivation for moving loader to a separate thread. Same thread loader seems a lot easier for APM products.
24+
* Geoffrey: heard the same thing from @Qard for APM. The primary goal for loader thread is isolate the environment. Only by having on-thread, we can
25+
* Geoffrey: The challenge of moving the loader back to thread is ?
26+
* Joyee: APM vendors don’t want the isolate. The primary usage is patching the module. Apart from the HTTP imports, they are synchronous. The off-thread stuff is necessary to
27+
* Jakob: the esm stuff is asynchronous. Would it be ok to be synchronous?
28+
* Joyee: customization of require must be synchronous, and it would be a problem on require(esm).
29+
* Joyee: the breakage would be @aduh95’s work ??(link).
30+
* Geoffrey: the proposal is not only bring the loader back to the thread, but also making them synchronous
31+
* Joyee: the breakage would be converting all the async function to be sync functions
32+
* Geoffrey: it would be hard to migrate HTTP importing
33+
* James: bmeck mentioned that ignore the HTTP imports because no body is using them
34+
* Joyee: ESM loaders are experimental. We also have more stable things in CJS. For those people, they can reuse the sync code. Maybe there are people doing things that are not possible in sync code. The only thing is network import mentioned by mcollina
35+
* Jakob: trying to come up with a list that may take a lot of time
36+
* Joyee: for downstream user, your app is probably not doing anything meaningful until your app is properly loaded. An exception is dynamic import.
37+
* Guy: bmeck's old design doc: [Loaders Design Document](https://docs.google.com/document/d/1J0zDFkwxojLXc36t2gcv1gZ-QnoTXSzK1O6mNAMlync/edit#heading=h.xzp5p5pt8hlq)
38+
* Guy: it's about the motivation about poisoning the loader. Agreed about the concerns, we might need something about a network thread similar to the browsers in node.js but it is noisy in messaging. If anyone is still interested in the original security statement, ??.
39+
* Guy: https://github.com/nodejs/node/pull/43245
40+
* Guy: `preImport` hook would be the only async hook and other hooks could be sync and mimic a network thread.
41+
* Guy: I completely happy with making the hooks synchronous with a way to implement things like network thread.
42+
* Geoffrey: my impression of advantage of preImport is that ?.
43+
* Geoffrey: it is less hacky and avoids blocking the process.
44+
* Joyee: Any package using this model?
45+
* Guy: the browsers
46+
* Joyee: any nodejs package counting this to work? Being able to do this async
47+
* Geoffrey: current package doing something async would not be easy to migrate. Say we add `preImport` hook, we can try to migrate them and see how hard it is. Or matteo’s idea, deasync-ify anything
48+
* Geoffrey: don’t know if it is realistic to make loader stable on Node.js v23 if we make any significant changes. If the work doesn’t make it in October, it will take another year.
49+
* Joyee: need to think about the loader hook and the link hook
50+
* Guy: TLA modules customization makes it no longer sync
51+
* Jakob: how would it be for preImport to be async and resolve and load be sync. What happens if preImport is not done
52+
* Guy: preImport blocks. Not doing any module graph linking until the promise of preImport finishes. Works with dynamic import because dynamic import is async. It wouldn’t work for require(esm) either. The same constraints apply for CJS that it can not require(network).
53+
* Geoffrey: it doesn’t work with TLA, the same restriction. Can not work with require(TLA)
54+
* Geoffrey: based on Joyee’s design doc, make a new doc describe resole and load migrate to on-thread, synchronizing, describe what happens to register, and land it and implement it on main before october.
55+
* Geoffrey: preImport hook as a follow-up.
56+
* Jakob: design doc is the way to go.
57+
* Joyee: might be too much on my doc?
58+
* Joyee: another top priority is deprecating CJS monkey patching.
59+
* Geoffrey: wouldn’t the described way be close to deprecating CJS monkey patching?
60+
* Guy: unification between CJS hook and ESM hook to be found by having more design work?
61+
* Joyee: not wise just to breaking existing API, can not be backported. One package detecting features to work with multiple packages. Stretching out the adoption timing.
62+
* Joyee: in a non-breaking way, they can be backported and make the adoption easier.
63+
* Jakob: for feature detection, could they just check if the loader is on the thread?
64+
* James: How can a loader know it is in a thread?
65+
* Joyee: you can have a loader on an application worker thread.
66+
* Geoffrey: not on a main thread, doesn’t mean you are on a loader thread
67+
* Guy: the current register model is really great, you
68+
* Guy: Joyee’s model is not obvious how a worker is attached. Automatically get registered loaders work for worker threads would be great
69+
* Joyee: whether sharing source code or instances. I think people are strongly against having shared instances across threads. Nice to have a shared configuration, rather than shread instance. Someone from sentry: is there a way to configure loader, instead of loading a package.
70+
* Joyee: API seems for instance. configuration for source codes.
71+
* Guy: beneficial to have a worker inherit configured loaders
72+
* Joyee: most intuitive behavior people in issues, suppose you don’t want to share?
73+
* Geoffrey: running the same code for worker threads and main threads. Sharing transpiled codes?
74+
* Geoffrey: an option bag to `register` to make it backport-able, allowing user to set if the loader would be passed down to a new worker thread, opt-in to the new approach. And get rid of the off-thread in a semver-major.
75+
* Guy: agree with Geoffrey.
76+
* Joyee: less confusing could be having a different name, allowing a loader to adapt to different Node.js version.
77+
* Geoffrey: I’ll take the lead to draft the doc. Create (three) lists of packages that uses the loader API, TLA, ??
78+
* Jakob: should we tweet on node.js account?
79+
* Geoffrey: maybe after we have the design doc.
80+
81+
82+
* Proposal for a simple, universal module loader hooks API to replace require() monkey-patching [#52219](https://github.com/nodejs/node/issues/52219)
83+
* doc: add synchronous hooks proposal [#198](https://github.com/nodejs/loaders/pull/198)
84+
85+
* depends on decisions above
86+
87+
88+

0 commit comments

Comments
 (0)