Fix type of next parameter in StoreEnhancer type
name: "Fix type of next parameter in StoreEnhancer type" about: Fix type of next parameter in StoreEnhancer type
PR Type
Does this PR add a new feature, or fix a bug?
Fix a bug. Resolves https://github.com/reduxjs/redux/issues/3768.
Why should this PR be included?
Without this PR the type for next in StoreEnhancer is incorrect. It assumes that next will create a store with the current enhancers Ext and StateExt already applied to store. Instead, next returns a store with NextExt and NextStateExt generic parameters since the enhancer should work when composed with any other enhancer. It then expects the StoreEnhancer to return a new store creator that will create a store with Ext and NextExt, and StateExt and NextStateExt.
Note that I had to remove the ExtendState type in order to make this work. This type was introduced in https://github.com/reduxjs/redux/pull/3524, but I wasn't able to get the types working while using ExtendState. I think the types are more clear without it anyway.
Checklist
- [x] Have you added an explanation of what your changes do and why you'd like us to include them?
- [x] Is there an existing issue for this PR?
- https://github.com/reduxjs/redux/issues/3768
- [x] Have the files been linted and formatted?
- [x] Have the docs been updated to match the changes in the PR?
- [x] Have the tests been updated to match the changes in the PR?
- [x] Have you run the tests locally to confirm they pass?
Bug Fixes
What is the current behavior, and the steps to reproduce the issue?
Steps to reproduce are in https://github.com/reduxjs/redux/issues/3768. There's also a new test for checking the types on stores created from composed enhancers. This test fails in master.
What is the expected behavior?
The types should allow for a generic NextExt and NextStateExt.
How does this PR fix the problem?
It adds those generics.
Deploy preview for redux-docs ready!
Built with commit c5863d52aaa2315c876dcefc0a39ce95e201a128
https://deploy-preview-3776--redux-docs.netlify.app
Deploy preview for redux-docs ready!
Built with commit 92cf2a2429ab87f1118e9f33cc43bcf731893743
https://deploy-preview-3776--redux-docs.netlify.app
Can you fix the conflicts?
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 5a084a62fda9ebf39ffa969458d9950b4f051688:
| Sandbox | Source |
|---|---|
| Vanilla | Configuration |
| Vanilla Typescript | Configuration |
I'll try to circle back and actually look at this in the near future. My main observation atm is that this is gonna go onto master, and we have no active plans to actually release the code in master at this point. It's the 4.x branch that matters for the foreseeable future.
@markerikson No problem, don't mean to guilt you into looking into this but the old types are objectively wrong and so I'd love to get this merged. I'd be happy to create a separate PR to target 4.x.
@markerikson Also https://github.com/reduxjs/redux/pull/3772 really needs to get merged at some point as well. 😉
Let me know when you're ready to look at either one and I can resolve merge conflicts.
I'd like to try to do some general Redux org issue triage and cleanup over the next couple weeks, so yeah, getting this knocked out would be high on the priority list.