Skip to content

TypeScript: Excessive stack depth comparing generic preloadedState with DeepPartial #3454

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
hswhite33 opened this issue Jun 20, 2019 · 9 comments

Comments

@hswhite33
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Passing a generically typed preloadedState parameter into createStore creates an "Excessive stack depth" error in TypeScript due to the comparison between S and DeepPartial<S>

Sandbox: https://codesandbox.io/s/busy-leavitt-wrpnx?fontsize=14

More information at microsoft/TypeScript#21592 (comment) - this is caused by a TypeScript bug, but the TypeScript devs have suggested that Redux may not actually need to be using the DeepPartial type here.

What is the expected behavior?
No TypeScript errors

Which versions of Redux, and which browser and OS are affected by this issue? Did this work in previous versions of Redux?
In theory this affects any version of Redux using the DeepPartial type. The symptoms and workaround have changed in recent versions of TypeScript; I've reproduced the issue with [email protected] and [email protected]

@timdorr
Copy link
Member

timdorr commented Jun 21, 2019

Do you have self-referential data/types in your state? You shouldn't, as your preloadedState needs to be serializable (by simple means, e.g. JSON.serialize, not via a special serializer) so it can be provided to the store. A circular reference would create a problem with this process, so would likely be another issue you run into even if the typings are fixed. As such, I'd almost say this is TS doing it's job and preventing a runtime error.

@hswhite33
Copy link
Author

The type S is unconstrained, so this function is making no assertions that the state doesn't have self-referential data/types, but neither does the signature of Redux.createStore itself (I don't know of any way to model this constraint using TypeScript). In practice I don't pass any self-referential objects into the function.

The issue as I understand it is in the fact that, because S is a generic type, TypeScript goes into a stack overflow trying to compare it with the recursively defined DeepPartial<S>.

@timdorr
Copy link
Member

timdorr commented Jun 22, 2019

It doesn't recurse infinitely, though. It will bail once it hits a non-object:

[K in keyof T]?: T[K] extends object ? DeepPartial<T[K]> : T[K]

Unless you have circular references in your types, or have a very deep state tree, this isn't something you should trip in your types.

Unfortunately, we have to keep the preloadedState as partial. It may not be a complete representation of the store state once the INIT action is fired. This is particularly true for those with dynamically-added reducers. That's a common enough use case that we need to support it.

If anything, we should probably extend it to handle Arrays:

export type DeepPartial<T> = {
  [P in keyof T]?: T[P] extends Array<infer U>
    ? Array<DeepPartial<U>>
    : T[P] extends ReadonlyArray<infer U>
    ? ReadonlyArray<DeepPartial<U>>
    : DeepPartial<T[P]>
}

That won't fix the issue you're running into, but I don't think there's something we can do without breaking a greater set of users. Sorry :(

@timdorr timdorr closed this as completed Jun 22, 2019
@hswhite33
Copy link
Author

Sorry, I hadn't looked at the latest master! This bail condition actually does fix the issue I'm having.

The latest release v4.0.1 has DeepPartial defined with no bail condition:

export type DeepPartial<T> = { [K in keyof T]?: DeepPartial<T[K]> }

Is there a timeline on a new release which will include this improved type definition?

@timdorr
Copy link
Member

timdorr commented Jun 23, 2019

I'm currently at the beach, so I can look at it next week.

@val-grasley
Copy link

@timdorr Hope you had a fun time at the beach. Any chance we could get a patch with this fix?

@timdorr
Copy link
Member

timdorr commented Jul 8, 2019

Yep, sorry. I can look into a release soon.

@val-grasley
Copy link

Thanks!

@timdorr
Copy link
Member

timdorr commented Jul 9, 2019

Pushed out 4.0.2 now: https://github.com/reduxjs/redux/releases/tag/v4.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants