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, 3/12/2024 #57756

Closed
DanielRosenwasser opened this issue Mar 12, 2024 · 0 comments
Closed

Design Meeting Notes, 3/12/2024 #57756

DanielRosenwasser opened this issue Mar 12, 2024 · 0 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

Reusing Type Nodes from Assertion Expressions

#57587
#57589

  • Gist of the PR is that when you have an as assertion, like const x = foo as number, you should be able to do declaration emit as const x: number = foo.
  • Valid to move between a top-level type in an as versus an annotation.
  • Last time we discussed, we liked this because it reused more nodes, but concerns were raised that this logic should live in the TypeChecker versus the EmitResolver.
  • TypeChecker and its NodeBuilders takes care of many of the concerns.
  • If we want to scale out builds, relying on the TypeChecker for this causes problems.
    • But if we actually care about perf, why are we not committing to a fast transpileDeclaration?
  • Current feedback we've provided is just to stick to checks and make our output conform more with an isolated declaration emitter.
    • A faster implementation can happen down the line.
    • Or an alternative emit resolver can do the logic in the PR instead of calling out to the checker.
  • Also: doing this in the checker means that JS declaration emit gets better.
  • A lot of PRs for a large change - asking to reorganize this has a lot of back-and-forth and risks slipping on 5.4.
  • Conclusion: we will prototype a change where this work is done in the NodeBuilder.

Do not preserve triple-slash references unless marked preserve

#57655
#57681

  • We have a ton of test cases that depend on this stuff working.
  • Have to decide for these test cases whether:
    • The new baselines are good.
    • Where we need to specify preserve.
    • The tests need to be fixed in some other configuration manner.
  • Seeing what was affected in the top 200 mostly showed up stuff where people would have React, Node, or whatever loaded up anyway.
  • Is isolatedDeclarations going to be compatible with --module amd/umd?
    • Mostly that isolatedModules is incompatible with --outFile, which those work with.
  • Adding preserve is not the worst workaround.
  • Two classes of issues:
    • Used to generated a reference, now don't.
    • References that used to be preserved, now don't.

Spec conformance (break) on decorator syntax in --experimentalDecorators

#57749

  • We have a fairly lax sytnax

  • Decorators proposal says either parenthesized expressions or dotted names with a trailing call expression.

    (
      @a.b.c // ✅
      class
    )
    (
      @a.b.c() // ✅
      class
    )
    (
      @(a.b().c) // ✅
      class
    )
    
    (
      @a.b?.c // ❌
      class
    )
    (
      @a.b().c // ❌
      class
    )
  • TypeScript's decorator implementation is much more relaxed on the grammar though. So we should decide on how we take the break.

    • In other words, do we do this in experimentalDecorators?
  • Current implementation still parses gracefully and emits well.

  • Most people feel like we want a consistent grammar error between the two.

  • Have a quick fix to just parenthesizes.

  • Top repos?

    • Top400 was clean!
  • We do allow postfix ! because it's TypeScript only.

  • Type argument instantiation expression?

    • No, not right now.
  • Conclusion: ship it.

Do not emit reverse mappings for syntactically deteriminable strings

#57686

  • Today when we see an enum like

    import { foo } from "./foo";
    enum E {
      TotallyAString = `${foo}`,
    }

    under transpileModule, we create a reverse-mapping for it(!)/

  • We need to choose what we want to happen.

    • Either reject the code under isolatedModules, or
    • Know that the initializer is trivially a string and avoid creating the reverse map.
  • Proposal: when we see see certain initializers, don't create the reverse mapping entry.

  • What happens if you don't have it in a template string?

    import { foo } from "./foo";
    enum E {
      NotClearlyAString = foo,
    }
  • We do create a reverse-mapping there, but it errors in isolatedModules.

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
2 participants