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

Latest commit

 

History

History
99 lines (78 loc) · 8.43 KB

2020-07-29.md

File metadata and controls

99 lines (78 loc) · 8.43 KB
Raw

Node.js Modules Team Meeting 2020-07-29

Links

Present

  • Modules team: @nodejs/modules
  • Jan Krems (@jkrems)
  • Geoffrey Booth (@GeoffreyBooth)
  • Guy Bedford (@guybedford)
  • Myles Borins (@MylesBorins)
  • Bradley Farias (@bmeck)
  • Jordan Harband (@ljharb)
  • Wesley Wigham (@weswigham)

Agenda

Announcements

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

nodejs/node

Policy conditions #34414

  • Bradley: Trying to align policy and module resolution. One example node: vs. nodejs: as the protocol for built-ins. Personal preference is simply node:. While testing, I realized that "exports" condition traversal isn’t shared between cjs and esm. Looked at cjs code but tied closely to cjs internals, using exceptions instead of return values.
  • Myles: No strong preference. Some people do prefer fewer characters (node:). Could also relate to built-in modules, even if it’s early in the standards process.
  • Jan: Feels risky to pick node:. Prior discussions about using node: for builtins raised concerns that it is too generic.
  • Bradley: As node evolves and may allow different languages, including js may become awkward.
  • Myles: Loaders are experimental, so breaking change may be fine. Aliasing could be good.
  • Jordan: Clarification: Does PR change nodejs to node? Which direction would be align?
  • Bradley: Would prefer node: if we align. Could support both like we did with getFormat.
  • Jordan: node is the name of the binary, using node: seems consistent.
  • Myles: node: could be registered, not registered yet.
  • Jan: Primary concern is consensus to allow the use of the same prefix for builtins in require expressions and import statements. In that context people had preferred nodejs:.
  • Myles: If this group is okay with protocol, don’t see a reason why it couldn’t be the direction.
  • Bradley: Will create 2nd PR to align protocol. May also refactor to reuse conditional logic.
  • Myles: Would like to make sure major refactorings don’t interfere with backporting to 12 and 14.
  • Bradley: Likely big refactor will not land cleanly.
  • Myles: General question about what we want to backport, may want to delay big refactoring until important features landed.

esm: Modify getFormat and getSource loader hooks #34144

  • Julian: Update from last meeting - dug into alternatives and implications. Question about transformSource relationship with getSource change. Linked doc with result of exploration in PR. After looking into loader chaining PR, seems conceptually compatible. transformSource out of scope for PR. Any further thoughts on proposed solutions and moving forward with doc updates?
  • (no fundamental concerns)
  • Julian will, therefore, move on with updating documentation and continue formalizing PR

Special treatment for package.json resolution and exports? #33460

  • Bradley: Suggestion is to special-case package.json so it’s always exposed in exports.
  • Jordan: Last time, we were looking at a PR to expose this data to tools outside of exports field resolution.

module: CJS exports detection #33416

  • Guy: Ongoing discussion about how to overcome existing objections. Latest was to explore if it’s possible to have a userland solution. Precondition was a PR to disable the ESM cache interactions in the CJS loader (nodejs/node#34467). Wrote a userland loader to allow experimentation.
  • Bradley: Would like to see some follow-up PRs. Haven’t seen strong desires in the ecosystem for CJS snapshotting, theoretical correctness concerns still exist.
  • Wes: Community has been using named exports using “custom loader hooks”, no further proof necessary that people expect it.
  • Guy: Still valuable to get feedback that this particular approach works for people.
  • Jan: Most importantly, this isn’t exactly what people have been using in the ecosystem. Finding out if this reduced version is useful would help make the case that it’s “good enough”.
  • Geoffrey: Have been working on tests to get a better idea about how good the detection works. Previous numbers were comparing it to all runtime properties but that isn’t necessarily what the actual expected named exports would be. Having a high number may overcome existing objections. Really about user expectations.
  • Guy: One example is that it should have full coverage for babel-generated outputs.
  • Wes: Should be the same coverage for TypeScript-generated outputs.
  • Geoffrey: Maybe it’s okay to “declare” all TS or babel generated sources as 100% because we know it covers their patterns.
  • Guy: Good place to discuss the TS side of things?
  • Wes: Likely the discord server (http://discord.gg/8CjzwY).
  • Bradley: Well known edge case with export * from.
  • Geoffrey: Shouldn’t be necessary to get to 100%, Gus should be okay with realistic but high coverage.
  • Jan: Would it maybe be good to put aggressive bailing on the table so that partially correct results aren’t as much of a risk?
  • Wes: Not good to speculate about Gus’ success criteria without talking to Gus.
  • Guy: May be possible to get “100% of targeted exports” by detecting transpiled output that can be clearly identified. Help with testing would be great. Pushed 3 proposals, it’s been a lot.
  • Jordan: Memory of Gus’ concerns was specifically about old code that’s unlikely to work. He wasn’t concerned as much about transpiled code.
  • Guy: May be an argument to focus on transpiled code and actively ignore hand-written code.

nodejs/modules

Subpath extension patterns and wildcard expansions #535

  • Guy: Noticed a lot of component libraries and SVG icon libraries that can have 100s of directly importable files. Using exports field requires prefix/path matching because maintaining 100s of entries isn’t practical. Usually exports field contains entries without file extension but for these packages it has to be a specifier with file extension. Do we want to allow users to append extensions so they can use prefix matches but still write extension-less specifiers?
  • Jordan: Agreed with desire. But we explicitly choose to not have extension searching. This drove us to very expansive lists. If we can find a way to make extensionless work, would be in favor. I don’t want people to use extensionless when importing from my packages.
  • Guy: We did add extension searching for prefix matching but only for CJS.
  • Jordan: If we care about consistent APIs, you always have to use exact paths. Can’t use prefix matching. Don’t want users to dirty their code with file extensions.
  • Guy: What is the concern about current proposal?
  • Jordan: Slash on RHS normally means “do what the module system normally does”.
  • Guy: Better way than “normal” path export. Current proposal makes it more complicated.
  • Jan: We don’t know where the ecosystem will be heading. Browsers require full relative paths, with file extensions. It may mean that in the future people do expect file extensions, in general. So extension-less bare imports may not be as desirable, may even stick out as “weird”.
  • Bradley: Like the idea of “templates” or something else that’s more expressive. Shouldn’t try to pass judgement on what the future will look like. Explicit extensions allow for surprising rewrites like import foo/x.json being actually file:///path/to/x.wasm. Relates to import assertions, may break unexpectedly.
  • Guy: Reason LHS doesn’t have wildcard. With both wildcard and path allows both, unclear precedent.
  • Jan: Active error for path vs wildcard?
  • Guy: Also different prefix length with nested prefixes. Can have further discussions on this.
  • Jan: One property that we lose: With current import map proposal, we cannot convert this to an import map without scanning file system.
  • Jordan: Not sure if we want to tie ourselves down by import map spec, may choose to pick dev ergonomics and accept heavier conversion cost to import map.

add API for "package dir" #516

(Out of time)