microsoft / microsoft/TypeScript
Replace Node interface with discriminated unions
Nessuno ha ancora preso questa issue.
- Lingua principale
- Go
- Stelle
- 111k
- Fork
- 14.4k
- Merge medio
- 1g 19h
- PR unite (30g)
- 117
Descrizione
🔍 Search Terms
discrimnated type union node internal source api
✅ Viability Checklist
- This wouldn't be a breaking change in existing TypeScript/JavaScript code
- This wouldn't change the runtime behavior of existing JavaScript code
- This could be implemented without emitting different JS based on the types of the expressions
- This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
- This feature would agree with the rest of our Design Goals: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals
⭐ Suggestion
Happy Halloween 🎃! In the spirit of the holiday, I'd like to resurrect #13634: using a discriminated unions for Node categories. Roughly paraphrasing #18285's very good description here:
As mentioned in https://github.com/Microsoft/TypeScript/pull/18214#issuecomment-326823448, TypeScript's source can remove a lot of casts by using a discriminated union for Node.
While this should not affect TypeScript's runtime performance (as it just changes its types, mostly) it may cause TypeScript to take longer to compile itself.
#13634 was declined in 2018 for performance concerns (https://github.com/microsoft/TypeScript/issues/13634#issuecomment-413289616) but I hope that the years of performance improvements since then have since made this possible.
📃 Motivating Example
Switching TypeScript's Node from an interface from a discriminated type union would allow .kind checks to narrow types of values declared as Node.
import * as ts from "typescript";
declare const node: ts.Node;
if (node.kind === ts.SyntaxKind.FunctionDeclaration) {
node.name?.text;
// ~~~~
// Property 'name' does not exist on type 'Node'.
}
💻 Use Cases
In TypeScript itself, a match-case-and-full-word search for /(?:name|node|parent) as \w{5,}/ in src/**/*.ts excluding *test* yields 1879 matches right now. After prototyping it locally and enabling @typescript-eslint/no-unnecessary-type-assertion, I mostly-automatically reduced it to 987 matches. More removals could likely be enacted with manual effort.
- Previous attempt: https://github.com/microsoft/TypeScript/pull/18285
- My attempt: https://github.com/Microsoft/TypeScript/pull/56274
- (edit) Wes' PR, which everyone else in this thread except Andarist has previous commented on but I somehow forgot about: #54148
In community projects that use the TypeScript AST, including typescript-eslint, we'd similarly be able to have more precise type information on TS AST nodes.
#13634 mentions other consumers too, such as Babel's https://github.com/babel/babel/blob/c446ff85c28e117ebf3cd72cd34ef358f1077aa8/packages/babel-types/src/validators/is.ts.
Guida per i contributori
Apri la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Direzione di ricerca
Inizia esaminando i tentativi precedenti in #18285, #56274 e #54148, quindi ispeziona i 1879 cast match in src/**/*.ts menzionati nell’issue. Il lavoro sarà completo quando l’interfaccia Node sarà sostituita da unioni discriminate, i cast giustificati saranno rimossi quando il narrowing li rende superflui e il comportamento a runtime sarà preservato senza emettere JavaScript diverso.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- typescript
- Ambito
- compilers
- Tipo di issue
- Refactoring
- Difficoltà
- 5/5
- Tempo stimato
- Più di una settimana
- Stato di attività
- Ferma
- Chiarezza
- Abbastanza chiara
- Idoneità per principianti
- 25/100