Skip to content
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

Add PreloadedState generic #4491

Merged
merged 1 commit into from Apr 16, 2023

Conversation

Methuselah96
Copy link
Member

@Methuselah96 Methuselah96 commented Feb 17, 2023

Summary

The main change to pay attention to here is the update to the Reducer type. Before this PR Reducer has a type of:

type Reducer<S, A extends Action> = (
  state: S | undefined,
  action: A
) => S

After this PR Reducer has this type:

type Reducer<S, A extends Action, PreloadedState = S> = (
  state: S | PreloadedState | undefined,
  action: A
) => S

Explanation

Why the need for this change? When the store is first created by createStore, the initial state is set to whatever is passed as the preloadedState argument (or undefined if nothing is passed). That means that the first time that the reducer is called, it is called with the preloadedState. After the first call, the reducer is always passed the current state (which is S).

For most normal reducers, S | undefined accurately describes what can be passed in for the preloadedState. However the combineReducers function allows for a preloaded state of Partial<S> | undefined.

The solution is to have a separate generic that represents what the reducer accepts for its preloaded state. That way createStore can then use that generic for its preloadedState argument.

Previous work

Up until now, this has been accounted for with the $CombinedState type which is a way to "mark" a type as returned by combineReducers. The PreloadedState type then looks to see if the type has that "mark" and then accepts Partial<S> if it has that "mark."

One of the problems this can cause is that if there are multiple copies of Redux in node_modules, then the "mark" for one copy of Redux is different than the "mark" for the other copy, and therefore they won't recognize each other.

This also seems to be causing some problems in reduxjs/redux-toolkit#2068.

This PR removes the need for $CombinedState altogether.

Upgrade Impact

This change does include some breaking changes, but overall should not have a huge impact on users upgrading in user-land.

Additional generic arguments

The Reducer, ReducersMapObject, and createStore types/function take an additional PreloadedState generic which defaults to S.

combineReducers

The overloads for combineReducers are removed in favor of a single function definition that takes the ReducersMabObject as its generic parameter. Removing the overloads was necessary with these changes, since sometimes it was choosing the wrong overload.

Enhancers

Enhancers that explicitly list the generics for the reducer will need to add the third generic.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 17, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ac804a5:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

src/createStore.ts Outdated Show resolved Hide resolved
@Methuselah96
Copy link
Member Author

@markerikson I think this is ready to merge. I've created reduxjs/redux-toolkit#3198 to show that RTK should be able to consume this update fairly easily, which I think was @phryneas's only remaining concern. Let me know if you need anything else from me.

@markerikson markerikson merged commit 6add8a2 into reduxjs:master Apr 16, 2023
17 checks passed
@Methuselah96 Methuselah96 deleted the preloaded-state-generic-4 branch April 16, 2023 19:18
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

Successfully merging this pull request may close these issues.

None yet

4 participants