Skip to content
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

Incorrect type inference on union type, unless using a third property as a discriminant #57758

Closed
FrenchDilettante opened this issue Mar 13, 2024 · 5 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@FrenchDilettante
Copy link

πŸ”Ž Search Terms

"inference" "inference narrow" "inference discriminant" "inference union"

πŸ•— Version & Regression Information

Tested on multiple versions, this is not a new bug.

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.4.2#code/C4TwDgpgBA6gTgewHYHMCSSBmE4SQY2gF4oBvAKAEgBbAVwBtgBLMeiALimDlogG4qANwCG9XpwDO3JqgDaAXQGV8o+gCNh+ANacAFAH1J0ufICUUIgD4oghEwAmAgL5QAPmSp1GLNp0yiJfiFRcSgpOBkUJRV6dU0dKAMjCNRzKxs7R3InAXJMWgJmZCg2YABVCQxsOF1hTnhkdCwcPEJzCkphADoYuO1arpExCFMlJkxE7q9mVhGPSk6e1Q1+7qHeUaoXCHpA+cXela0B9ZGlJ2zyIA

πŸ’» Code

type Selection = {
	multiple: true;
	value: string[];
	callback: (_: string[]) => void;
} | {
	multiple: false;
	value: string;
	callback: (_: string) => void;
};

function test(a: Selection) {
	// Error: Typescript does not recognise that `value` and `callback` use the same type.
	a.callback(a.value);
	if (a.multiple) {
		// Valid: Typescript narrowed down the type to `string[]`
		a.callback(a.value);
	} else {
		// Valid: Typescript narrowed down the type to `string`
		a.callback(a.value);
	}
}

πŸ™ Actual behavior

Typescript does not recognise that callback will always accept value as a parameter, since they both use either string or string[].

Interestingly, Typescript can infer the types properly using the if (multiple) { ... } condition.

πŸ™‚ Expected behavior

All three callback(value) calls should be valid, from a type point of view.

Additional information about the issue

No response

@MartinJohns
Copy link
Contributor

Essentially a duplicate of #30581. Not sure why that is closed.

@Andarist
Copy link
Contributor

Andarist commented Mar 13, 2024

It's probably closed because it has been determined that it would be too computationally expensive to do it and the current best solution is to use the "indexed mapped type as a union" pattern from TS 4.6 (see here).

However, this discriminant value isn't easily used as a property key so while we can make the internal implementation of a function happy about it... it still doesn't quite infer a concrete K that is used and it doesn't discriminate the input argument for contextual parameters etc: TS playground

Using some intersections we can fix the latter: TS playground. It still doesn't infer K though and I'm not sure how to make true/false a viable inference source for "true"/"false"

@Andarist
Copy link
Contributor

kudos to @ssalbdivad: TS playground

it's not particularly intuitive how to put all the pieces together but this one works very well

@fatcerberus
Copy link

It looks like #30581 was auto-closed when #47109 got merged.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Mar 15, 2024
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Design Limitation" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

6 participants