Skip to content

Inferred type changes from 5.5 to 5.6 #60077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
danvk opened this issue Sep 26, 2024 · 5 comments
Closed

Inferred type changes from 5.5 to 5.6 #60077

danvk opened this issue Sep 26, 2024 · 5 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@danvk
Copy link
Contributor

danvk commented Sep 26, 2024

πŸ”Ž Search Terms

  • variance

πŸ•— Version & Regression Information

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.6.2#code/CYUwxgNghgTiAEYD2A7AzgF3gMwFzwB4AFAPgAoAoeHFfMgBxiXrXyIEp4BeE+ANyQBLYABoq8QSkEYA-GzGce8IgG4KFSRhAxsUMAiJMW8AN7i4AR3yYYkgOZrqzWdYy2UDigF91ydFkZmNG4cSmoGNiM0RV4AclixahN4S3wAIjT4LwU1CkCWADpnCgB6EvgAURgmGAkUeAAVAGV4AFYCgDYReBQkLEl4EFgIQW1+bTRBVDQKIA

πŸ’» Code

declare const f: <P>(
  fn: (props: P) => void,
  init?: P,
) => P;

interface Props {
  req: string;
  opt?: string;
}

const props = f(
  (p: Props) => '',
  { req: "" },
);

props.opt
// Error in TS 5.6, not in earlier versions

πŸ™ Actual behavior

P is inferred as {req: string}, so accessing the optional property causes a TypeScript error.

πŸ™‚ Expected behavior

P should be inferred as Props, as it is in TS 5.5 (and earlier). If you drop the init parameter from the call to f, then P is also inferred to be Props in TS 5.6.

Additional information about the issue

This seems related to #59764 but I'm not sure it's the same, so I figured I'd file an issue. I tried the same code on the playground for #59709 and it does not fix this issue.

cc @Andarist for whether TS 5.6 is right and I'm wrong πŸ˜„

@Andarist
Copy link
Contributor

This seems related to #59764 but I'm not sure it's the same

I think this boils down to the same issue and I would refer to Ryan's comments there: #59764 (comment) and #59764 (comment)

Those are just heuristics and there are no perfect answers to which type should be preferred. Some cases perform better with one candidate picked and some cases perform better with the other one picked.

I think that especially the case presented by Ryan in the first linked comment sways this way more in favor of the covariant candidate. It's way more likely that the object's shape available at runtime will have the properties coming from a covariant argument position than those coming from a contravariant parameter.

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Sep 26, 2024
@RyanCavanaugh
Copy link
Member

I agree with @Andarist that I was correct in those comments πŸ™‚

@danvk
Copy link
Contributor Author

danvk commented Sep 26, 2024

OK, I can see I'm outgunned here. πŸ™‚

FWIW, I do think it's a bit surprising that this wasn't mentioned in the release notes given that it can cause confusing changes when you upgrade β€” I only found the PR via every-ts bisect.

@danvk danvk closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2024
@danvk
Copy link
Contributor Author

danvk commented Sep 27, 2024

Though I guess looking at #59764 (comment) a little more closely, one difference is that in that case, the widened type of defaultT isn't assignable to Foo (at least with excess property checking). Whereas in my example the type of init is assignable to Props, so that seems like a more natural type to infer.

I can see that there's no perfect answer in Ryan's case, but it seems there is here.

@Andarist
Copy link
Contributor

That is a difference that I implicitly talked about here. Honestly, I don't know how to think about this in the TS's structural type system since there are tradeoffs to both approaches

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

3 participants