ES-conformant symbol typing - #64451
Michael FIG (michaelfig) wants to merge 10 commits into
Conversation
Move the new test cases and accepted baselines into tsc/testdata so the compiler test runner executes them. Regenerate baselines with the repository's CRLF convention. Co-authored-by: Codex <codex@openai.com>
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
typescript-automation
Bot
added
the
For Uncommitted Bug
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Non-literal and union registry keys currently receive unsound single unique-symbol identities.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: None
What changed in this PR
Adds registered-symbol typing and preserves symbol identities through inference, property access, and declaration emit.
Changes:
- Introduces
RegisteredSymbol<Key>and genericSymbol.for. - Preserves unique-symbol types in const-like contexts.
- Adds compiler tests and baselines for symbol-keyed properties.
| File | Description |
|---|---|
tsc/internal/checker/checker.go |
Implements registered-symbol types and unique-symbol preservation. |
tsc/internal/checker/nodebuilderimpl.go |
Emits registered symbol types and computed property names. |
tsc/internal/bundled/libs/lib.es2015.symbol.d.ts |
Declares the new intrinsic and Symbol.for signature. |
tsc/testdata/tests/cases/compiler/registeredSymbolIntrinsic.ts |
Tests intrinsic inference. |
tsc/testdata/tests/cases/compiler/registeredSymbolPropertyAccess.ts |
Tests symbol-keyed property access and emit. |
tsc/testdata/tests/cases/compiler/uniqueSymbolConstLikeWidening.ts |
Tests const-like unique-symbol widening. |
tsc/testdata/baselines/reference/compiler/registeredSymbolIntrinsic.types |
Records inferred types. |
tsc/testdata/baselines/reference/compiler/registeredSymbolIntrinsic.symbols |
Records resolved symbols. |
tsc/testdata/baselines/reference/compiler/registeredSymbolPropertyAccess.types |
Records property-access types. |
tsc/testdata/baselines/reference/compiler/registeredSymbolPropertyAccess.symbols |
Records property symbols. |
tsc/testdata/baselines/reference/compiler/registeredSymbolPropertyAccess.js |
Records declaration emit. |
tsc/testdata/baselines/reference/compiler/uniqueSymbolConstLikeWidening.types |
Records widening types. |
tsc/testdata/baselines/reference/compiler/uniqueSymbolConstLikeWidening.symbols |
Records widening symbols. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Resolve broad registered-symbol keys to symbol and distribute literal union keys over their possible registered symbol identities. Add compiler coverage for both cases. Co-authored-by: Codex <codex@openai.com>
|
The TypeScript team hasn't accepted the linked issue #27524. If you can get it accepted, this PR will have a better chance of being reviewed. |
Michael FIG (michaelfig)
changed the title
| // Keep a type parameter deferred so instantiating Symbol.for's generic | ||
| // return type can resolve the actual key supplied at the call site. | ||
| if keyType.flags&(TypeFlagsStringOrNumberLiteral|TypeFlagsTypeParameter) == 0 { | ||
| return c.esSymbolType | ||
| } |
There was a problem hiding this comment.
How does this keep the type parameter deferred? Doesn't this just return symbol?
I think you not only need a new intrinsic type, you need a new kind of type (e.g. see newStringMappingType). Similar to how a type with TypeFlagsStringMapping doesn't have TypeFlagsString, it wouldn't have TypeFlagsSymbol - maybe because it could be instantiated to never? I'd have to consult with others.
There was a problem hiding this comment.
I guess we just handle that in assignability separately and keeping the bitflags disjoint on this avoids other issues.
There was a problem hiding this comment.
I agree that a separate type flag is a better way to represent a registered symbol whose key is still a type parameter. I’ve updated the PR to use that representation and added a regression test where two Symbol.for calls with generic keys previously shared an unsound unique-symbol identity.
My main hesitation was the limited remaining space in the TypeFlags uint32. It seems that all the bits up to bit 31 are already in use, so this uses the former Reserved3 bit at 31 and keeps TypeFlags as uint32. That consumes the remaining reserved bit, but seems justified for a distinct type kind.
I’d welcome your assessment of that tradeoff.
Avoid assigning a unique symbol identity to Symbol.for calls with unresolved keys. Add a regression case for indexed access through two generic registry keys. Co-authored-by: Codex <codex@openai.com>
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
Review effort: Lite
Findings: 1
Open (3)
Files not reviewed (1)
- tsc/internal/api/enum_values_generated.go: Generated file
| return r.isRelatedTo(source.AsStringMappingType().target, target.AsStringMappingType().target, RecursionFlagsBoth, false /*reportErrors*/) | ||
| } | ||
| case source.flags&TypeFlagsRegisteredESSymbol != 0: | ||
| return r.isRelatedTo(source.AsRegisteredESSymbolType().target, target.AsRegisteredESSymbolType().target, RecursionFlagsBoth, false /*reportErrors*/) |
| TypeFlagsBooleanLike = TypeFlagsBoolean | TypeFlagsBooleanLiteral | ||
| TypeFlagsEnumLike = TypeFlagsEnum | TypeFlagsEnumLiteral | ||
| TypeFlagsESSymbolLike = TypeFlagsESSymbol | TypeFlagsUniqueESSymbol | ||
| TypeFlagsESSymbolLike = TypeFlagsESSymbol | TypeFlagsUniqueESSymbol | TypeFlagsRegisteredESSymbol |
| @@ -505,7 +505,7 @@ const ( | |||
| // This *should* be every type other than null, undefined, void, and never | |||
| TypeFlagsNarrowable = TypeFlagsAny | TypeFlagsUnknown | TypeFlagsStructuredOrInstantiable | TypeFlagsStringLike | TypeFlagsNumberLike | TypeFlagsBigIntLike | TypeFlagsBooleanLike | TypeFlagsESSymbol | TypeFlagsUniqueESSymbol | TypeFlagsNonPrimitive | |||
Add regression coverage for generic key relations and symbol union and intersection reduction. Co-authored-by: Codex <codex@openai.com>
Preserve const alias identities in declaration output and refresh the unique-symbol declaration baselines. Co-authored-by: Codex <codex@openai.com>
Refresh 19 type, symbol, and diagnostic baselines while leaving the computed registered-symbol type baseline for further investigation. Co-authored-by: Codex <codex@openai.com>
Serialize computed expressions through their known registered-symbol types, preserving ordinary and readonly properties. Add declaration emit coverage and accept the corrected symbol-name baseline. Co-authored-by: Codex <codex@openai.com>
This PR is AI-generated from human-written test cases that demonstrate discrepancies between TypeScript typing and ES standard behaviour. The PR itself has also been reviewed by me before submission. Here are the main issues it addresses:
Closes: #35909
Two
Symbol.forcalls with the same key currently produces non-identicalunique symboltypes, so TypeScript rejects equality checks and assignments that succeed at runtime. This PR gives them the same registered-symbol type when their keys are the same literal.Closes: #27524
Assigning a known symbol to another
constcurrently widens its type tosymbol, preventing the alias from serving as the same computed property key. This PR retains the symbol's narrow type through the const alias.Closes: #55901
A
constinitialized with an assertion such asu as typeof ucan still lose itsunique symboltype, complicating uses such as isolated declaration emit. This PR preserves the asserted symbol identity in that const-like declaration.Summary
RegisteredSymbol<Key>intrinsic and preserve literal registry keys in the return type ofSymbol.for.unique symboltypes in const-like declarations and allow const assertions on them.Tests
go -C ./tsc test -run='TestLocal/(registeredSymbolComputedNameDeclaration|registeredSymbolGenericRelations|registeredSymbolDeferredKey|registeredSymbolIntrinsic|registeredSymbolPropertyAccess|uniqueSymbolConstLikeWidening)\.ts$' ./internal/testrunner -count=1go -C ./tsc test ./internal/checker ./internal/apigo -C ./tsc test -run='^TestLocal$' ./internal/testrunner -count=1Related Work
Refs: #35562
This issue asks for symbols stored in an
as constobject to retainunique symboltypes so they can be used as computed class property names. It relates to this PR's preservation of narrow symbol types in const-like contexts.Refs: #37469
This issue proposes symbol-specific return types for functions and aliases of
SymbolandSymbol.for, including a type tied to a global registry key. This PR addresses the registry-backed portion throughRegisteredSymbol<Key>; it does not add a generalnew symbolreturn type.Refs: #54778
This earlier PR proposed preventing readonly symbol values from widening, including cases involving indirect unique-symbol declaration emit. Its approach overlaps with this PR's const-like symbol preservation.
Refs: #60052
This merged PR relaxed restrictions on computed names in declarations and preserved computed names expressed as entity names during declaration emit. Those changes are relevant to representing and emitting the symbol-keyed properties handled here.
Co-authored-by: Codex codex@openai.com