-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Upgrade dev dependency to react-redux v7 #4431
base: master
Are you sure you want to change the base?
Conversation
That's... odd. I would have expected the number of re-renders to be pretty consistent between v6 and v7. The number of times |
Thanks for your continued work on this @erikras , many people still love and rely on Redux-Form! |
Hi! I have been debugging this a bit and I think that I know what one of the issues is (quite possibly the main issue). I think that I have some ideas on how to fix it, but it's getting late and I want to go to bed. So, I will keep looking at this in another moment. In the meanwhile, I will explain here what I have found, in case someone else wants to have a look at it before I get back to it: The issue that I have found is that when a Field registers itself to the reduxForm a dispatch takes place ( |
When exactly in the component lifecycle is that action being dispatched? One thing that did change in v7 is that we're subscribing in a |
Oh, ok, thanks @markerikson ! So, let me describe in more detail what I'm seeing:
node --inspect-brk node_modules/.bin/jest --runInBand -t 'should be `false` when `errors` has a `string` property'
|
Sorry, I was wrong... However, I found something interesting/weird: For this test, the following line of I know that sounds like non-sense, because the |
More interesting stuff, check this out.. That sandbox does the exact same thing that this test does. However, in the sandbox App the output is the correct one. That makes me think that the problem probably has to do with the way that the tests are being setup. Either that, or perhaps there is an issue with |
I think we should add calls to See for example this commit in a hook version of react-redux-promise-listener: mikkom/react-redux-promise-listener-hook@eebee1e |
Thanks for your effort on this, guys. |
Codecov Report
@@ Coverage Diff @@
## master #4431 +/- ##
==========================================
- Coverage 99.47% 0.00% -99.48%
==========================================
Files 74 74
Lines 1728 1687 -41
==========================================
- Hits 1719 0 -1719
- Misses 9 1687 +1678
Continue to review full report at Codecov.
|
b2cbf03
to
154cde4
Compare
This breaks a lot of the tests, and I'm not certain why. Many of the
redux-form
tests focus on how many times stuff gets rerendered, and something about that has changed withreact-redux@7
.🤔