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

experiment: clean $CombinedState out from getState #4312

Closed
wants to merge 1 commit into from

Conversation

phryneas
Copy link
Member

Hey @markerikson take a look - does this solve reduxjs/redux-toolkit#2068 ?

@codesandbox-ci
Copy link

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 e2d1c7c:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@github-actions
Copy link

Size Change: 0 B

Total Size: 8.62 kB

ℹ️ View Unchanged
Filename Size Change
dist/redux.js 6.9 kB 0 B
dist/redux.min.js 1.72 kB 0 B

compressed-size-action

@markerikson
Copy link
Contributor

Just tried this. It does fix the issue... if you infer type RootState = ReturnType<typeof store.getState>. However, it does not fix the issue if you infer directly from ReturnType<typeof rootReducer>. Which is what I suspected might happen.

So clearly something is going on with Reselect and that $CombinedState type.

@mpeyper
Copy link
Contributor

mpeyper commented Mar 16, 2022

This would likely be a workable solution for my issue, as just having an option to erase the $CombinedState would at least keep me moving, but I agree with @markerikson that it doesn't actually solve the root of the issue.

@markerikson
Copy link
Contributor

Where does this idea stand? I think I saw us talking recently about dropping $CombinedState completely? What does that even do, anyway?

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.

3 participants