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, 9/15/2023 #55754

Closed
DanielRosenwasser opened this issue Sep 15, 2023 · 0 comments
Closed

Design Meeting Notes, 9/15/2023 #55754

DanielRosenwasser opened this issue Sep 15, 2023 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 15, 2023

API for Skipping JSDoc

#55739

(background is #52921)

  • Skip parsing JSDoc when not needed #52921 made it so that tsc can skip JSDoc so that fewer nodes need to be allocated.
    • Nice reduction in memory use + allocation/GC time!
  • Prettier and typescript-eslint care about comments, but they don't care about how TypeScript's parser populated comments.
  • In Make JSDoc skipping public, plus more configurable #55739, the proposal is to add a flag so that other 3rd party tools can call into createSourceFile while also skipping JSDoc nodes.
  • The downside is that it's more API surface area.
  • ESLint already has something similar to say whether or not to bother parsing comments.
  • 3 new modes:
    • Skip-all
    • Keep-all
    • Keep semantic only.
      • What is a semantic comment?
      • Comments that contain @see or @link, or any comment outside of TS/TSX.
      • Not "safe" to use in the services layer where you want quick info to display JSDOc and whatnot.
  • Could we do JSDoc population lazily?
    • Complicated by the fact that JSDoc checking really expects the comments to be around for JS files.
    • Probably best to keep it consistent for now.
  • Still a good improvement overall, excited to make 3rd party tools faster too.
  • Want to still bikeshed the name.

Enabling Implementations of Functions with Overloads and Conditional Return Types

#33014
#33912

Imagine (from our handbook)

/**
 * @param idOrName {string | number}
 */
function createLabel(idOrName) {
    if (typeof idOrName === "number") {
        return { id: idOrName };
    }
    return { name: idOrName };
}
  • Could do overloads

    function createLabel(id: number): LabelWithId;
    function createLabel(name: string): LabelWithName;
    function createLabel(isOrName: string | number): LabelWithId | LabelWithName;
    function createLabel(isOrName: string | number): LabelWithId | LabelWithName {
        // ...
    }
    • But no checking that the overloads themselves correspond to the implementation.
    • Can implement this incorrectly.
  • Conditional types can describe what will happen - but today you cannot usually assign to them without a type assertion/cast.

    • When you introduce a cast, you are very likely to still make a mistake.
    • Also very verbose - you have to rename the casted type over and over again between the declared return type and each return expression.
  • Easy to get this stuff wrong.

  • So do we have agree we should fix this?

    • And if so, which? Overloads or conditional types?
    • Would love to have both. If we had to pick one, conditional types because overloads don't play well with unions etc.
  • Have a prototype working for conditional types - can't cover every case, requires distributive conditional and the like.

  • There's also a question of type soundness - conditional types aren't quite sound.

    • Still, today the workaround is to blast a wall instead of sneaking in through the air vent.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Sep 15, 2023
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

2 participants