Fix reverse mapped type inference when all properties are context-sensitive - #64252
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
The unresolved inference issue prevents correctly inferring the generic context member.
Pull request overview
Fixes reverse-mapped type inference when all properties are context-sensitive.
Changes:
- Broadens partial inferability for non-empty object literals.
- Adds regression coverage and compiler baselines.
- Updates existing inference expectations.
The callback is contextually typed, but the generic D["context"] remains inferred as object rather than { foo: number }.
File summaries
| File | Summary |
|---|---|
tsc/testdata/tests/cases/compiler/reverseMappedTypeWithAllContextSensitiveProperties.ts |
Adds regression coverage. |
tsc/testdata/baselines/reference/compiler/reverseMappedTypeWithAllContextSensitiveProperties.types |
Records inferred types. |
tsc/testdata/baselines/reference/compiler/reverseMappedTypeWithAllContextSensitiveProperties.symbols |
Records symbol mappings. |
tsc/testdata/baselines/reference/compiler/reverseMappedPartiallyInferableTypes.types |
Updates inference expectations. |
tsc/internal/checker/inference.go |
Adjusts reverse-mapped type inferability; generic context inference remains incomplete. |
Review details
Suppressed comments (1)
tsc/internal/checker/inference.go:1060
- This change only makes the callback contextually typed; it does not make the generic
Dinfer itscontextmember. The generated.typesbaseline for this test still reports the call result as{ context: object; ... }, so a consumer ofconst machine = createMachine(...)cannot accessmachine.context.fooeven though the callback parameter can. Please add a return-type assertion to the regression test and adjust the inference path soD["context"]is inferred as{ foo: number }, not just used to contextualizeentry.
return t.objectFlags&ObjectFlagsNonInferrableType == 0 || isObjectLiteralType(t) && len(c.getPropertiesOfType(t)) != 0 || isTupleType(t) && core.Some(c.getElementTypes(t), c.isPartiallyInferableType)
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
nice catch copilot fixed it! (just needed to tweak the userland types, the compiler is already good) |
|
TypeScript Bot (@typescript-bot) test it |
Fixes #64251
The code change feels wrong because there should be a reason why the original code was like that but given there are no tests failing (well there's one baseline change but it doesn't look like a bad change to me) so I thought of opening this PR.
Happy to be proven wrong by showing some counter bad changes. Maybe I can be a little more conservative and target only the linked issue case.