String constant fails to infer type inside nested unions

[nmain]

Bug Report

🔎 Search Terms

infer contextual string union

🕗 Version & Regression Information

• This changed between versions 5.1.3 and 5.0.4

⏯ Playground Link

Playground link with relevant code

💻 Code

export interface UnionAltA {
  type?: 'text';
}

export interface UnionAltB {
  type?: 'image' | 'video' | 'document';
}

export type ValueUnion = UnionAltA | UnionAltB;

export type MapOrSingleton =
  | {
      [key: string]: ValueUnion;
    }
  | ValueUnion;

const withoutAsConst: MapOrSingleton = {
  1: {
    type: 'text' /*as const*/,
  },
};

const withAsConst: MapOrSingleton = {
  1: {
    type: 'text' as const,
  },
};

🙁 Actual behavior

The declaration of withoutAsConst fails, as type: "text" fails to be inferred as "text", and instead infers as string in this situation:

Type 'string' is not assignable to type '"text" | "image" | "video" | "document" | undefined'.

🙂 Expected behavior

The declaration of withoutAsConst should succeed. Typescript understands that the only valid values in this position are "text" | "image" | "video" | "document" | undefined from the contextual type, so the literal "text" should match that.

[fatcerberus]

Not related to the issue, but the types being defined in reverse order in the repro is kind of confusing. I kept going "Wait, where is this type coming from?" as I read through it. At one point I even suspected some of the types might be external.

[nmain]

Not related to the issue, but the types being defined in reverse order in the repro is kind of confusing. I kept going "Wait, where is this type coming from?" as I read through it. At one point I even suspected some of the types might be external.

My apologies. That was not coincidental but at the same time not intentional. I produced this example from a real world code that was faulting, and the way I did it was to copy and paste in first the failing lines, and then recursively grab any undefined types (and paste them below the existing lines, whoops), until I had it all. I'll update the example to flip everything around.

[typescript-bot]

The change between origin/release-5.0 and origin/release-5.1 occurred at 4fcb8b8.

[RyanCavanaugh]

Bisects to #51884, FYI @jakebailey @nebkat @weswigham in case you have any immediate takes

[nebkat]

This appears to be a very obscure unintended consequence of #43633/#43937 within getApparentTypeOfContextualType, only the scenarios in which it can occur have expanded since #51884.

Background

When discriminating between union constituents we consider not just the existence/value of the discriminator property but also the omission of properties that are required/optional in some constituents.

This is implemented by checking which of the constituent types allow optional values for discriminable properties which are not present in the object literal - that is whether [[constituentType]]#discriminableProperty is assignable to undefined.

Discriminable properties are known properties which have differing types or differing literal values amongst some of but not necessarily all of the union constituents. Index signatures do not generate any discriminable properties, but they don't prevent them from being made between other constituents either, the property just gets marked as "partial".

function discriminateContextualTypeByObjectMembers(node: ObjectLiteralExpression, contextualType: UnionType) {
        return getMatchingUnionConstituentForObjectLiteral(contextualType, node) || discriminateTypeByDiscriminableItems(contextualType,
            concatenate(
                map(
                    filter(node.properties, p => !!p.symbol && p.kind === SyntaxKind.PropertyAssignment && isPossiblyDiscriminantValue(p.initializer) && isDiscriminantProperty(contextualType, p.symbol.escapedName)),
                    prop => ([() => getContextFreeTypeOfExpression((prop as PropertyAssignment).initializer), prop.symbol.escapedName] as [() => Type, __String])
                ),
===>                map(
===>                    filter(getPropertiesOfType(contextualType), s => !!(s.flags & SymbolFlags.Optional) && !!node?.symbol?.members && !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName)),
===>                    s => [() => undefinedType, s.escapedName] as [() => Type, __String]
===>                )
            ),
            isTypeAssignableTo
        );
    }

Side note it might be good if some comments got added here to explain that this is doing property omission discrimination as it is not immediately obvious.

In the original example for #41759 this makes sense as the property is explicitly required in some union constituents but not others i.e. we can discriminate on the omission of disc below.

