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 44
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #392 from nodejs/missing-notes
doc: add missing sept notes
- Loading branch information
Showing
2 changed files
with
203 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,96 @@ | ||
| # Node.js Foundation Modules Team Meeting 2019-09-11 | ||
|
|
||
| ## Links | ||
|
|
||
| * **Recording**: https://www.youtube.com/watch?v=YyuP1hAdWOk | ||
| * **GitHub Issue**: https://github.com/nodejs/modules/issues/383 | ||
| * **Minutes Google Doc**: https://docs.google.com/document/d/1pvFPml9LWjaCvCs6wBFitC_Hm3EorLrqOoP9FmZ2rV8 | ||
|
|
||
| ## Present | ||
|
|
||
| * Modules team: @nodejs/modules | ||
| * Wesley Wigham (@weswigham) | ||
| * Jan Krems (@jkrems) | ||
| * Jordan Harband (@ljharb) | ||
| * Rob Palmer (@robpalme) | ||
| * Darcy Clarke (@darcyclarke) | ||
| * Geoffrey Booth | ||
| * Guy Bedford | ||
| * Bradley Farias | ||
| * Hassan Sani (@inidaname) | ||
|
|
||
| ## Agenda | ||
|
|
||
| ## Announcements | ||
|
|
||
| *Extracted from **modules-agenda** labelled issues and pull requests from the **nodejs org** prior to the meeting. | ||
|
|
||
| ### nodejs/modules | ||
|
|
||
| * Update Phase 3 per current status [#382](https://github.com/nodejs/modules/pull/382) | ||
|
|
||
| GB: Status update, not altering any designs. | ||
| JH: WRT LTS would unflagging of things for CJS like @ as a root be able to be pushed in a minor or would it require a major. | ||
| GBD: It is expected to not land CJS features after LTS. | ||
| JH: Even if ESM is not unflagged for 12 LTS, I would like to ensure exports and package-root for CJS get through before 12 LTS. | ||
| JK: Having/expecting more discussion around CJS to unflag features before ESM it should be done to help alleviate concerns about breaking changes. CJS features are altering of behavior. | ||
| GB: Ok to land? | ||
| -- no objections -- | ||
| SM: these features could tie our hands because people start using them, and it limits the design space when we do go for unflagging ESM. It could be either helpful or hurtful. | ||
| JH: We want to tie our hands, this means the features in CJS ensure they are in ESM. | ||
|
|
||
| * Divergent specifier hazard [#371](https://github.com/nodejs/modules/issues/371) | ||
|
|
||
| GBD: Added a PR about loading ESM under .js causing errors is now on core. https://github.com/nodejs/node/pull/29492 | ||
| GB: A different PR with updated info on how to publish a package with both CJS and ESM sources. https://github.com/nodejs/node/pull/29497 "." main PR is also coming and amend that PR. | ||
| GBD: We should get a few more reviews of those PRs. Would these PRs resolve this issue? | ||
| JH: PR answers the question, even if it is not ergonomic. This underscores one of the use cases that is not being met / we lack a migration strategy. This seems to be an unpleasant thing that hinders adoption. I have serious concerns if we unflag without an ergonomic solution to the use case of shipping a single package that is both CJS and ESM, it may be better not to ship ESM. | ||
| GB: Encouraging people to use the "module" field provides a standardized place to document entry point for the module | ||
| JH: We are not standardizing that field. It is structured; but if node is not using it, we should not recommend it. This may poison the package.json ecosystem in the same way that altering builtin prototypes has done in the past. | ||
| JK: Agree partially with JH, migration strategy is not perfect/ergonomic. Trade off was made with if a package can force dependents to use a certain form of your package [CJS or ESM]. Discussion around if implicitly switching is dangerous or not. We have been erring on the side of avoiding breaking usage at the cost of ergonomics. This will be an issue with migration. I don't see an alternative without risks of accidental breakage. I would prefer to be conservative over ergonomics. | ||
| JH: Some large and popular packages do fall into the singleton problem being discussed. People will switch to ESM in a non-major version and this will break people. Packages maintaining singletons are aware of their problem and can use peerDependencies and/or manage their current solutions. It seems a safer approach to allow for the interop and not the current direction of things. | ||
| GBD: There are many ways of approaching this problem. JS community has experience with solving problems in creative ways. Approaching this hazard in a sensible way is good, but userland will also likely find ways to solve this. This leads into the next topic. | ||
| JK: Not just singletons, but also relying on Babel/TS interop are affected. An ESM package relying on CJS package moved to ESM could also blow up due to the difference in interop. People doing things in de-facto minor versions is more of a concern than the singleton hazard. | ||
| JH: How is that different from creating a second package/entry point? | ||
| JK: Their dependent would need to explicitly opt-in to the different version. Could alter the default, in which case it would still be implicit. | ||
| JH: As a package author adding something to package.json to declare multiple formats is sensible, but I do not want package consumers to need to know and/or opt-in to a specific format. | ||
|
|
||
| * Revisit dot-main in exports field [#368](https://github.com/nodejs/modules/issues/368) | ||
|
|
||
| GBD: Dual environment use case and all features of "exports" are available to the "main" entry point. PR up on node core https://github.com/nodejs/node/pull/29494 . Also allows a string rather than an object form. Affects both CJS and ESM, singleton package effectively with only 1 exported module. | ||
| SM: Is this always going to be a "bare" package? import(pkgName) vs import(pkgName/sub). Does this prevent searching nested packages? | ||
| JK: Only look at a single "exports" entry during a resolve, won't look at nested packages. import(foo/node_modules/bar) would only look through the top package.json in foo | ||
| SM: Does this search up for the package? | ||
| JK: It preparses the base name and doesn't search upward. Only looks at /((@scope)/)?name/ it doesn't allow arbitrary depth. | ||
|
|
||
| * Loader Hooks [#351](https://github.com/nodejs/modules/issues/351) | ||
|
|
||
| BF: we split the hooks, altered some things for nested instrumentation, and need to add more success criteria. | ||
| GB: what is the status / who could give a way of building our own hooks. | ||
| BF: we don't have an implementation, this is design work. Our last implementation is from last July. I don't have hours to work on this. | ||
| GB: wasn't someone working on this? | ||
| JK / AA: ?? could start work on this ?? | ||
| SM: perhaps we could recreate the new API by reimplementing it in the old hooks? | ||
| AA: We have a more modern multi-loader implementation | ||
| SM: Does this need any C++ work? | ||
| AA: Will take offline. | ||
| GBD: since loaders have become monumental, could we progressively land incremental loader evolution? What happens if we expect breaking changes for LTS? | ||
| BF: There is a history due to http2 shipping a breaking change even after being unflagged. Go through LTS WG -> TSC . Ping myles | ||
| GBD: we could upgrade our existing API to help ease forward compatibility. | ||
| SM: could change the existing API a different name to keep backwards compatible if we want to promise interop. I have concerns about having a true backwards compat story with the current APIs. | ||
| JK: It is worthwhile to make breaking changes as quick as possible for backward/forwards compat. Will reduce the pain either way. We should try and have something for LTS even if it called `--experimental-loader`. | ||
| GBD: Don't think we need to call it `experimental` | ||
| JK: As long as we mark things as experimental, the actual consumers of the existing API could be helped even with a breaking change due to only having a handful of usages in the wild currently. | ||
| GBD: It would be good to ensure that any breaking change we ship still allows writing a loader that runs in the older node versions / can ship a loader than works in the new API and the old API. | ||
|
|
||
| * Package Exports [#341](https://github.com/nodejs/modules/issues/341) | ||
|
|
||
| SKIPPED | ||
|
|
||
| * Proposal: Support loading package by own "name" [#306](https://github.com/nodejs/modules/issues/306) | ||
|
|
||
| JK: Discussion about options/specifiers. Compromise of using the `@` seems to lack outstanding usage / objections. Can think about it as the "non-scope". Last blocker for "exports", useful for some applications seeking to test their "exports". Could be used as a "require from root". Can move from Do Not Merge to a real PR. Right now only has CJS implementation and needs tests/docs. | ||
|
|
||
| -- Conclusion: Move forward with PR -- | ||
|
|
||
| JK: Hope to merge next week. Might make sense to wait on approvals until the tests/docs are finished since right now it is ½ a PR. |
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,107 @@ | ||
| # Node.js Foundation Modules Team Meeting 2019-09-25 | ||
|
|
||
| ## Links | ||
|
|
||
| * **Recording**: http://www.youtube.com/watch?v=7c2gWo-g_Zs | ||
| * **GitHub Issue**: https://github.com/nodejs/modules/issues/386 | ||
| * **Minutes Google Doc**: https://docs.google.com/document/d/1JjL2yo8Mm3La_1-2P51PmJp2q4efE49YKyR62YxB6GA/edit | ||
|
|
||
| ## Present | ||
|
|
||
| * Myles Borins (@MylesBorins) | ||
| * Jan Krems (@jkrems) | ||
| * Ryan Day (@soldair) | ||
| * Alex Aubuchon (@A-lxe) | ||
| * Saleh Abdel Motaal (@SMotaal) | ||
| * Guy Bedford (@guybedford) | ||
| * Geoffrey Booth (@geoffreybooth) | ||
| * Hassan Sani (@inidaname) | ||
| * Rob Palmer (@robpalme) | ||
|
|
||
| ## Agenda | ||
|
|
||
| ## Announcements | ||
|
|
||
| *Extracted from **modules-agenda** labelled issues and pull requests from the **nodejs org** prior to the meeting. | ||
|
|
||
| ### nodejs/node | ||
|
|
||
| * esm: globals should be wrapped to stay in sync with ESM [#29426](https://github.com/nodejs/node/pull/29426) | ||
|
|
||
| I'm lacking context, but here's some rough notes - Alex | ||
|
|
||
| Myles: I played around with removing the flag, but found that it changed the way initialization worked -- involving proxies. Matteo gave a -1 on Bradley's PR for performance concerns. | ||
|
|
||
| Bradley: There's also an issue with getters. | ||
| A few approaches: disable importing the process module; allow the process module to go out of sync with the named exports; remove named exports from the process builtin; remove named exports from everything | ||
| We have to accept that we ban getters that throw. | ||
|
|
||
| Guy: There's a possibility of deprecation control by catching thrown errors. | ||
|
|
||
| Jan: At some point we will have to disallow naming certain properties. API design-wise, process and buffer were always focused around a default export, so restricting named exports isn't the worst thing. | ||
|
|
||
| Myles: It seems unergonomic to only have two builtins without named exports. Most people use the global process object, though that's not true for buffer. | ||
|
|
||
| Saleh: It might be a better path to have people import particular things from process. Selectively export particular things from process via a wrapper module around it. | ||
|
|
||
| Guy: I'd be very much for keeping the ability to import process and buffer. In an environment without globals, it's nice to be able to rely on that. | ||
|
|
||
| Bradley: Keep things importable, no objections for removing names. Not sure what Saleh means by having only particular names. We would need a proxy around the entire object, which is the problem currently. | ||
|
|
||
| Saleh: I am thinking less about the primitive types, and more about the object types from the process object. Primitive values needing to be synced complicates things. ie for process.next, export a wrapper function. | ||
|
|
||
| Bradley: What about `process.next`? | ||
|
|
||
| Saleh: That would be a wrapper function `(...args) => actualProcess.next(...args)` or a proxy of it if necessary. | ||
|
|
||
| Bradley: Upstream is open to anything that doesn't impact the performance of process.next. | ||
|
|
||
| Bradley: We can't freeze builtins; too much of the ecosystem relies on monkeypatching them etc. | ||
|
|
||
| ... | ||
|
|
||
| Guy: We could stop syncing between the esm and cjs exports. Loaders eventually would let us work around this (?). | ||
|
|
||
| Gus: Back when I added this proxy, it just copied the proxies. While I was adding this, I asked as a policy that we don't add any more deprecations that fire on access. | ||
|
|
||
| Jan: We need to focus on the core choice that Bradley mentioned earlier, rather than getting into the weeds. It might be easiest to just make it impossible to import the builtins now. | ||
|
|
||
| Myles: This specific issue is absolutely blocking unflag. Any changes that we need to make outside esm need to land asap for this. If we remove the flag and that changes performance characteristics of cjs, then that could be considered a breaking change and prevent unflag. We should find a proposal for solving this and bring it upstream needs to happen in the next week and a half. Does anyone want to make a specific proposal? | ||
|
|
||
| Bradley: If we want to unflag, adding the proxy would be problematic, but leaving it out wouldn’t. I think letting esm+cjs go out of sync is fine. I can't see a clear solution that fixes all the edge cases. | ||
|
|
||
| Gus: How often do APMs patch things more than once after starting? | ||
|
|
||
| Bradley: An audit last year says not often, only one group was *interested* in doing it. | ||
|
|
||
| Saleh: Given going out of sync, can we determine where process/buffer will be snapshotted | ||
|
|
||
| Bradley: It has to be when first accessed (imported) but we could expose an API down the road to update the snapshot. | ||
|
|
||
| Gus: Just to clarify, first accessed, ie imported or required. | ||
|
|
||
| ... | ||
|
|
||
| Myles: I'm plus one on removing the proxies. We could unflag, but keep things experimental -- allowing us to solve ecosystem issues that emerge in later versions. | ||
|
|
||
| ... | ||
|
|
||
| Myles: Action item for Bradley to make these changes, using Guy's api proposal. (:+1: from Bradley) | ||
|
|
||
| ### nodejs/modules | ||
|
|
||
| * Divergent specifier hazard [#371](https://github.com/nodejs/modules/issues/371) | ||
|
|
||
| Geoffrey: There's not much going on here. A doc change was made, and there's not much more to be done for this version. | ||
|
|
||
| * Loader Hooks [#351](https://github.com/nodejs/modules/issues/351) | ||
|
|
||
| Action item for Alex to alter the flag scheme for esm loaders. Either add a flag to enable the --loader flag, or change the --loader flag to something like --experimental-loader. | ||
|
|
||
| * Proposal: Support loading package by own "name" [#306](https://github.com/nodejs/modules/issues/306) | ||
|
|
||
| https://github.com/nodejs/node/pull/29327 | ||
|
|
||
| * Fallback Support Removed in Import Maps [#385](https://github.com/nodejs/modules/issues/385) | ||
|
|
||
|
|