This repository has been archived by the owner on Sep 2, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 43
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add meeting notes for 2020-01-15 (#470)
Add meeting notes for 2020-01-15
- Loading branch information
Showing
1 changed file
with
135 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| # Node.js Foundation Modules Team Meeting 2020-01-15 | ||
|
|
||
| ## Links | ||
|
|
||
| * **Recording**: https://www.youtube.com/watch?v=SuFC_yzc51A | ||
| * **GitHub Issue**: https://github.com/nodejs/modules/issues/466 | ||
| * **Minutes Google Doc**: https://docs.google.com/document/d/1odxHwuFIuZsbaQ9SgH23riGiwvKg0y2pHlTwdaIOIbE/edit | ||
|
|
||
| ## Present | ||
|
|
||
| * Jan Krems (@jkrems) | ||
| * Jordan Harband (@ljharb) | ||
| * Myles Borins (@MylesBorins) | ||
| * Guy Bedford (@guybedford) _GB_ | ||
| * Geoffrey Booth (@GeoffreyBooth) _GFB_ | ||
| * Wesley Wigham (@weswigham) | ||
| * Corey Farrell (@coreyfarrell) | ||
| * Evan Plaice (@evanplaice) | ||
|
|
||
| ## Agenda | ||
|
|
||
| ## Announcements | ||
|
|
||
| *Extracted from **modules-agenda** labelled issues and pull requests from the **nodejs org** prior to the meeting. | ||
|
|
||
| ### nodejs/node | ||
|
|
||
| #### WIP: Move ESM loaders to worker thread [#31229](https://github.com/nodejs/node/pull/31229) | ||
|
|
||
| * MB: Anything important to add? Not time sensitive to discuss. | ||
| * CF: Did we decide on this direction? If so, should we prepare / remove dynamicInstantiate? | ||
| * MB: My understanding: Was always meant to move to worker threads. If there’s things not compatible with that, we should act on it. | ||
| * WW: Not at odds with having synchronous module load, if anything it helps. | ||
|
|
||
| #### esm: implement import.meta.resolve [#31032](https://github.com/nodejs/node/pull/31032) | ||
|
|
||
| * GB: Last was people were open to shipping it flagged. | ||
| * MB: Chatted with Domenic about this. Brought up `import(await resolve(specifier))` as a potential concern. Import maps are 1:1, maps URL on the left to URL on the right. As specified it’s not recursive, I think it should be. Adds complexities around cycles. Need to resolve those concerns. Overall Domenic was interested in node going forward with this, possibly start a spec in WICG. No concerns about a flagged implementation landing, can act as reference implementation. | ||
| * GB: Second argument allows customizing parent, async. Any API concerns? | ||
| * JK: Some concerns about how this relates to loaders and idempotency vs. isolation. | ||
| * MB: Objections to moving remaining discussion to Github? | ||
| * *(No objections)* | ||
|
|
||
| #### module: unflag conditional exports [#31001](https://github.com/nodejs/node/pull/31001) | ||
|
|
||
| * GB: Discussed conditional ordering in the past. Right now on 13 exports are shipped. Objects always pick the default key. | ||
| * MB: Want to make sure we can unflag conditional exports. | ||
| * WW: Voiced concerns in the past. Encourage shipping two copies in packages duplicates dependency trees. Bad transition path during migrations. We can do require(esm) independently of this change. | ||
| * MB: Dual specifier hazard definitely still less than ideal. | ||
| * WW: Not looking forward to people accidentally including react twice and using a mix. | ||
| * JH: This assumes bad setup in react that doesn’t take conditional exports into account? | ||
| * WW: Applies to any project that deals with current ESM tooling (rollp, webpack). | ||
| * JH: Projects will have to adjust their approach now that native ESM is a thing. | ||
| * MB: Same kind of dual instancing can appear without this feature. Also, all of this is still experimental. If require(esm) lands in the future, this could relieve the need for using this pattern. Many reports of issues with this, we could still remove the behavior even though very painful. | ||
| * WW: Node compat usually means after unflag nothing changes. | ||
| * MB: While experimental, both workers and http/2 support had dramatic changes after unflagging. | ||
| * WW: Resolver and module system have stronger ecosystem ties. | ||
| * MB: Does anybody object to unflagging? | ||
| * *(No objection)* | ||
| * MB: We can still make adjustments before April. This includes changes to the resolver. | ||
| * WW: How closely are we working with npm, e.g. around their plans for peer deps? | ||
| * MB: We have a channel with them. | ||
| * JH: Also interested in discussing it? | ||
| * WW: Skipping to charter - are we custodians of code in core? Or also including ecosystem questions? | ||
| * MB: Let’s punt for chartering discussion. There’s a team focussed on package maintenance. Scope could be as large as we want it to be, as long as TSC signs off on it. | ||
|
|
||
| ### nodejs/modules | ||
|
|
||
| #### unflagging in 12.x LTS [#450](https://github.com/nodejs/modules/issues/450) | ||
|
|
||
| * MB: We can drop from agenda if people don’t see problems. Targos got V8 7.8 against node 12 tested and landed. There’s now a [PR for next 12.x minor](https://github.com/nodejs/node/pull/31368). Got module loader on there over the weekend. Primary difference is internals, external behavior should be the same. Exception is unflagging. Everything (exports, resolve self) behind --experimental-modules. People should test that it doesn’t do new-modules-stuff without the flag, does all the things with the flag. We can update implementation until April, then flip the flag. | ||
| * JH: If we add new features, e.g. sigil for self-reference. If it lands on 13, can it be backported? | ||
| * MB: Yes, unless they depend on V8 features. Synthetic modules in V8 were primary reason that backporting of features was hard. We can make changes, including breaking, for flagged implementation in patch releases. What we have in the roadmap, should be backportable. And we really want to backport asap to de-risk April unflagging. | ||
| * JH: Is backporting `exports` for CommonJS to 10 something we want? | ||
| * MB: Likely not, would take a lot of resources. | ||
| * JH: Even if showed up with a magical PR? | ||
| * MB: If it introduces instability, may be better to not risk it. | ||
| * JH: Package maintainers hesitant to adopt exports if they want to support 10.x until EOL. People need to use rollup etc to hide internal files if they want to get encapsulation. | ||
| * MB: Good point, April maintenance makes it awkward to actually get it in. | ||
| * WW: Could a polyfill be used? | ||
| * JH: Already looking into polyfill, alternative for getting it backported. | ||
| * MB: Gut feeling is changing resolver this late into | ||
| * CF: Worked on `import()` fixes, are they going to be in 12.x backport? | ||
| * MB: Will verify. | ||
| * GB: Is loader work part of a backport? | ||
| * MB: Will run branchdiff, everything not labelled to prevent backport should have made it as long as they have made it into a 13.x release. | ||
| * JK: Will it be okay to change loader flag name during unflag in April? | ||
| * MB: Should be fine. Need to make sure that we leave silent aliases for flags we remove. We did this for `--experimental-specifier-resolution`. | ||
|
|
||
| *Some time later.* | ||
|
|
||
| * MB: Follow-up, branch diff: https://gist.github.com/MylesBorins/2157cd56e9cf17068c4d77d539edb6f2 | ||
| * GB: Last few PRs are landing. | ||
| * MB: List only includes 13.x commits, expected. | ||
|
|
||
| #### ERR_REQUIRE_ESM LTS backport [#468](https://github.com/nodejs/modules/issues/468) | ||
|
|
||
| * GB: In the CJS resolver, we have all new behavior flagged. We wanted to make `require(.mjs)` or `require(.js)` in `type:module` both error. Should we make these warnings? Only time when this comes up is when an early adopter is mixing file types. Might be worth to accept bug reports coming out of this. | ||
| * MB: Any action item? | ||
| * GB: Originally thought it was breaking a valid use case. But can’t repro now. | ||
| * WW: They opted into an experimental feature if they run into this error. | ||
|
|
||
| #### Chartering the Modules team [#412](https://github.com/nodejs/modules/issues/412) | ||
|
|
||
| * MB: 8 minutes are not a lot of time. Future meeting should maybe be 30 minutes of just discussing this? | ||
| * GBB: Yes, may be good after some of the more urgent items are sorted out. | ||
| * JK: Should we make the proposal more concrete before the discussion? Move conversation about scope on Github? | ||
| * MB: Can somebody pair on a high-level items to be done for chartering? | ||
| * JK: Volunteered. | ||
| * JH: Volunteered. | ||
| * GBB: Volunteered. | ||
| * MB: Will send out Doodle with times that work on calendars I can see (Jan & myself). | ||
|
|
||
| #### Table source type detection [#465](https://github.com/nodejs/modules/pull/465) | ||
|
|
||
| * GBB: Seems to reflect status quo? | ||
| * MB: May still want API to tell package type. | ||
| * GBB: Different API, this is about source type-based detection. | ||
| * GBB: This and conditional exports are last items before retiring the roadmap. Can more to other channels. | ||
| * MB: Any objections to landing this roadmap change? | ||
| * (No objections) | ||
| * MB: There’s a list of things we want to keep working on (package type). We should track those somewhere. | ||
| * GBB: Also includes feature gaps in loader hooks. | ||
|
|
||
| #### Stabilizing the resolver [#451](https://github.com/nodejs/modules/issues/451) | ||
|
|
||
| *Tabled.* | ||
|
|
||
| #### Loader Hooks [#351](https://github.com/nodejs/modules/issues/351) | ||
|
|
||
| *Tabled.* | ||
|
|
||
| #### Patch/Instrument a module [#339](https://github.com/nodejs/modules/issues/339) | ||
|
|
||
| *Tabled.* |