Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design Meeting Notes, 10/13/2023 #56103

Closed
DanielRosenwasser opened this issue Oct 13, 2023 · 1 comment
Closed

Design Meeting Notes, 10/13/2023 #56103

DanielRosenwasser opened this issue Oct 13, 2023 · 1 comment
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

Node Proposal

nodejs/node#50043

  • In Node today, a .js file is CommonJS unless there's a package.json at the top level saying otherwise.
  • If you have an import or export in a CommonJS file, it's a hard crash.
  • Idea is how to push Node.js to be more ESM-y over time.
  • Set of heuristics - if...
    1. no node_modules and
    2. no package.json found at all, or no "type" field and
    3. parse contains some numbers of imports or exports and
    4. ? (come back to this)
  • ...then it's ESM.
  • Does the rule kick in only if there's no package.json? Or also no "type": ... field at all?
    • It's either!
  • The big problem here is that this introduces ambiguity because you don't know which file is the entry-point.
    • Ideally everything is consistent or it's not.
    • It is weird that you can't import a file without exports.
  • An alternative is that you could do import/export detection on every file
  • If you create a package that was derived from a script, it will break if you don't test it.
    • Grow up story from single file to package, you've got a footgun.
    • True, but package publishers are typically a bit more advanced.
    • Should note this in the issue.
  • Would be great if package managers could stamp packages with CJS if not currently.
  • Could imagine a mode in TypeScript that just errored as soon as something was ambiguous.
    • Basically say we need
      • An explicit type field
      • An explicit file extension
      • Otherwise error!
    • This is kind of what a lot of us wanted from our Node/ESM interop, but "oops, it uses ESM but it's CJS!" was a footgun we shipped.
    • What are the requirements?
      • verbatimModuleSyntax
      • tsconfig.json with something to opt out or something.
  • The lack of a known entry-point is ambiguous and even worse for us because it dictates what we generate as well.
  • How does the "error on ambiguity" mode work across different projects (including different projects in the editor)? That would be bad.
  • If Node hands users a footgun, does TypeScript need to adhere to the footgun?
    • We can just say add configuration.
    • Whole feature is designed to remove the need for configuration.
  • Top-level await makes the feature awkward, right?
    • Yes.
  • What about inferred projects?
  • Why is changing the default npm init not a reasonable change for adding "type": "module"
    • Wait, it doesn't today!?
    • Node.js module changes npm/cli#6855
    • No, and Node itself doesn't directly control any specific package manager, so it's not technically within their control.
    • Plus, yarn 1 and older versions of npm are still popular.
  • How does this affect the interpretation of .d.ts files?
    • These files use export and import, so it's ambiguous (still!)
    • To set ourselves up for success in the future, we will need to start encoding a marker in the .d.ts file (or start performing resolution to a .js file in addition to a .d.ts file)
  • Hold up, there is some ambiguity about whether or not code within node_modules is affected.
    • Yes, current intent is that within node_modules is still CJS unless otherwise declared.
    • Foot-guns around vendoring a package.
  • If you split things between JS/TS and loose files/configured"
    • TS/no config - does this scenario practically exist?
      • Maybe for people using ts-node and Bun and Deno - but our default language service will give you errors.
  • Per-file detection was something we were open to; however, this is different because it's dictated by the entry-point.
  • This is a fundamental problem for static analyzers trying to detect things on a non-configured best-effort manner. You need some configuration to make this work well in TypeScript.
    • Plus, what if two people do this? Showing errors that exist from one entry-point versus another is confusing.
  • There is a pressure to make ESM easier to work with in Node. It is annoying that CJS is the default. But adding new rules is going to add more confusion.
  • Feels like npm init and family should be changed, not add a new "forever" rule.
  • Going back to the declaration file disambiguator - what if we had a CJS-specific declaration syntax for export? We already have one for require.
    • We wish we'd done this years ago.
    • Isn't this incorrect? You detect the format from the surrounding context (package.json).
    • Well it tells you how the interop strategy will work - tells you if you need to write .default on something you imported.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Oct 13, 2023
@GeoffreyBooth
Copy link

  • An alternative is that you could do import/export detection on every file

This is in nodejs/node#50064. Currently it seems the more promising of the two proposals; I think we’ll probably go with nodejs/node#50064 unless it proves to be nonviable for some reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

3 participants