Breaking change to Simplify<> introduced in v2.16.0

[andyjy]

The change to Simplify<> introduced in #414 looks great, but turns out it breaks my codebase vs the prior definition of Simplify<>, in two ways:

1. Type instantiation is excessively deep and possibly infinite errors in the VSCode typescript checker
2. RangeError: Maximum call stack size exceeded fatal error when running tsc

I would love to create a Minimum Reproducible Example but unfortunately the types involved span a bunch of files and types (as suggested by the nature of these errors) so that would be quite a challenge.

I've confirmed 100% this is the cause and created a workaround by patching back to the prior Simplify<> definition using patch-package and the patch below.

I also appreciate this error isn't caused by type-fest alone - only occurring in combination with my own code - so I can see some may not consider this strictly a breaking change. But I figured the fact the breakage only came with the update to type-fest seemed worth sharing - and potentially revertible.

In terms of a potential solution - I see that:

1. the change in Fix: Simplify handle all types #414 is to improve the behaviour of Simplify<> when applied to non-object types, and
2. Simplify<> is used "internally" within other type-fest types (e.g. Merge<>, SetOptional<>, Spread<>, Writable<>) where the input types are expected to be objects and thus the improvement implemented by Fix: Simplify handle all types #414 shouldn't be applicable(?)

Would it therefore make sense to revert #414 and reimplement using two separate types in a way such that the original "simple" Simplify<> definition is used by these internal usages?

E.g. re-add the original Simplify<> implementation under another name and update the internal usages to refer to that (where the new functionality to cope with non-object types is not required), leaving type-fest users free to pick from either type as appropriate?

If this sounds good I could provide a PR - I figured to share the issue & check for feedback first.

Thanks!

patch-package patch I have currently used to revert this change:

diff --git a/node_modules/type-fest/source/simplify.d.ts b/node_modules/type-fest/source/simplify.d.ts
index 7112dc3..cccd870 100644
--- a/node_modules/type-fest/source/simplify.d.ts
+++ b/node_modules/type-fest/source/simplify.d.ts
@@ -75,9 +75,16 @@ fn(someInterface as Simplify<SomeInterface>); // Good: transform an `interface`
 
 @category Object
 */
-export type Simplify<
-	AnyType,
-	Options extends SimplifyOptions = {},
-> = Flatten<AnyType> extends AnyType
-	? Flatten<AnyType, Options>
-	: AnyType;
+// PATCH: revert Simplify<> type change made in type-fest v2.16.0
+//        to version used in v2.15.0 and prior
+//        since the updated version causes a world of pain
+//        with tsc RangeError: Maximum call stack size exceeded,
+//        introduction of circular reference errors, etc
+//        when it meets our api-framework types
+export type Simplify<T> = {[KeyType in keyof T]: T[KeyType]};
+// export type Simplify<
+// 	AnyType,
+// 	Options extends SimplifyOptions = {},
+// > = Flatten<AnyType> extends AnyType
+// 	? Flatten<AnyType, Options>
+// 	: AnyType;

[sindresorhus]

// @skarab42

[skarab42]

@andyjy I would love to have an idea of what type is causing the bug. I think I can see what's wrong (but that's just a supposition without a reproduction). I'll make a temporary fix that you can test. Then we'll see what to do from there.

[skarab42]

@andyjy Just an idea, you add the type in question to a file, then compile it and get the full type in a .d.ts ?

[skarab42]

@andyjy can you test with this PR #437

[andyjy]

Thanks @skarab42 - great thinking, unfortunately the change in the PR #437 does not solve this issue for me (still same errors).

Separately I confirmed my error is caused by the use of Simplify<> within SetRequired<> - the patch below to revert to the prior usage of Simplify<> within SetRequired<> while retaining the change from #414 also resolves the errors.

This increases my confidence in the point in my original post that - unless I'm missing something? - the use of Simplify<> within SetRequired<> (and the like) is always intended to be applied to object types that do not benefit from the change in #414, hence retaining the original simple definition of Simplify<> to use for such cases could be advantageous in any case(?)

