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

Suggestion: Discriminate between union types based on extracted property #31870

Closed
5 tasks done
inversion opened this issue Jun 12, 2019 · 3 comments · Fixed by #44730
Closed
5 tasks done

Suggestion: Discriminate between union types based on extracted property #31870

inversion opened this issue Jun 12, 2019 · 3 comments · Fixed by #44730
Labels
Duplicate An existing issue was already created

Comments

@inversion
Copy link

inversion commented Jun 12, 2019

Search Terms

Discriminated union type guard destructuring destructured extracted assigned property

Suggestion

If the discriminating property of an interface is assigned to a const via destructuring or directly it is not usable as a type guard.

Use Cases

  • Code brevity

Examples

In the example I would expect bool to be usable as a type guard in functions f and g but it is not. Is there something I am missing here that would make this unsound?

interface A {

	bool: false;
}

interface B {

	bool: true;

	data: string;
}

type C = A | B;

function f(x: C) {

	const { bool } = x;

	if (bool) { // Error: Property 'data' does not exist on type 'C'. Property 'data' does not exist on type 'A'.

		return x.data;
	}
}

function g(x: C) {

	const bool = x.bool;

	if (bool) { // Error: Property 'data' does not exist on type 'C'. Property 'data' does not exist on type 'A'.

		return x.data;
	}
}

function h(x: C) {

	if (x.bool) {

		return x.data;
	}
}

https://www.typescriptlang.org/play/#src=interface%20A%20%7B%0D%0A%0D%0A%09bool%3A%20false%3B%0D%0A%7D%0D%0A%0D%0Ainterface%20B%20%7B%0D%0A%0D%0A%09bool%3A%20true%3B%0D%0A%0D%0A%09data%3A%20string%3B%0D%0A%7D%0D%0A%0D%0Atype%20C%20%3D%20A%20%7C%20B%3B%0D%0A%0D%0A%0D%0Afunction%20f(x%3A%20C)%20%7B%0D%0A%0D%0A%09const%20%7B%20bool%20%7D%20%3D%20x%3B%0D%0A%0D%0A%09if%20(bool)%20%7B%0D%0A%0D%0A%09%09return%20x.data%3B%0D%0A%09%7D%0D%0A%7D%0D%0A%0D%0A%0D%0Afunction%20g(x%3A%20C)%20%7B%0D%0A%0D%0A%09if%20(x.bool)%20%7B%0D%0A%0D%0A%09%09return%20x.data%3B%0D%0A%09%7D%0D%0A%7D

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@inversion inversion changed the title Discriminate between union types based on extracted property Suggestion: Discriminate between union types based on extracted property Jun 12, 2019
@jack-williams
Copy link
Collaborator

Tracked here #12184

@ahejlsberg ahejlsberg added the Duplicate An existing issue was already created label Jun 12, 2019
@fatcerberus
Copy link

Is there something I am missing here that would make this unsound?

No, but this in general is difficult from an implementation standpoint - the compiler would have to track interactions between different variables. Basically right now the control-flow analyzer only considers what the expression inside if (...) implies for the variables directly involved in that expression. At that point it doesn't really know where, e.g. your bool variable actually originated from.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants