fix: IsEqual, {a: t, b: s} and {a: t} & {b: s} are equal

[taiyakihitotsu]

Current: #1338 (comment) -> #1338 (comment) -> #1338 (comment) -> #1338 (comment)

---

This PR is separated from #1336.

(See #1336 (comment))

Major Changes

The previous (meaning before this PR) IsEqual<{a: 0} & {b: 0}, {a: 0; b: 0}> returns false.
This PR prevents it to return true via SimplifySimplifyDeep if both are extends object.

(intersection of objects, it's deprecated though.
https://www.typescriptlang.org/docs/handbook/unions-and-intersections.html#intersection-types)

Minor Changes

Refactor: delete unused import

[taiyakihitotsu]

I found this definition is incorrect, so made this PR to draft now.

I reopen it after fixing it.

Reopened.

[taiyakihitotsu]

@som-sm

I've finally fixed it.

Now IsEqual<A, B> returns true correctly if A and B are objects and mutually assignable.
This means IsEqual now treats {a: t, b: u} as equal to {a: t} & {b: u} recursively.

I also removed the test case (commited in dabe872) below because the four new test cases in this PR already cover it. (Please let me know if I missed anything.)

// Distinct whether an object is merged by `&` or via `Simplify`, to ensure Branded Types are handled strictly.
export type IntersectionMerge<tuple extends readonly unknown[]> = Except<tuple, 'length'> & {__brand: 'tag'};
type SampleTuple = [0, 1, 2];

expectType<true>({} as IsEqual<{a: 0} & {b: 0}, {a: 0; b: 0}>);
expectType<true>({} as IsEqual<Simplify<IntersectionMerge<SampleTuple>>, Merge<IntersectionMerge<SampleTuple>, IntersectionMerge<SampleTuple>>>);

Thanks!

[taiyakihitotsu]

I found this case failed even if just using SimplifyDeep in IsEqual.

expectType<true>({} as IsEqual<{aa: {a: {x: 0} & ({y: 0} | {y: 0})} & {b: 0}}, {aa: {a: {x: 0; y: 0}; b: 0}}>); // eslint-disable-line @typescript-eslint/no-duplicate-type-constituents

export type IsEqual<A, B> =
	[A, B] extends [B, A]
		? [A, B] extends [object, object]
			? _IsEqual<SimplifyDeep<A>, SimplifyDeep<B>>
			: _IsEqual<A, B>
		: false;

For example, this is a simple case.
https://github.com/taiyakihitotsu/type-fest/blob/fb14a98d577d543ba3eebab454f28fda5a0b5b5f/source/internal/type.d.ts#L218

type NT = _IsEqual<{z: {a: 0}}, {z: {a: 0} | {a: 0}}>; // => false

Using SimplifyDeep can eliminate intersections of A & A which should be equal to A, and {a: t} & {b: u} which should be equal to {a: t, b: u}.
But it cannot remove unions like A | A which should be equal to A.

To remove duplicated types in union, I defined UniqueUnionDeep in internal/type.d.ts.
(I don't know this helper will be needed for the other definitions though.)

---

Please let me know if I've missed any cases.

[taiyakihitotsu]

After a sleep, I've found another error in the definition of UniqueUnionDeep.

export type UniqueUnion<U> = _UniqueUnion<U, never>;
type _UniqueUnion<U, R> =
	LastOfUnion<U> extends infer K
		? [K] extends [never]
			? R
			: _UniqueUnion<Exclude<U, K>, K extends R ? R : R | K>
		: never;

extends is completely wrong here.

 _UniqueUnion<Exclude<U, K>, K extends R ? R : R | K>

I will reopen after fixing it.

Reopened.

[taiyakihitotsu]

Status

I've added 5 helper type functions in source/internal/type.d.ts.

• MatchOrNever: used to define ExcludeExactly
• ExcludeExactly: used to define UniqueUnion, which would "fix" Exclude / ExcludeStrict as below.
• LastOfUnion: used to define UniqueUnion
• UniqueUnion: used to define UniqueUnionDeep
• UniqueUnionDeep: used to define IsEqual

This PR passes those test cases (which fails before this PR):

// Ensure `{a: t; b: s}` === `{a: t} & {b: s}`, not equal to `{a: u} & {b: v}` if `u` !== `t` or `v` !== `s`.
expectType<true>({} as IsEqual<{a: 0} & {b: 0}, {a: 0; b: 0}>);
expectType<true>({} as IsEqual<{aa: {a: {x: 0} & {y: 0}} & {b: 0}}, {aa: {a: {x: 0; y: 0}; b: 0}}>);
// Ensure `{a: t} | {a: t}` === `{a: t}`
expectType<true>({} as IsEqual<{a: 0} & ({b: 0} | {b: 0}), {a: 0; b: 0}>); // eslint-disable-line @typescript-eslint/no-duplicate-type-constituents
expectType<true>({} as IsEqual<{aa: {a: {x: 0} & ({y: 0} | {y: 0})} & {b: 0}}, {aa: {a: {x: 0; y: 0}; b: 0}}>); // eslint-disable-line @typescript-eslint/no-duplicate-type-constituents

I've submitted this to fix IsEqual, ready for review.
And I need a discussion about ExcludeExactly.

Background

I found the bug described above, where unions inside recursive objects cannot be compared directly by the previous IsEqual definition which used SimplifyDeep.

To resolve this, I implemented UniqueUnion to deduplicate types within unions.

export type UniqueUnion<U> = _UniqueUnion<U, never>;
type _UniqueUnion<U, R> =
	LastOfUnion<U> extends infer K
		? [K] extends [never]
			? R
			: _UniqueUnion<
				Exclude<U, K>, // (1)
				(<G>() => G extends K & G | G ? 1 : 2) extends
				(<G>() => G extends R & G | G ? 1 : 2)
					? [R, unknown] extends [never, K]
						? K
						: R
					: R | K>
		: never;

I think this is the idiomatic way to iterate over union types, but it didn't work as expected.
I finally found that Exclude (at (1)) doesn't distinguish between {readonly a: 0} and {a: 0} for the following reason:: 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.

ExcludeStrict in type-fest is also the same.
(I commented those details in the ExcludeExactly section in source/internal/type.d.ts.)

So I decided to define ExcludeExactly to satisfy requirement (1) which must distinguish readonly and optional avoid incorrectly removing {a: 0} when compared to {readonly a: 0}.

And I added the helper function: MatchOrNever.
This MatchOrNever has a similar construct to _IsEqual, so it has the same limitations as noted in the comments.

export type MatchOrNever<A, B> =
	[unknown, B] extends [A, never]
		? A
		// This equality code base below doesn't work if `A` is `unknown` and `B` is `never` case.
		// So this branch should be wrapped to take care of this.
		: (<G>() => G extends A & G | G ? 1 : 2) extends (<G>() => G extends B & G | G ? 1 : 2)
			? never
			: A;

I've finished creating the strict IsEqual.

export type IsEqual<A, B> =
	[A, B] extends [B, A]
		? [A, B] extends [object, object]
			? _IsEqual<SimplifyDeep<UniqueUnionDeep<A>>, SimplifyDeep<UniqueUnionDeep<B>>>
			: _IsEqual<A, B>
		: false;

---

@som-sm

PR for IsEqual here

Now IsEqual is able to judge the four cases described above correctly.

But this is involving many diffs in internal.
I want to point them out, e.g., in case I missed some test cases.

PR for ExcludeExactly

Should I separate this PR and create another branch to fix ExcludeStrict?
I think this is not necessarily "a bug" because the ExcludeStrict comment refers to assignability here:

A stricter, non-distributive version of extends for checking whether one type is assignable to another.

But I also think that another version of Exclude is needed to maintain strict types.

If there is no need to touch ExcludeStrict, I would like to propose adding ExcludeExactly as a new type.

Is that okay?
Or should it be kept as a local helper function in this PR?

Thanks for reading!

[taiyakihitotsu]

Note:
Open to draft because of #1345

Reopen.

[som-sm]

@taiyakihitotsu Not sure if want to make IsEqual so complicated just to make {a: t, b: s} and {a: t} & {b: s} equal. Isn't there a simpler way of achieving this? And maybe in the future, this might just work automatically, refer microsoft/TypeScript#60726.

[taiyakihitotsu]

@som-sm
I've not found an elegant solution to unify a union type...

And maybe in the future, this might just work automatically

Thanks for your information!
I hope it to be merged, but I think we should not wait for it because the last comment was posted a month ago, no reviews.
We cannot predict when and (even) whether it will be merged.

I believe we should close any potential bugs which we can.
Even if this PR is merged and results in redundant logic in IsEqual, I expect type safety will not be compromised.
(But, if merged, a separate PR for performance optimization would likely be required later.)

---

I've done a refactor for UniqueUnion with UnionToTuple.

type UniqueUnion<U> = UnionToTuple<U>[number]

Simple.
But in this naive definition, npm test spits a stack-overflow.

✔ lint-rules/validate-jsdoc-codeblocks.test.js (11785.496384ms)
ℹ tests 37
ℹ suites 1
ℹ pass 37
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 11849.291703

<--- Last few GCs --->

[25259:0x556c1abd6000]    64324 ms: Scavenge 6096.5 (6140.3) -> 6084.4 (6141.8) MB, pooled: 0 MB, 4.60 / 0.00 ms  (average mu = 0.359, current mu = 0.354) allocation failure;
[25259:0x556c1abd6000]    65764 ms: Mark-Compact (reduce) 6110.4 (6154.5) -> 6063.4 (6113.3) MB, pooled: 0 MB, 47.19 / 0.00 ms  (+ 1233.1 ms in 247 steps since start of marking, biggest step 5.2 ms, walltime since start of marking 1416 ms) (average mu = 0
FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
----- Native stack trace -----

 1: 0x556bfb9bddd9 node::OOMErrorHandler(char const*, v8::OOMDetails const&) [node]
 2: 0x556bfbe20da4  [node]
 3: 0x556bfbe20e89  [node]
 4: 0x556bfc0a91fb  [node]
 5: 0x556bfc0a92f2  [node]
 6: 0x556bfc0a93c5  [node]
 7: 0x556bfc0b8cec  [node]
 8: 0x556bfc0bbcc5  [node]
 9: 0x556bfcb0de17  [node]
ERROR: "test:tsc" exited with 134.

I found it was caused by the definition of UniqueUnion and UnionToTuple.

• In UniqueUnion, a result of UnionToTuple should be stored with infer.
• In UnionToTuple, a result of ExcludeExactly<T, L> before expanding it.

export type UnionToTuple<T, L = LastOfUnion<T>> =
IsNever<T> extends false
	? ExcludeExactly<T, L> extends infer E // Improve performance.
		? [...UnionToTuple<E>, L]
		: never // Unreachable.
	: [];

Implementing either of these is enough to fix the stack overflow though, I’ve applied both just to be safe.

[som-sm]

I believe we should close any potential bugs which we can.

Yeah true, I just linked that PR so that in future when that gets merged we might be able to revert the changes done here.

Let me give this a thorough read sometime next week. I'll also go through all your previous attempts and the issues that existed in those.

In the meanwhile, feel free to open PRs for:

• Exposing LastOfUnion.
• Adding ExcludeExactly.
• Fixing the UnionToTuple bug.

These should be easier to review and get merged.

[taiyakihitotsu]

@som-sm

I've pushed a PR!
#1349

[taiyakihitotsu]

See this PR: #1349

---

And I found another bug in UnionToTuple:

type DifferentModifiers = {a: 0} | {readonly a: 0} | {a?: 0} | {readonly a?: 0};
expectType<UnionToTuple<DifferentModifiers>[number]>({} as DifferentModifiers);

Expected this to pass, but npm test says:

  test-d/union-to-tuple.ts:16:0
  ✖  16:0  Parameter type { readonly a?: 0; } is not identical to argument type DifferentModifiers.

UnionToTuple in this PR uses ExcludeExactly instead of the built-in Exclude or ExcludeStrict in type-fest to fix it.

---

To separate this point from this PR, LastOfUnion and ExcludeExactly should be defined independently.

And we should avoid duplicate definitions.
LastOfUnion was defined as a local in IsNever.

So I will make a separate PR for it, including:

• LastOfUnion: Useful to define recursion of union types.
• ExcludeExactly: Stricter version of built-in Exclude and type-fest's ExcludeStrict.
• UnionToTuple: Fix the above case, improve performance.

[taiyakihitotsu]

NOTE

944a74b

This change is a temporary fix to trigger the CI.
I’m keeping this PR as a Draft until the PR #1347 is merged, even if there are no conflicts here.

---

#1347 was merged, but this PR waits for #1349 as well.

[sindresorhus]

A couple of more tests. Maybe add around line 34:

expectType<true>({} as IsEqual<(value: {a: 1}) => {b: 1}, (value: {a: 1}) => {b: 1}>);
expectType<false>({} as IsEqual<(value: {a: 1}) => {b: 1}, (value: {a: 2}) => {b: 1}>);

[taiyakihitotsu]

@sindresorhus

Thank you!

I’ve left source/exclude-exactly.d.ts part as is because it’s waiting for #1349 to be merged.
These will be resolved automatically.

While adding the tests you suggested, I discovered a similar issue about lambda arguments and included an update for it as well.
(I'll post about it here.)

[taiyakihitotsu]

Found another edge case where lambda arguments and return types are identical unions or intersections.

This isn't a regression; the previous UniqueUnionDeep simply didn't support it, so I've included a fix.

---

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAbwKYA8xIMYwCoE90C+cAZlBCHAOQwDOAJpQNwCwAUGzPknAHICuIAEZIoAGQCGQuuLgBeOAAoAbuIA2fJAC5E47QDsBwqAQCUcgHyJB+wyIItWndL1tjJg6QEk9METUwwwBB6cooq6lo6NkJ2cABkUXAGMcZmspYI1kmu9hxcLikSUuIAqnpBIfLKahraCLrZKUQAPonJRqYWVtEdDk7c-IXu0gBKSDB8UHrevlD+WBWh1RF1De12aRlZ68bx3Y29ec6DRkUe4mMTU2WLVeG1bTmb+zstLzkOR9wA+p40AKIARz4agAPABBAA0cAAQpZZGwAJAKUEAcXMCmeqLgqF8ejoNDg4L22Na2IA-HAAIxwbQAJjMuKQ+JoSJR6MxXWxTJZsJJcDJcEpNPpJiRiMpMCgGnF2mIan8n1YqHQWDw6FBUo0GIQRHEhN+AOBYJOIjOXh8fgCFWhprcxXMJgcAHpnekcVAyFA2CqAuqkJrpUgdXqDX8gSDVKC7ebSuVgrbXLHHS63ZYRF6fWg-VxA9qFLq4Pq4IaIyak8MLuNJtNLXNrQmCqdKym2K73RnoFnVThc1rgwXQyXw8aozHK5cazdG+OHU622mPV6gA

import {expectType} from 'tsd';

type NumberLambda = (value: {a: number}) => {b: number};
type NumberLambdaIntersection = (value: {a: number} & {a: number}) => {b: number};
type NumberLambdaUnion = (value: {a: number} | {a: number}) => {b: number};
type NumberLambdaReturnIntersection = (value: {a: number}) => {b: number} & {b: number};
type NumberLambdaReturnUnion = (value: {a: number}) => {b: number} | {b: number};

type _IsEqual<A, B> =
	(<G>() => G extends A & G | G ? 1 : 2) extends
	(<G>() => G extends B & G | G ? 1 : 2)
		? true
		: false;

expectType<true>({} as _IsEqual<NumberLambdaIntersection, NumberLambda>);
//=> error
expectType<true>({} as _IsEqual<NumberLambdaUnion, NumberLambda>);
//=> error
expectType<true>({} as _IsEqual<NumberLambdaReturnIntersection, NumberLambda>);
//=> error
expectType<true>({} as _IsEqual<NumberLambdaReturnUnion, NumberLambda>);
//=> error