type DiscriminatorTrue = {
  disc: true;
  cb: (x: string) => void;
}

type DiscriminatorFalse = {
  disc?: false;
  cb: (x: number) => void;
}

Issue

However we now take a problematic example similar to the original post:

export type FooA = { foo?: 'a' }
export type FooB = { foo?: 'b' }

export type IndexSignatureType = {
    [key: string]: { bar: 'baz' };
}

export type Problem = IndexSignatureType | FooA | FooB;

const test: Problem = {
    1: {
        bar: 'baz'
    }
};

Link to playground

1. foo is taken to be a (partial) discriminable property because it allows differentiation between FooA, and FooB.
2. test does not define a foo property, so we can't differentiate on its value, but we can differentiate on its omission.
3. discriminateTypeByDiscriminableItems checks if [[type]]#foo is assignable to undefined for each of the constituents.
4. As foo is indeed optional in FooA/FooB, these pass the test. IndexSignatureType fails the test because [[type]]#foo is not defined, thus can't be tested.
5. Before Consider all union types matching discriminator for excess property checks #51884 this did not matter because the lack of a single matching constituent meant the entire union was returned anyway - even though IndexSignatureType was considered non-matching.
6. After Consider all union types matching discriminator for excess property checks #51884 FooA | FooB is returned and contextual apparent typing does not allow the object literal to take into account the expected object literal types...

---

Interestingly the problem can be replicated before #51884 by making FooA#foo non-optional:

export type FooA = { foo: 'a' }
export type FooB = { foo?: 'b' }

export type IndexSignatureType = {
    [key: string]: { bar: 'baz' };
}

export type Problem = IndexSignatureType | FooA | FooB;

const test: Problem = {
    1: {
        bar: 'baz' // Fails on 5.0.4: Type 'string' is not assignable to type '"baz"'. (2322)
    }
};

Link to playground

Here discriminateTypeByDiscriminableItems also incorrectly discriminates to FooB and causes the same problem as before.

Solution

I think the solution is either to skip omission based discrimination on "partial" properties altogether, or perhaps better yet to treat the index signature type as being optional and therefore passing the omission test.

Here is a diff for the former solution, which fixes the original issue without failing any tests.

diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
index 72d7f692f9..fa281c99b0 100644
--- a/src/compiler/checker.ts
+++ b/src/compiler/checker.ts
@@ -25491,7 +25491,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
         return false;
     }

-    function isDiscriminantProperty(type: Type | undefined, name: __String) {
+    function isDiscriminantProperty(type: Type | undefined, name: __String, skipPartial: boolean = false) {
         if (type && type.flags & TypeFlags.Union) {
             const prop = getUnionOrIntersectionProperty(type as UnionType, name);
             if (prop && getCheckFlags(prop) & CheckFlags.SyntheticProperty) {
@@ -25501,7 +25501,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                         ((prop as TransientSymbol).links.checkFlags & CheckFlags.Discriminant) === CheckFlags.Discriminant &&
                         !isGenericType(getTypeOfSymbol(prop));
                 }
-                return !!(prop as TransientSymbol).links.isDiscriminantProperty;
+                return !!(prop as TransientSymbol).links.isDiscriminantProperty && (!skipPartial || !((prop as TransientSymbol).links.checkFlags & CheckFlags.Partial));
             }
         }
         return false;
@@ -29358,7 +29358,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                     prop => ([() => getContextFreeTypeOfExpression((prop as PropertyAssignment).initializer), prop.symbol.escapedName] as [() => Type, __String])
                 ),
                 map(
-                    filter(getPropertiesOfType(contextualType), s => !!(s.flags & SymbolFlags.Optional) && !!node?.symbol?.members && !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName)),
+                    filter(getPropertiesOfType(contextualType), s => !!(s.flags & SymbolFlags.Optional) && !!node?.symbol?.members && !node.symbol.members.has(s.escapedName) && isDiscriminantProperty(contextualType, s.escapedName, true)),
                     s => [() => undefinedType, s.escapedName] as [() => Type, __String]
                 )
             ),

I will try to submit a PR later this week but I may be busy, feel free to proceed with the above if it is the right approach. Hope this helps!