Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Latest commit

 

History

History
90 lines (71 loc) · 11.8 KB

2019-10-23.md

File metadata and controls

90 lines (71 loc) · 11.8 KB
Raw

Node.js Foundation Modules Team Meeting 2019-10-23

Links

Present

  • Myles Borins (@MylesBorins)
  • Jan Krems (@jkrems)
  • Wesley Wigham (@weswigham)
  • Geoffrey Booth (@GeoffreyBooth)
  • Alex Aubuchon (@A-lxe)
  • Jordan Harband (@ljharb)
  • Rob Palmer (@robpalme)
  • Ruy Adorno (@ruyadorno) (Observer/Guest)
  • Saleh Abdel Motaal (@SMotaal)
  • Guy Bedford (@guybedford)

Agenda

Announcements

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

nodejs/node

module: resolve self-references #29327

  • Jan: New issue for open questions modules 403. #403 Is this in scope for shipping modules? Slim use case - slightly nicer unit testing for package with exports. Everything else is unrelated. Looking for disagreements - otherwise it’s sunk cost and out of scope.
  • Myles: Engineering is now done behind a flag. What are we waiting on?
  • Jan: Current version works in theory Disagreements on things needed for unflagging - so uncomfortable landing now. Many things are more important for landing modules than this. It’s lost time on an unimportant feature. Should focus on core module experience.
  • Myles: Your advice - table and revisit later?
  • Jan: Yep.
  • JH: Agree. I hoped unflagging would have all resolution changes at that time. For example, resolve() needs updating - would be nice to handle self referencing at that time. Not a reason to hold back modules.
  • Myles: Apart from the sigil - is there a case for not moving forwards?
  • JH: Matteo’s running inside and outside package. Demands package name be a self-reference. We decided that we needed a sigil - package-name is insufficient. Sigil bikeshedding is not worth it.
  • GB: I thought we would ship package name then do the sigil later.
  • JH: Shipping sigil later is fine. I’d prefer both go unflagged.
  • MB: Objections to landing self-refs imports behind a flag using package name as the reference?
  • CONSENSUS ACHIEVED
  • Jan: As long as…
  • MB: This is separate to unflagging modules. It bitrots - landing it is to allow it to stay good so people can play with it.
  • WW: This name is the “name” in the package.json? package.json#name
  • JH: So resolving self-ref will not block modules AND will not block exports AND we’ll land the current PR behind an additional flag and land it in future.

nodejs/modules

Lets document the current state of Modules in Node.js #402

  • MB: We shared it with TSC. TSC discussed. Hazard was discussed. Existing hazards + new hazards. Equal good and bad responses. TSC want us to figure it out. TSC cannot give advice. Some people had more ideas - Myles will follow-up.

Divergent specifier hazard (DiSH) #371

