Surprising excess property check with recursive generic constraint

🔎 Search Terms

excess property check recursive generic contraint type variable inference

🕗 Version & Regression Information

This is the behavior in every version I tried

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.3.0-dev.20230906#code/C4TwDgpgBAysCGwIwMYAsIFt5QLxQG8AoKKAZwSTIH4AuKAJQhQHsAnAEwB4K2BLAHYBzADSxKydFngA+ANxEAvgqIdmAG3htoAMwCuAlMD4sBUFNsQQAsvHSCIXACoBhUzr5CoEAB5IBHGTiVqgY2DIAFCTm7p70rrGiRACU8W4CHkIqFhBWtvYCEBHEpBACwGwg9ADaBFCgkPQARDosLE1QigC6ItEUVmT0JaRQ8EPRI97llTV1DRDNAEZaHd29k4rrnb2KyQpAA

💻 Code

type StateSchema = {
  states?: Record<string, StateSchema>;
};

declare function createMachine<TConfig extends StateSchema>(
  config: TConfig,
): TConfig;

createMachine({
  entry: [{ type: "foo" }],
  states: {
    a: {
      entry: [{ type: "bar" }],
    },
  },
});

🙁 Actual behavior

At the nested entry property the error is raised:

Object literal may only specify known properties, and 'entry' does not exist in type 'StateSchema'.(2353)

but it isn't raised at the entry property at the root of this object.

🙂 Expected behavior

I'd expect no excess property error to be raised with a recursive constraint like this.

Additional information about the issue

This particular case is somewhat easily fixable by adding [k: string]: unknown to StateSchema. I still find the reported behavior to be a problem though.

I think this is actually a weak type assignability thing being reported as an excess property check thing after inference fails. You can see the common properties check by moving the assignment out to a variable declaration, and you can also see make it work in inference by adding a states: undefined property to a. https://www.typescriptlang.org/play?ts=5.3.0-dev.20230906#code/C4TwDgpgBAysCGwIwMYAsIFt5QLxQG8AoKKAZwSTIH4AuKAJQhQHsAnAEwB4K2BLAHYBzADSxKydFngA+ANxEAvgqIdmAG3htoAMwCuAlMD4sBUFNsQQAsvHSCIXACoBhUzr5CoEAB5IBHGTiVqgY2DIAFCTm7p70rrGiRACU8W4CHkIqFhBWtvYCEBHEpBACwGwg9ADaBFCgkPQARDosLE1QigC6ItEUVmT0JaRQ8EPRI97llTV1DRDNAEZaHd29k6QA9JvkEoNQBmoehRwTnevnSskq6hDAUD54hNFlFVVQtfXgC1Atbas9Pp7caTMbPDZTN6zL6NX7LNgAi6kRQXFFKG53KDvOAhKTYJ4+BRAA

I think the bug here is that the error message is wrong.

This might be the case, I see that - indeed - the excess property check fails in getSignatureApplicabilityError. The compareTypes call fails in getInferredType and thus the successfully inferred type ({ entry: { type: string; }[]; states: { a: { entry: { type: string; }[]; }; }; }) gets replaced by the constraint. This step might fail cause of the weak type thing. The question is - should it fail at this point if we are inferring? It becomes increasingly challenging to infer a recursive structure like this - at some point, the states must be missing since the runtime structure itself isn't infinitely expanding (and is not meant to contain cycles 😉 ).

What you’re asking is whether this code should be error-free:

interface Options {
  x?: boolean;
  y?: string;
  z?: number;
}

declare function f<T extends { options?: boolean }>(obj: T): T;

f({
  opitons: {
    x: true,
    y: "hello",
    z: 42
  }
})

https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgPIAczAPYgM7IDeAUMsgB4D8AXMgEbbYA2EcIA3KcgJ43J5gooAOacyALz4gArgFs60TgF9ixACYQETOFBQxpIBFlzIYAHgAqyCOUgg1BQsmyYc+Pg2asQyJQD4ACmw6ACtaCwBKcM5iGACSMhdgMFw8WgSyClpBaQgAGi4ybloAIgALCCYmbBKCzORxWgAWACYuFSUI4iA

It feels to me like the mistake that common property checks are intended to catch is just as relevant in inference as in other assignability checks.

Yee, so I guess what I'm most focusing on here is the recursive aspect of my constraint. I know though that distinguishing those situations might be rather hard. This is just the most reduced repro case of my problems. On its own, I don't mind this as much. I created a linked issue to this one that shows better why this is a problem. The intersection thing there changes everything though - it's just that the root problem is similar. The compareTypes check in getInferredType happens in isolation from other things. It just compares the inferred type against the constraint (so it fails on the common property checks) but it doesn't account for the fact that this ends up being intersected so the final result actually doesn't suffer from either common property errors or the excess property errors.

I think though that some issue has been identified here - the error is somewhat confusing here. It doesn't report why this failed in practice. Inferred object types are exempt from the excess property checks because they are widened in getCovariantInference.

I tried reordering the excess property check with the common property check but that didn't pan out. The common property check embeds rename suggestions and stuff like that. Those are much more helpful than common property errors and thus they should be prioritized in simple situations.

So my best ideas to fix this are:

maintain a set of instantiatedConstraints that had to be assigned to inferredType on compareTypes(inferredType, instantiatedConstraint) failures. Then based on this information return false from isExcessPropertyCheckTarget. This could get a little bit hairy when it comes to the actual implementation since this set would have to be somehow maintained, cleaned up/disposed and passed around (or be kept among global~ variables).
in the case of the same failure - set a flag on inferenceContext, tweak argCheckMode based on this here and then call getRegularTypeOfObjectLiteral on the argument here. This sounds much simpler. However, it sounds like it would have a big potential of suppressing too many errors. I also prototyped this locally (some extra work had to be done beyond what I mentioned above) - but it still didn't yield the expected results. elaborateElementwise wasn't aware of my tweaked argCheckMode anyhow and thus it continued to operate on a fresh literal.
Based on seeing what elaborateElementwise does... maybe it would just be the easiest to set some information in node links of the value declaration of the inference candidate that got widened. This way I could maybe target it precisely in isExcessPropertyCheckTarget. Object types themselves don't really have symbols and thus they are not linked to value declarations - so I'm not sure if that route could bring us closer to the solution. OTOH, an empty object type can't report an excess property check so perhaps using symbol properties for this would be enough.

After a second consideration, maybe the error message shouldn't even get fixed here. See my comment here. If we think about inferred type arguments as "assignments" then yes - this should very much fail at that stage. The error message is somewhat surprising because that assignment is just hidden from us so we don't see it.

The error that is reported today isn't incorrect - it just feels off a little bit. If we disassociate argument type checking from the implicit type argument assignments... then it stop being so weird 😅 What I'm saying, is perhaps it just doesn't matter why the inferred candidate was discarded. All that matters is the relation between what got finally inferred (the instantiatedConstraint here) and the argument type.

Navigation Menu

Repository navigation

Surprising excess property check with recursive generic constraint #55644

🔎 Search Terms

🕗 Version & Regression Information

⏯ Playground Link

💻 Code

🙁 Actual behavior

🙂 Expected behavior

Additional information about the issue

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Surprising excess property check with recursive generic constraint #55644

Description

🔎 Search Terms

🕗 Version & Regression Information

⏯ Playground Link

💻 Code

🙁 Actual behavior

🙂 Expected behavior

Additional information about the issue

Activity

andrewbranch commented on Sep 7, 2023

Andarist commented on Sep 7, 2023

andrewbranch commented on Sep 7, 2023

Andarist commented on Sep 8, 2023

Andarist commented on Sep 8, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions