Skip to content

Declaration merging can break equality tests for named tuples #61162

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
LukeAbby opened this issue Feb 11, 2025 · 12 comments
Closed

Declaration merging can break equality tests for named tuples #61162

LukeAbby opened this issue Feb 11, 2025 · 12 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@LukeAbby
Copy link

🔎 Search Terms

named tuples, equal, unequal

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ

This happens in typescript@next, typescript@latest, and all the way back to [email protected] (the earliest version that has tuples in the playground).

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/PTAEBUAsEsGdQKYEcCuBDANtALgT1AMaQIEDWochA9gLY1UB2G+KsCAJhQ6FgEYBOaftASwAdKACS2AOTxWHULxJoFFWfDShY2YQWwJ+2hAzagqAM0SpMOfNkhpuNFNnQZQaWLGgBzBmi80Fh4YgBQIFKgAA6GsIyYzKCC3NAM2FTqoA6UaZ7JCNGZrGm+oAAGAG44otjl4XixoACiNhiwADzgADSgAKoAfKAAvGGgoAAUHQBqAxMAlCND04gAHgYM7PDgoAD82fwoCKAAXKAWmGyLCOsmW5Mzc4vDy2sb9317B0en55cI8zG4y+uiOQPGZwu7QQAG4wmFGsdwLU+gwAjQOOAUNEMKIRi02p0ANo6YQMXwAXV6JN0pQpAzhkXGAD1vggImAoLl4PwEFhAklQQhwpEufBKFhSHz8MoCKozFo3DiEHJQOjjpQnBVJOwTNhoBYRPxyp5NvlokJsOYrA5jnKiMcpbgRWAAEIqNS2hW80AoNFoDGcW34aL8KixfhJXwIK0OKgoXyQK2WTzi3XpaByjASCZQJykeAAQU2QjgVosVCMOXghs2AEJAQjcE0APLRfUJDBY5X4okWwQ0XZnUl0uFNprInTgZsIQv82BtjsBLvY3HwYYE9ydRfQTvd3G9Hd71cIBkc4GsoXnsUUHl86AC+yHWHZYjZGcUe5CY6YADuaFweBkHccJxyRWp9zxDdWi3Do+yEANh1pckqVAeCByQslKTPJlQFZKE2HPABlSB4wwThlAqIV6nhREIFqI9l0g9dN0uOD+wDIdtGQylqQ4wdMLpHCwBZP5oWI0iUHIpRjnKajQJuIp+CtABvABfMddQIDBv1AXwMCoXhMFAFTwS4Ax+AuAhjkLfhBFwLohlM4EXNAUU33VeB2EyBgqCtGg0GwCzwlc4FIniDEAFkY1I9gOhbOY-OIfgzmadZBH0eLenAAZ5jOIk0t0NBMp6UAEopGE3LAIj-2iaJSnMVxzkrV9KACqV4EYJJwyXYylTXagaH7Y4-WAxJnTMlyIoQaK4zihKJiSwwzhbN47m2L5VrOJxcDytCdhud54FW-Ydt+HaKqqiAYHgdq8V4JLzHbXdl1NThfIYABaHqXr6k94AIWhht9BgxowZgQuBNSwjUoA

💻 Code

// This equality check is commonly used in libraries. It's used because it's a stricter sense of equality than mutual assignability.
// I personally ran into it this in a repo using `vitest`.
type Equals<T, U> =
  (<V>() => V extends T ? true : false) extends (<V>() => V extends U ? true : false)
    ? true
    : false;

type TestUnnamedTuples = Equals<[string], [string]>;
//   ^ true
// This is reliably true.
// This is likely because a tuple's name is an `Identifier` and a part of the cache key.
// Because these are unnamed they properly get thought of as identical. (Thanks Andarist for this find!)

type OptionalTuple = [param?: string];

type TestTypeAliasOptionalTuples = Equals<OptionalTuple, OptionalTuple>;
//   ^ true
// This is reliably true; the type ids are always equal.

type TestTuples = Equals<[param: string], [param: string]>;
//   ^ false
// Should be `true`.

type TestOptionalTuples = Equals<[param?: string], [param?: string]>;
//   ^ false
// Should be `true`.

export {};

declare global {
    interface Array<T> {
        // The names do not matter.
        // someMethod<O>(other: Extract<O, T>): [Extract<T, O>]; // Swapping out for this makes only optional tuples compare unequally.
        someMethod<O>(other: O extends T ? O : any): [T extends O ? any : any]; // This makes both optional and non-optional tuples compare unequally.
    }
}

🙁 Actual behavior

TestTuples and TestOptionalTuples are false whereas TestTypeAliasOptionalTuples and TestUnnamedTuples are true.

🙂 Expected behavior

For all of these tests to return true.

Additional information about the issue

While this someMethod merge is obviously contrived, I actually ran into this while typing a library that adds an Array.equals and Array.partition method.

After hunting it down I simplified the types to find this case.

@jcalz
Copy link
Contributor

jcalz commented Feb 11, 2025

Not a TS team member.

Looks like the TS team isn't really interested in supporting that Equals check, which probes implementation details of TS:
#57918 (comment)
#58163 (comment)

Two types that pass the Equals check are almost certainly equal, but two types that fail the check might well be "equal", they might just not share a single internal reference. (Like how in JavaScript one should not be surprised that, say, {} !== {}.) Maybe if there's some trivial fix that doesn't degrade compiler performance they'd accept it, but generally speaking they'd mostly just discourage you from using that check in the first place.

@LukeAbby
Copy link
Author

While I'm sympathetic to that point of view, the issue with the other mutual assignability test defined as type Equals<T, U> = [T, U] extends [U, T] ? true : false is that it has a number of adverse cases:

Equals<{}, { x?: number }> // true
Equals<any, 1> // true
Equals<{}, object> // true
Equals<void | undefined, void> // true

When a type testing library is involved I'd say you probably don't want any of those to count as equal. If you want to deal with them what you'll need to do is do a recursive walk of the type (properly bailing on loops in the type!) and looking for these edge case types and then special casing them.

While not impossible they're definitely not performant either.

@jcalz
Copy link
Contributor

jcalz commented Feb 12, 2025

So then maybe we want upvotes and feedback on #48100 asking for a better type level equality operator, since neither mutual assignability nor reference equality does the trick, and user-defined versions are complex and slow. Maybe an argument could be made there that in the absence of a reasonable native operator, too many people are choosing to use an unsupported/unsupportable approach.

I thought the main problem is that there are too many edge cases where people disagree on what equality is.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Feb 12, 2025
@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" 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 Feb 15, 2025
@HansBrende
Copy link
Contributor

HansBrende commented May 7, 2025

The only way to frame this issue to have it taken seriously is to show how it breaks assignability, not equality. Any mention of "equality" will seriously decrease the chances of a fix (case in point: this issue was closed without even being looked at simply because you referenced the equals hack as motivation).

General principle: any type that is copy-and-pasted from one place to another (with the exception of unique symbol types) should be assignable to itself.

So how does this issue manifest in a breakage of assignability? We can reframe it as follows:

declare global { interface Array<T>{ m<O>(other: O extends T ? O : any): [T extends O ? any : any] }}

let a: <T>() => T extends [param?: string] ? 0 : 1 = null!;
let b: <T>() => T extends [param?: string] ? 0 : 1 = null!;

a = b; // ❌ Error!

TS2719: Type
() => T extends [param?: string] ? 0 : 1 is not assignable to type
() => T extends [param?: string] ? 0 : 1. Two different types with this name exist, but they are unrelated.

Expected outcome: these two types should be assignable to each other, because they are syntactically identical and don't contain any unique type.

Actual outcome: the compiler incorrectly complains that the two types are unrelated.

I'd recommend opening a new issue without using the word "equals" anywhere, if you want this fixed.

@LukeAbby
Copy link
Author

LukeAbby commented May 7, 2025

I raised Equals because I care about this issue in the context of type testing. I don't know how attempting to reframe it as purely about assignability helps beyond assuming maintainers engage with anything that even mentions Equals in bad faith and also are oblivious to some slight of hand reframing.

On the contrary I've seen plenty of issues that mention Equals still get fixed as long as the maintainers view it as not at the heart of the issue or important enough and not going to regress something somewhere else. Apparently I have not indicated the importance of this issue enough and in fairness it involves a declaration merge into Array which is not best practice. In fact I would prefer it didn't exist, I'm simply typing some JS code. I can accept that it's not a priority.

Besides saying that assigning a: <T>() => T extends [param?: string] ? 0 : 1 to b: <T>() => T extends [param?: string] ? 0 : 1 fails really is just saying Equals returns false in a trenchcoat. These two things are completely homomorphic and it's patently obvious to anyone who knows Equals why you're suddenly raising the assignment of these particular generic functinos.

I understand that the TypeScript team is not willing to chase a definition of Equals to the infinite beyond in order to try to catch as many thorny cases as possible. See #60140, #56721, #57918 (comment), #57117 (comment) as well as many others. Specifically the phrase "internal implementation details are subject to change at any time. Equals may evaluate to true but there are no guarantees on this -- it's dependent on whether or not we choose to intern a given type" applies here and so I've simply accepted the issue being closed.

Of course I do disagree with the issue being closed. I believe the practical reality is that Equals as a type fulfills a very necessary niche. I do testing for my types because I care about quality assurance in the libraries I write and one of the things that implies is that I don't want string and any comparing equally, nor { x?: T } and {}, nor () => string and (() => string) & (() => number), nor any number of other things "equal" according to the barebones reflexive equals operator that it seems TypeScript is willing to support.

Now I do think something should be done, after all the performance of an alternative Equals operator without "hacks" that recursively walks these types to special case such problematic situations is abysmal and writing it to robustly avoid loops is rather challenging. But I'm not going to force TypeScript to improve the Equals we already have.

@HansBrende
Copy link
Contributor

HansBrende commented May 7, 2025

@LukeAbby It's not slight-of-hand, it's proof that an issue exists in its own right with non-internal implementation details, independently of this particular "equals" hack. Which is not obvious to a maintainer if the issue is framed only in terms of "equals" and does not demonstrate why it causes an assignability problem. But if you don't care about it anymore, that's fine too 🤷‍♂

Aside: it's funny that you should mention #57918 since the "equals" issue I mentioned there was fixed and merged after I reframed it in terms of assignability (see #61098).

@LukeAbby
Copy link
Author

LukeAbby commented May 7, 2025

My point was that since assignability and extends are two sides of the same coin, I think I'd get the same reaction. To engage in a hypothetical, if I say that I'm surprised that type Foo = {} extends { prop?: string } ? true : false is true I think I'd get the same reaction as reframing it by saying:

declare let x: {};
declare let y: { x?: string };

x = y; // Ok

Because I'm really saying the same thing as that's the whole point of the extends operator; to be part of a type-level ternary where the conditions are assignability tests. It just turns out that the assignability of conditional types with uninstantiated generics happens to expose internal implementation details about what exactly "equality" is that fails here. In fact I know why, it's because two named tuples contain identifiers with different line and column numbers and erroneously become a part of the identity check.

For example you'd see that this works:

declare global { interface Array<T>{ m<O>(other: O extends T ? O : any): [T extends O ? any : any] }}

type NamedTuple = [param?: string]

let a: <T>() => T extends NamedTuple ? 0 : 1 = null!;
let b: <T>() => T extends NamedTuple ? 0 : 1 = null!;

a = b; // Ok

And this functions the same with extends. Ultimately this is because the internal implementation that this uses can fall back to type id equality. Both snippets expose the isTypeIdenticalTo function which @RyanCavanaugh seems to consider a mistake given that they say "introduced [Equals] into the wild before we noticed and could un-expose it without breaking a ton of people" in #57918 (comment).

@HansBrende
Copy link
Contributor

@LukeAbby you realize that I was part of that whole conversation you are referring to... right? And the very issue I was complaining about (the subject of that conversation) was fixed & merged a few days ago after I reframed it in terms of assignability in #61098.

@LukeAbby
Copy link
Author

LukeAbby commented May 8, 2025

I noticed the edit after posting. I considered making an edit too but I don't feel it diminishes my point. Nowhere did RyanCavanaugh say "we won't look at a PR for this." The original issue is still open. I consider this issue being closed to mean that they, at a minimum, won't consider implementing a PR themselves but may look at a PR if I open it.

@RyanCavanaugh
Copy link
Member

but may look at a PR if I open it.

No guarantees, since it's not in the Backlog milestone. Experiment PRs that try to demonstrate something can be done with no ill effects are useful evidence, but we would like changes to the language to be motivated by use cases, not availability of code edits.

@RyanCavanaugh
Copy link
Member

the compiler incorrectly complains that the two types are unrelated.

While in the trivial case of two types that are textually equivalent this seems like an obvious case for a "bug" label, in general the assumption is that any two types are unrelated, and assignability has to be proven by some known path.

It is always going to be possible to construct an assignment that "should" pass but fails, the question in practice is simply what other code are you willing to break (via introducing new circularity errors) or slow down (by doing new checks that don't go anywhere) or both (in the case of touching Equals's behavior) in order to figure out that the assignment is valid. Unfortunately it's just trade-offs all the way down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

5 participants