- 
                Notifications
    
You must be signed in to change notification settings  - Fork 13.1k
 
Description
Reusing Type Nodes from Assertion Expressions
- Gist of the PR is that when you have an 
asassertion, likeconst x = foo as number, you should be able to do declaration emit asconst x: number = foo. - Valid to move between a top-level type in an 
asversus 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 
TypeCheckerversus theEmitResolver. TypeCheckerand itsNodeBuilders takes care of many of the concerns.- If we want to scale out builds, relying on the 
TypeCheckerfor this causes problems.- But if we actually care about perf, why are we not committing to a fast 
transpileDeclaration? 
 - But if we actually care about perf, why are we not committing to a fast 
 - 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
- 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 
isolatedDeclarationsgoing to be compatible with--module amd/umd?- Mostly that 
isolatedModulesis incompatible with--outFile, which those work with. 
 - Mostly that 
 - Adding 
preserveis 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
- 
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? 
 - In other words, do we do this in 
 - 
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
- 
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.
 
 - Either reject the code under 
 - 
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.