Conditional exports proposal #401

  • GBD: This is a middle-ground - could exists separately from divergent specifier hazard issue.
  • MB: We talk in circles about DiSH. Conditional exports is the first thing that made me think it’s worth introducing the hazard in order to get the DX of conditional exports. There’s non-trivial work to do on conditional exports.
  • WW: I don’t get conditional exports. We started talking about DiSH - multiple things refer to the same thing via the same name. Feels bad a wrong to do the same thing another way.
  • GB: Default extensions was another way to allow import/require of the sme specifier to lead to different modules. We avoided because author can fall into the trap accidentally - want to minimize a footgun. Explicit syntax - needs to be taught at which point the hazards are taught too. It’s a power tool with a safety latch.
  • MB: To Wes, 1. Extension resolution led to DiSH and incompatibility with browser implementation. I know that's not perfect. But if we had arbitrary file extension resolution, import maps grow unbounded. It wasn’t the only reason. When we fixed the order of resolution, we removed overloaded “main” being used. So these are separate spaces to discuss. With DiSH, we already have a hazard today, creating another one is a problem. From consumer PoV, separate specifiers introduces a variant of this today (GraphQL plugins might bring in two versions of GraphQL). Could be transparent from a graph of different things. It’s on the authors to eliminate the hazard. We could block it. But people still need to solve i entrypoints can end up in both caches. The benefit of having it via single-specifier is more explicit. Education either way. With conditional exports, CJS packages can lead to auto generated ESM wrappers. Whilst I am uncomfortable, I am -0 on this - not an objector. We’ve spent a long time on this. Conditional exports allow us to hit almost every use-case we wanted. Still unsure if ready to land unflagged. Not sure if we should block unflagging on this being done. We should consider being flexible.
  • Jan: You stole my thunder. Wes brought up two points. 1 hey wait, I thought we cared about DiSH. Why bring it back? 2. Hey if we want this possibility, why is exports different from file resolution solution? The reason why is because ext resolution works for exactly 2. Does not scale beyond. IF you want Node solution and a generic one for the browser, it does not help you. Extension searching would never have provided that. For the hazard, I used to be “we should not allow specifier to resolve to different things”. Currently convinced that package and package#esm just mask the problem and ignore dual instance issue - but they cannot! The message is dangerous - “it’s safe, you can be safe from dual instances” but nobody is writing import and require in the same file. So if a single file imports… In practice a user doesn’t import the same thing using import and require. In a realistic app, the hazard does not appear in a file, or a project. Solution is to port the project to ESM. Hazard exists in a package tree. Can’t fork all dependencies if different specifiers used. It’s a dangerous path. We should be clear it should not be possible two ship two versions. If you want to ship a require in an ESM package, then you are responsible for managing the situation of both versions being loaded. Conditional exports is a good way to tell that story!
  • WW: Ok. If you started believing the hazard was a problem, then it’s still a problem with multi-entrypoints, it should still be a problem with mutli-entrypoint and package.json referencing. I like the current version because you can require(ESM) in future. This feature precludes that. I do not like allowing two copies to be loaded. The overhead is non-trivial. People talk about huge node trees. Doubling might not be the best idea.
  • JH: I agree with Wes’s idealised goal. require(ESM) sidesteps the issue. Export allows new and old node. If we don’t ship conditional exports, the browser field does not handle ESM. People are still targeting pre and post module browsers. ConExps give us a solution for bundlers. Handles many use-cases. Does not eliminate the hazard. People can always write dumb code. Node should not police this. ConExp avoids a tragedy. require(ESM) does not eliminate the need for ConExps. So I want this feature to get to unflagging.
  • GC: long muted speech
  • GBD: Wes’ points are valid. Except we should be decisive about require(ESM). Wes did good work. Now that work has stalled, we can’t say it will appear in 6 months. But it’s not spec compliant with ESM. Promises are specced to suspend main thread. Flattening promises would be non-spec compliant. So problem is lack of work and spec adherence. Top-level await makes this observable. Additionally, we should realize strong arguments on both sides. The problem is to find consensus and to avoid stalling. We only have 1 or 2 meetings. People are already shipping packages with multiple versions for different target. So what should Node do to reduce fragmentation? If we don’t give users ESM in Node, people will dual publish and . It’s a short transition rather than something we live with forever.
  • GC: still muted Gus says he mostly agrees with Wes and Jordan but will not suggest require(ESM). He warns to not get stuck in sunk cost fallacy.
  • BF: I want us to be more careful about framing. Some people actively want to use the workflows we might state as “dumb”. This is such a primal issue, we lose focus on if the issue will be solved by us. If you install two versions of React, this issue already exists in CJS. Discussion might be overblown. Tone it back. I firmly believe this multi-copy issue will be a natural fallout of many workflows and is already the case in the wild today.
  • Jan: To BF, I agree that it’s an existing issue. I agree that the framing of “we cna’t stop the user being dumb” is dangerous. Middle ground, people keep using a workflow and something just happens. Explicitly calling out some of these patterns: Babel transforms don’t rewrite specifiers. A reasonable package author will cause this if we have two specifiers. Comes from reasonable users and they get a bad outcome. This is different for trying to stop everyone from hurting themselves. For React, the new bit is the tools to manage the hazard. Npm ls shows you the multiple copies - and npm7 will surface errors during install for peer deps - but for the new hazard there is no tooling. If we use magic conventions, no tool will ever be able to understand what is going on. Those conflicts will appear and there is no way to debug. Declarative method lets you know why it breaks. I agree it’s bad for the world to be duplicated. If people want to ship both, and they cannot use require(ESM), there are two copies. As long as that’s true, we can’t prevent that. The issue is, how can we create patterns that have the right mental model. And how to track the issue without knowing all the implementations of your dependencies.
  • BF: I want to pushback. Sure we don’t have tooling. React case is not the only way that leads to duplication. Vendoring - shipped in multiple bundles. Today’s tooling cannot prevent duplication. It’s not a solved problem. We cannot point to a tool and say the problem is prevented. I question how important this whole issue is. We could write tools. Or we could educate. One or the other. So maybe we should turn our attention elsewhere.
  • Jan: Agree with Brad. There’s already code duplication. I pushback on the fact we could create future tooling. In one situation . In the other it needs to know about all the packages. So it’s the difference between having metadata ready for tools, or not having metdata.
  • GBD: To Jan, there’s a big diff between dupe code in the same runtime and different runtimes. On the point of shipping, it’s really about authors. Everyone is waiting. A long time. If the only way they can ship it is to break CJS consumers, it gives them a hard choice.
  • MB: Can we land ConExp behind a flag?
  • Wes objects.
  • MB: Can we land ESM as it exists?
  • JH - I object. We need ConExp. (but Wes blocks ConExp)
  • JH - Are we lone objectors?
  • silence
  • BF: I object to ConExp but not if it’s behind a dedicated flag.
  • JH: If someone has to run Node with a flag to get a feature in a dependency, it’s like it does not exist. People don’t run Node with flags. So defaults matter. If defaults do not support dual packages, you can’t publish it. So my objection remains.
  • MB: We can discuss a vote, but not now - time is up.