Skip to content

Conversation

arash-mosavi
Copy link

Summary

This PR fixes issue #1727 where tsgo failed to contextually type callback parameters in object literals
when a discriminant property (e.g., visit: 'Declaration') was present. Unlike tsc, tsgo previously
gave the enter(node) parameter an implicit any type.

Details

  • When the contextual type of an object literal is a union of object types with a discriminant property:
    • Narrow the union to the constituent type that matches the literal discriminant value.
    • Use the narrowed type when applying contextual typing to other members (e.g., function parameters).
  • Fallback: if the discriminant is missing or ambiguous, fall back to current behavior.

Tests

  • Added baseline tests:
    1. Matching discriminant (visit: 'Declaration') → enter(node) is properly typed, no TS7006 error.
    2. Absent/mismatching discriminant → falls back to existing behavior (parity with tsc).

Notes

@Copilot Copilot AI review requested due to automatic review settings September 20, 2025 13:07
@arash-mosavi
Copy link
Author

@microsoft-github-policy-service agree

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes discriminant-based contextual typing for object literals in union types. Previously, tsgo failed to properly type callback parameters when a discriminant property was present, unlike the reference TypeScript compiler.

  • Adds reverse inference from literal types to indexed access types for discriminated unions
  • Implements type narrowing based on discriminant property values in object literals
  • Falls back to existing behavior when discriminants are missing or ambiguous

arash-mosavi and others added 3 commits September 20, 2025 19:12
- Add cssTreeTypeInference.ts test case to verify discriminated union type inference
- Include baseline files showing expected type inference behavior
- Ensures node parameter is correctly inferred as Declaration type in contextual typing scenarios
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

…enceInfoForType

- Change from TypeFlagsTypeParameter to TypeFlagsTypeVariable to match the pattern used in getInferenceInfoForType function
- This ensures consistency in type flag checking across inference-related functions
- TypeFlagsTypeVariable includes both type parameters and indexed access types, which is more comprehensive
- Updated comment to reflect the more accurate terminology 'type variable' instead of 'type parameter'
- Move blockedStringType check to beginning of loop for early exit
- Avoid expensive getIndexedAccessType() and isTypeIdenticalTo() calls when union member is blocked
- Improves performance by skipping unnecessary computations for blocked string types
- No functional changes, pure optimization
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

@arash-mosavi
Copy link
Author

Hello,

This pull request addresses #1727 by adding discriminant-based contextual typing for
object literals (e.g., visit: 'Declaration' now correctly narrows the union before typing
enter(node)), aligning the behavior with tsc 5.8.

  • CLA signed
  • Baseline tests added (covering both matching and
    This pull request implements a new reverse inference algorithm in the type checker to improve type inference for indexed access types, particularly when a literal value is assigned to an indexed access type. This change addresses issues with discriminated union inference (such as in CSS tree-like ASTs), ensuring that type parameters are correctly inferred in scenarios like T['type'] = 'Declaration'. Comprehensive tests are also added to verify the fix.

Type inference improvements:

  • Added a new inference case in Checker.inferFromTypes to handle situations where the source is a literal type and the target is an indexed access type, enabling reverse inference from the literal to the type variable.
  • Implemented the inferFromLiteralToIndexedAccess method, which infers the correct union member for a type parameter by matching the literal value with the indexed property type of union members. This is especially useful for discriminated unions.

Testing and validation:

  • Added a new test case (cssTreeTypeInference.ts) that reproduces a real-world issue with CSS tree type inference, ensuring that the fix works as intended.
  • Included baseline files for symbols and types to verify correct inference and symbol resolution in the new test scenario. [1] [2]
  • Updated the JavaScript baseline to include the expected runtime output for the new test case. fallback scenarios)
  • Local CI passing (pending workflow approval here)

Could you please approve the workflows and review this change?

@jakebailey — for workflow approval/merge
@weswigham or @sheetalkamat — for a checker/inference review

Thank you!

@ahejlsberg
Copy link
Member

@arash-mosavi Appreciate your PR, but unfortunately I don't think we want to merge it. See my explanation here: #1727 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsc vs tsgo: inference mismatch of parameter in passed callback
2 participants