Skip to content

Type assignable to 1st but not 2nd param to Exclude isn't assignable to Exclude result #51793

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
wbt opened this issue Dec 6, 2022 · 9 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@wbt
Copy link

wbt commented Dec 6, 2022

Bug Report

🔎 Search Terms

exclude assignable union

🕗 Version & Regression Information

  • This changed between versions 3.9.7 and 4.0.5, at least in terms of adding incorrect broadening of ‘swims’ to ‘string’ in the last error message

⏯ Playground Link

Playground link with relevant code

💻 Code

type AttributeMap = {
	Animal: { swims: boolean; expectedLifespan: number; };
	Robot: { swims: boolean; serial: string; };
}
type Mover = keyof AttributeMap;
type IrrelevantAttribute = 'flies' | 'slithers';
//In this example, there is no overlap between the two unions passed to Exclude (regardless of M),
//so the Exclude type shouldn't be making any difference and testExcludeLeft
//should always have the same type behavior as testExcludeResult, but it doesn't.
//Exclude (https://www.typescriptlang.org/docs/handbook/utility-types.html#excludeuniontype-excludedmembers):
//Constructs a type by excluding from UnionType all union members that are assignable to ExcludedMembers
type RelevantAttribute<M extends Mover> = Exclude<keyof AttributeMap[M] & string, IrrelevantAttribute>;
declare function testExcludeLeft<M extends Mover>(m: M, demo: keyof AttributeMap[M] & string): any;
declare function testExcludeRight<M extends Mover>(m: M, demo: IrrelevantAttribute): any;
declare function testExcludeResult<M extends Mover>(m: M, demo: RelevantAttribute<M>): any;
export const bugDemo = function<M extends Mover>(m: M) {
	const attributeName = 'swims' as const;
	testExcludeLeft(m, attributeName); //fine, as expected
	testExcludeRight(m, attributeName); //should be & is an 'is not assignable' error:
	//Argument of type '"swims"' is not assignable to parameter of type 'IrrelevantAttribute'.ts(2345)
	testExcludeResult(m, attributeName); //should be fine but isn't, 'not assignable' error:
	//Argument of type 'string' is not assignable to
	//parameter of type 'Exclude<keyof AttributeMap[M] & string, IrrelevantAttribute<M>>'.ts(2345)
	//Why is the argument of type 'string' when calling testExcludeResult 
	//but '"swims"' when calling the others?
}

🙁 Actual behavior

In the call to testExcludeResult in TypeScript 4+, the second parameter is broadened to 'string' which differs from the two calls above.
That second parameter, though assignable to the union in the first parameter to Exclude and not assignable to the second, is not assignable to the type constructed with Exclude.

🙂 Expected behavior

Per documentation, Exclude "constructs a type by excluding from UnionType all union members that are assignable to ExcludedMembers" so a type demonstrably assignable to the first argument but not the second should be assignable to the constructed type.

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Dec 7, 2022
@RyanCavanaugh
Copy link
Member

While true, the argument used to make this valid only works in cases where you didn't need the generic in the first place - specifically, you could have written

type SharedKeys = keyof AttributeMap[keyof AttributeMap];
declare function testExcludeResult<M extends Mover>(m: M, demo: Exclude<SharedKeys, IrrelevantAttribute>): any;

without any difference in generality.

@wbt
Copy link
Author

wbt commented Dec 8, 2022

The generic is actually needed within the function, to help limit typing to functions that calls. Using function declarations where the generic affects the return type as follows shows an example, though the simpler version above still shows the violated expectation about Exclude. (Of these three, the last one is most similar to the inspiring example, but the ones before that are helpful in demonstrating that the issue is with assignability of a type constructed with Exclude and not in some other aspect of those type definitions.)

declare function testExcludeLeft<
	M extends Mover
>(m: M, demo: keyof AttributeMap[M] & string): AttributeMap[M];
declare function testExcludeRight<
	M extends Mover
>(m: M, demo: IrrelevantAttribute): AttributeMap[M];
declare function testExcludeResult<
	M extends Mover, A extends RelevantAttribute<M>
>(m: M, demo: RelevantAttribute<M>): AttributeMap[M][A];

The actual types in the motivating example are further derived from the type constructed with Exclude, and the generic parameter is needed there too - some of the places where the constructed type is used use attributes other than those which are shared in common (in this example across all Movers). It needs to work for attributes that are shared in common just as much as those that aren't, and this example of it not working for an attribute that is shared in common is puzzling.

It's somewhat discouraging to see how many posts have reports of very unexpected behavior in TypeScript which goes directly contrary to documentation, and even where responses acknowledge incorrectness of the existing behavior, there's a general message being received of "you should just expect TypeScript to not behave in a consistent, predictable, or as-documented manner because making it behave that way is too hard."

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Dec 8, 2022
@wbt
Copy link
Author

wbt commented Dec 9, 2022

Why is this closed as not planned, despite Exclude not working as documented?

@RyanCavanaugh
Copy link
Member

@wbt where in the documentation does it specifically say that this kind of example should work?

@wbt
Copy link
Author

wbt commented Dec 9, 2022

The documentation says that Exclude "constructs a type by excluding from UnionType all union members that are assignable to ExcludedMembers."

Here we have a type which is assignable to UnionType but NOT assignable to ExcludedMembers, so it should be assignable to the type constructed with Exclude, but it isn't. That seems pretty clearly to be a bug.

It's also not clear that the bug is in the definition of what's assignable to Exclude; the bug could be in how the type narrowing fails to stick and narrow 'swims' becomes broader 'string' for the purpose of that specific assignability check.

@RyanCavanaugh
Copy link
Member

What you're basically expecting here is that Exclude<T, U> works like the intersection T and Not U. But that's not what it is and the type system can't make conclusions based on that higher-order observation. It's just a conditional type and a conditional type instantiated with a type parameter is always kept in a deferred form, and the deferred form is almost never something that we can make sound conclusions about.

Feel free to send a PR to fix this, though!

@wbt
Copy link
Author

wbt commented Dec 9, 2022

What you're basically expecting here is that Exclude<T, U> works like the intersection T and Not U. But that's not what it is

The documentation should be updated to reflect that; might save folks a lot of frustration. I'm not sure what to update it to though. If that's not the idea Exclude should express, what is?

Would it be accurate to have the documentation say something like "Exclude<T, U> works like the intersection T and Not U unless the types T or U contain a type parameter, in which case it won't work because conditional types such as Exclude are kept deferred if they contain a type parameter and thus can't be reasoned about in the current state of TypeScript development"?

Feel free to send a PR to fix this, though!

There seems to be little chance of anything like that being accepted if submitted by me, or provided by anyone else more familiar with the codebase, if the issue it addresses was closed, especially "not planned!"

@RyanCavanaugh
Copy link
Member

The documentation should be updated to reflect that

The documentation provides a straightforward presentation of the definition of the type suitable for a lay person to read it and come away with a basic understanding of what it does. It can't possibly contain every variant of "You might think it actually does X, but it doesn't"; that's effectively what the issue tracker is for.

There seems to be little chance of anything like that being accepted if submitted by me, or provided by anyone else more familiar with the codebase, if the issue it addresses was closed, especially "not planned!"

Sometimes we think issues aren't fixable due to fundamental limitations. In this case it makes sense to close the issue -- it would make no sense to have tens of thousands of open issues around "TypeScript doesn't solve the halting problem" and "TypeScript can't be used to prove the correctness of the Collatz conjecture". That's what 'closed' means.

If it turns out we were wrong and someone is able to fix it, of course we'd accept a PR for it. We're not trying to actively make the product worse.

@wbt
Copy link
Author

wbt commented Dec 12, 2022

The documentation provides a straightforward presentation of the definition of the type suitable for a lay person to read it and come away with a basic understanding of what it does. It can't possibly contain every variant of "You might think it actually does X, but it doesn't";

The documentation does contain other examples of "the oversimplified description above might lead you to think X but it doesn't actually do that" and if the documentation is intended for a layperson to get a general sense of how it works instead of intended for a technical person to actually be able to figure out how to use it, that's a problem.

It leads to situations where executives can read the documentation and get a general sense that converting a codebase to TypeScript is pretty easy and can be done progressively simply by adding type annotations and fixing any actual typing errors (not true: TS conversions can actually require a lot of refactoring especially when commonly used types aren't expressible in TS and/or in the face of the many known issues especially in narrowing and assignability); utility types like these will of course just work and any developer who can't make that happen is clearly just incompetent because the documentation states what to do so clearly...per documentation, this isn't hard and shouldn't be slow.

The documentation should be aimed at developers trying to make use of TS and understand what these types do and how they work. Currently, the documentation is actively misleading by telling developers the Exclude type does something it doesn't do. There's a difference between documenting exceptions to every possible variant of everything a person might possibly think a type does and documenting exceptions to what the documentation clearly says it does.

that's effectively what the issue tracker is for.

The documentation is already poorly organized by requiring readers to potentially go through every part of every "What's New" to find any summary documentation that may exist for some type or operator; implying that every TS user should have to read through all the comments on all the open AND closed issues is not a realistic or feasible expectation. By GitHub's default (a poor choice IMO), searching Issues excludes those which are closed. Therefore, even if someone has this issue, knows to ignore what the official documentation says so clearly and use the issue tracker instead, and they know exactly which search terms to use, they probably still won't find it.

The odds of me being the last person to waste a lot of time on this seem very slim, and efforts to help others in the community avoid a similar experience seem to be undesired. I don't think that's good for the long-term health of the ecosystem, as these experiences tend to drive people away and make future recommendations against using TS.

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

2 participants