I will try spend some time to extract the offending type(s) in question as you suggest - in short I'm wrapping SetRequired<> around an abstract class type that uses a bunch of generics of other relatively complex types, some involving (depth-limited) recursion - so I appreciate it's me who dug the hole I've fallen in.. it'll just take me a bit of time to try extract an example.

diff --git a/node_modules/type-fest/source/set-required.d.ts b/node_modules/type-fest/source/set-required.d.ts
index a62da3b..894eabd 100644
--- a/node_modules/type-fest/source/set-required.d.ts
+++ b/node_modules/type-fest/source/set-required.d.ts
@@ -1,5 +1,5 @@
 import type {Except} from './except';
-import type {Simplify} from './simplify';
+import type {SimplifyObject} from './simplify';
 
 /**
 Create a type that makes the given keys required. The remaining keys are kept as is. The sister of the `SetOptional` type.
@@ -27,7 +27,7 @@ type SomeRequired = SetRequired<Foo, 'b' | 'c'>;
 @category Object
 */
 export type SetRequired<BaseType, Keys extends keyof BaseType> =
-	Simplify<
+	SimplifyObject<
 		// Pick just the keys that are optional from the base type.
 		Except<BaseType, Keys> &
 		// Pick the keys that should be required from the base type and make them required.
diff --git a/node_modules/type-fest/source/simplify.d.ts b/node_modules/type-fest/source/simplify.d.ts
index 7112dc3..cb4f166 100644
--- a/node_modules/type-fest/source/simplify.d.ts
+++ b/node_modules/type-fest/source/simplify.d.ts
@@ -75,9 +75,12 @@ fn(someInterface as Simplify<SomeInterface>); // Good: transform an `interface`
 
 @category Object
 */
+export type SimplifyObject<T> = {[KeyType in keyof T]: T[KeyType]};
+
 export type Simplify<
 	AnyType,
 	Options extends SimplifyOptions = {},
 > = Flatten<AnyType> extends AnyType
 	? Flatten<AnyType, Options>
 	: AnyType;
+// ^- this time retaining the updated version of Simplify<> from https://github.com/sindresorhus/type-fest/pull/414

[skarab42]

@andyjy Thank you for testing and don't waste time with the extraction of the type if it is too complicated. In fact the change was made in anticipation of this PR #408 (comment)
Two things; I need the deep option (which doesn't seem to be a problem as you don't use it) and I need it to not simplify classes and functions. I'm still making a change and if it's not right we can go back and provide another type for my case.

[skarab42]

@andyjy I have just updated the PR can you test? #437

[andyjy]

@andyjy I have just updated the PR can you test? #437

Thank you - switching the conditional to extends Function ? in the updated PR does solve the 2nd error (RangeError: Maximum call stack size exceeded fatal error when running tsc) but the 1st error remains (Type instantiation is excessively deep and possibly infinite in the VSCode typescript checker).

This is curious as now tsc compiles without error - so the 'excessively deep' error only appears within VSCode. I don't know what might be causing the difference in behaviour here - any pointers welcomed! (I have tried restarting VSCode and applying/reverting the change a few times to ensure this is indeed the behaviour.)

(Both errors are still resolved if I revert back to the second patch I posted above that uses the original simple implementation for SetRequired<>.)

[skarab42]

@sindresorhus ?

[skarab42]

@andyjy Just one more test 🙏, I need to understand what is going on and where the problem is. Can you patch with the code below and test with OldSimplify which is the original version, if it's ok, test with Flatten and finally with Simplify.

export type OldSimplify<T> = {[KeyType in keyof T]: T[KeyType]};

export interface SimplifyOptions {
	deep?: boolean;
}

export type Flatten<
	AnyType,
	Options extends SimplifyOptions = {},
> = Options['deep'] extends true
	? {[KeyType in keyof AnyType]: Simplify<AnyType[KeyType], Options>}
	: {[KeyType in keyof AnyType]: AnyType[KeyType]};

export type Simplify<
	AnyType,
	Options extends SimplifyOptions = {},
> = AnyType extends Function ? AnyType : Flatten<AnyType, Options>;

[skarab42]

With the above code (if you don't use the deep option) I don't see how it's different from calling the original version? and how it could cause an excessively deep instantiation.

[andyjy]

Can you patch with the code below and test with OldSimplify which is the original version, if it's ok, test with Flatten and finally with Simplify

OldSimplify works.
Flatten causes the error (i.e. export type Simplify<AnyType, Options> = Flatten<AnyType, Options>)

In fact - introducing any conditional seems to cause the error, e.g.:

export type Simplify<T> = true extends true ? {[KeyType in keyof T]: T[KeyType]} : never;

I can't explain why, but evidently the compiler is not simply optimising away this simple conditional.

Unless anyone has a better understanding of why this is, I would suggest proceeding by reverting to the original definition of Simplify and implementing a new SimplifyDeep rather than passing SimplifyOptions as a parameter.

~~

This is curious as now tsc compiles without error - so the 'excessively deep' error only appears within VSCode.

I subsequently realised tsc on the command line typechecks without error, but still outputs the same errors as the VSCode plugin when configured to output declaration files. So while I'm no expert on the compiler internals this is no longer concerning to me - if I understand correctly, type checking is implemented with optimisations that require the compiler to evaluate less vs. calculating full type definitions.

[skarab42]

@andyjy Thanks for testing.

@sindresorhus I think Simplify is a UX tool and should not be used internally. For me it is up to the user to apply Simplify (or SimplifyDeep) where they need it. At the moment there are scenarios where Simplify is called redundantly when using multiple types that use it internally.

[sindresorhus]

@skarab42 Some of the types IMHO do need Simplify otherwise they show very complicated output in docs/typehints and a user would generally not know to use Simplify.

[sindresorhus]

I think we should split up Simplify as suggested. However, we cannot make a breaking change, so for now, we can do this.

Make:

• Simplify
• SimplifyDeep

But keep the old Simplify as the exported Simplify (duplicated code is ok as this one will be removed in v3). Export the new SimplifyDeep. Export the new simple Simplify as _Simplify for now for anyone that needs an immediate workaround. Use the new simple Simplify internally.

[skarab42]

@sindresorhus While doing the PR, I realise that I don't see how not to introduce a breaking change. If we export the old Simplify which had no options, new users that use the deep option will have to change their code (I think this number is close to zero). And we can't keep the existing signature, as tested by @andyjy , because introducing any condition produces an error in his IDE (which I still don't understand, I would really like to reproduce the error).

In any case I would like to reduce the number of conditions even if it means having several signatures, that's what I propose.

simplify.d.ts

// The old one
export type Simplify<Type> = {[KeyType in keyof Type]: Type[KeyType]};

// The old one but deep
export type SimplifyDeep<Type> = {[TypeKey in keyof Type]: SimplifyDeep<Type[TypeKey]>};

conditional-simplify.d.ts

export type ConditionalSimplify<Type, ExcludeType> = Type extends ExcludeType
  ? Type
  : {[TypeKey in keyof Type]: Type[TypeKey]};

export type ConditionalSimplifyDeep<Type, ExcludeType> = Type extends ExcludeType
  ? Type
  : {[TypeKey in keyof Type]: ConditionalSimplifyDeep<Type[TypeKey], ExcludeType>};

The conditional version is much more flexible than the current version because the user can specify the type(s) he does not want to simplify and the total number of conditions is greatly reduced by removing the options.

[sindresorhus]

@skarab42 Alright. Let's do what you propose, but let's keep ConditionalSimplify internal for now. It would give us a chance to try it out and fix any possible issues.

Long-term I would like for Simplify to handle things that cannot be flattened, like before. It does sound like a TypeScript bug.

[skarab42]

@skarab42 Alright. Let's do what you propose, but let's keep ConditionalSimplify internal for now. It would give us a chance to try it out and fix any possible issues.

Long-term I would like for Simplify to handle things that cannot be flattened, like before. It does sound like a TypeScript bug.

@sindresorhus I have made an TS Playground that illustrates a part of the problem in case you want to report it to the TS team.

[sindresorhus]

The release is out: https://github.com/sindresorhus/type-fest/releases/tag/v3.0.0