Provide implicit index signatures for JS expando object literals#3808
Open
ahejlsberg wants to merge 4 commits into
Open
Provide implicit index signatures for JS expando object literals#3808ahejlsberg wants to merge 4 commits into
ahejlsberg wants to merge 4 commits into
Conversation
ahejlsberg
requested review from
DanielRosenwasser,
RyanCavanaugh,
Copilot and
jakebailey
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts how the checker treats JavaScript “expando” object literals with respect to inferred/implicit index signatures, aiming to make behavior consistent across scopes and to align diagnostics with the expected TypeScript behavior described in #3739/#3691.
Changes:
- Updates
Checker.isObjectTypeWithInferableIndexto treat more object types (including JS literals) as having an inferable index. - Adds a new compiler test (
expandoObjectIndexSignatures) with corresponding reference baselines. - Updates
expandoNoInferredIndexbaselines to reflect the (re-)introduced TS7022 circularity/implicit-any diagnostic.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/checker/relater.go |
Adjusts the predicate that decides when an object can be treated as having an inferable index signature. |
testdata/tests/cases/compiler/expandoObjectIndexSignatures.ts |
Adds coverage intended to exercise expando object literal index-signature behavior. |
testdata/baselines/reference/compiler/expandoObjectIndexSignatures.types |
Reference types baseline for the new test. |
testdata/baselines/reference/compiler/expandoObjectIndexSignatures.symbols |
Reference symbols baseline for the new test. |
testdata/baselines/reference/compiler/expandoObjectIndexSignatures.errors.txt |
Reference error baseline for the new test. |
testdata/baselines/reference/compiler/expandoNoInferredIndex.types |
Updates expected types output due to the behavior change. |
testdata/baselines/reference/compiler/expandoNoInferredIndex.errors.txt |
Adds/updates expected diagnostic output for TS7022 in the existing scenario. |
Comment on lines
+4586
to
+4595
| t.objectFlags&ObjectFlagsObjectRestType != 0 || | ||
| t.symbol.Flags&(ast.SymbolFlagsObjectLiteral|ast.SymbolFlagsTypeLiteral|ast.SymbolFlagsEnum|ast.SymbolFlagsValueModule) != 0 && | ||
| t.symbol.Flags&ast.SymbolFlagsClass == 0 && !c.typeHasCallOrConstructSignatures(t) || | ||
| t.objectFlags&(ObjectFlagsJSLiteral|ObjectFlagsObjectRestType) != 0 || |
Comment on lines
4592
to
4596
| return t.symbol != nil && | ||
| (t.symbol.Flags&(ast.SymbolFlagsObjectLiteral|ast.SymbolFlagsTypeLiteral) != 0 && len(t.symbol.Exports) == 0 || | ||
| t.symbol.Flags&(ast.SymbolFlagsEnum|ast.SymbolFlagsValueModule) != 0 && t.symbol.Flags&ast.SymbolFlagsClass == 0) && | ||
| !c.typeHasCallOrConstructSignatures(t) || | ||
| t.objectFlags&ObjectFlagsObjectRestType != 0 || | ||
| t.symbol.Flags&(ast.SymbolFlagsObjectLiteral|ast.SymbolFlagsTypeLiteral|ast.SymbolFlagsEnum|ast.SymbolFlagsValueModule) != 0 && | ||
| t.symbol.Flags&ast.SymbolFlagsClass == 0 && !c.typeHasCallOrConstructSignatures(t) || | ||
| t.objectFlags&(ObjectFlagsJSLiteral|ObjectFlagsObjectRestType) != 0 || | ||
| t.objectFlags&ObjectFlagsReverseMapped != 0 && c.isObjectTypeWithInferableIndex(t.AsReverseMappedType().source) |
Member
Author
There was a problem hiding this comment.
No, it's fine the way it is.
Comment on lines
+8
to
+16
| /** @param {Record<string, number>} obj */ | ||
| function fx(obj) {} | ||
|
|
||
| const obj1 = {}; | ||
| fx(obj1); | ||
|
|
||
| const obj2 = {}; | ||
| obj2.x = 1 | ||
| fx(obj2); |
Member
Author
There was a problem hiding this comment.
The desired behavior is for an empty {} to be assignable to Record<string, ...>, so tsgo was already doing it right. But that wasn't the case for an expando object literal. We don't need more test cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With this PR we consistently provide implicit index signatures for JS expando object literals. We're currently in a very confused state in this area as discussed here.
Note that this will resurrect the error observed in #3691, but that was fixed in the belief that TS6 was consistent here, which it isn't. In particular, the repro from #3691 errors when contained in a function or method, but succeeds at top-level. With this PR we'll consistently catch the circularity and report it with
noImplicitAny: true.Fixes #3739.