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, 5/28/2024 #58686

Open
DanielRosenwasser opened this issue May 28, 2024 · 3 comments
Open

Design Meeting Notes, 5/28/2024 #58686

DanielRosenwasser opened this issue May 28, 2024 · 3 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

Options for import cjs from "cjs" and require(esm)

#54102

  • Two options

  • What is the motivating example?

    // package.json
    {
        "type": "module"
    }
    
    // index.js
    import pkg from "./vendor/pkg/index.js";
    console.log(pkg);
    
    // vendor/pkg/index.js
    module.exports.__esModule = true;
    module.exports.default = "pkg";    
  • So there is a divergence here.

    • Lots of bundlers say this just echoes the string pkg to the console.
    • esbuild and Webpack do the "Node-specific" behavior by saying that the entirety of vendor/pkg/index.js is brought in as a default import, and itself is an object with a default property. So it logs either [Object object] or { __esModule: true, default: "pkg" }.
      • Also, the behavior for Webpack differs between JavaScript and TypeScript.
  • Rollup makes this all configurable based on the build, and uses a flag called defaultIsModuleExports to control the import cjs from "cjs"` behavior.

    • That's the option name we have in option 2.
  • What don't we like?

    • It isn't clear how much impact there is - not a ton of reactions, and linked issues seem unrelated.
      • But if you're bundling, you might not hit this because "type": "module" often isn't specified (or people don't use .mts).
        • We've even told people not to turn it on because bundlers differ in behavior.
      • Could clarify a bit on how the code ended up in this state.
    • There's also a concern around complexity - more flags.
    • Also, discoverability - how do you know that you need to set it?
      • Also, lots of examples where libraries are resolved as ESM in bundler, but the types don't indicate that.
      • Better error message to say "if you're really trying to do this, you need this flag."
  • Could imagine this just being a granular flag for a bigger module flag that specifies a specific behavior for esbuild, swc, etc.

    • If we had a plan there, maybe we'd feel a lot better doing this.
  • What if we took the opposite stance - turn these on and people need to turn the flags off.

    • Arguably, every bundler tries to align with Node, so the node-bundler behavior feels like what people want, right?
    • It's hard to fully convince ourselves there. e.g. Bun decided not to adopt the esbuild/webpack behavior, so maybe it's not as clear-cut as we'd want.
  • We would like to hold off on this - both awaiting more feedback and want to understand how the original scenarios were hit.

Checking of string literal union object keys and template types

#58673

  • Union types of computed properties get smooshed into a string index signature.

  • Would imagine { [... as "x" | "y"]: string } would be equivalent to { x: string } | { y: string }.

  • What about template types?

    • { [... as `foo${string}`]: string } - should this be equivalent to { [props: `foo${string}`]: string }?
    • Uhhh.
  • Arguably, it feels rare that you want to get an index signature because you end up with a way-too-permissive object.

    • Only solved via noUncheckedIndexedAccess.
  • Another perspective - you say that every constant-y value gets a unique type for indexing.

  • A few different treatments.

    declare const xy: "x" | "y";
    
    const x1 = {
        [xy]: 123,
    };
    
    // could be
    
    const x1: { [props: string]: number }; // today's behavior
    
    // or
    
    const x1: { x: number } | { y: number };
    
    // or
    
    const x1: { x?: number, y?: number };
    
    // or
    
    const x1: { x: number, y?: number } | { x?: number, y: number };
    
    // or something where where an index signature actually acts differently,
    // defers some of its computation for different behavior of reading/writing.
    
    const x1: { [props: "x" | "y"]: number };
  • Today, the last example just desugars to { x: number, y: number } because literal types desugar into individual properties.

    • That largely aligns behavior with mapped types.
  • And there's questions of overlap etc., but we have index signature applicability rules, so maybe there's something there.

@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label May 28, 2024
@DanielRosenwasser DanielRosenwasser changed the title Design Meeting Notes Design Meeting Notes, 5/28/2024 May 28, 2024
@robpalme
Copy link

  • Lots of bundlers say this just echoes the string pkg to the console.
  • esbuild and Webpack do the "Node-specific" behavior

Vite uses esbuild so I'm wondering which bundlers do the non-Node behavior? I checked out the linked issues but could not immediately see a list.

@DanielRosenwasser
Copy link
Member Author

@andrewbranch knows best - Parcel, Bun, and Rollup at least come to mind.

@andrewbranch
Copy link
Member

Parcel, Bun, and Rollup are the ones I tested latest versions of in preparation for this design meeting. Versions are a little older, but Vite is included in https://andrewbranch.github.io/interop-test/#default-export-esmodulejs and you can see it’s aligned with Parcel/Bun/Rollup, not esbuild/Webpack. Vite uses Rollup in production and esbuild in development. The report linked above is showing results with Vite instrumented in production mode. I’m not sure whether it’s possible to observe the esbuild interop behavior in Vite development mode.

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