-
-
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
add support for props with nested immutables #3019
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3019 +/- ##
======================================
Coverage 100% 100%
======================================
Files 67 68 +1
Lines 1395 1436 +41
======================================
+ Hits 1395 1436 +41
Continue to review full report at Codecov.
|
From your description, this sounded like a massive change, and then it only turned out to be two little lines. So all you're doing is allowing additional "props not to update for" to be specified. It's unclear to me what this has to do with immutability or how it would help with Draft JS. Why are you passing |
I find small changes to be easier to get merged, and I really want this to
get merged :-).
With immutability: the component should update if any prop changed. That
means we should deep equals all the props. The only exception would be
immutable structures. I'm game for a name change if there's a better one.
For my use case, my editor pops up a modal with a form and feeds it the
state.
…On Jun 6, 2017 6:44 AM, "Erik Rasmussen" ***@***.***> wrote:
From your description, this sounded like a massive change, and then it
only turned out to be two little lines.
So all you're doing is allowing additional "props not to update for" to be
specified. It's unclear to me what this has to do with immutability or how
it would help with Draft JS. Why are you passing editorState to your form
as a prop?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#3019 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFQjv42lqZVyKmo5FkdmRmZQJzpnbohUks5sBVTtgaJpZM4NwUIM>
.
|
The problem I see, though, is that you are totally ignoring changes to the props listed. It seems like you should be Let's call it |
you're correct, i was being selfish & just solving for my use case. |
Can you get the code coverage back up to 100%?
|
src/__tests__/reduxForm.spec.js
Outdated
) | ||
TestUtils.Simulate.click(initButton); | ||
|
||
// no need to rerender form on initialize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is kinda misleading, but IMO fix it if you want 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch, i was just blindly cut & pasting 😄
Fantastic! |
oh super duper, thank you! |
Published in |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Background:
Sometimes, a prop might be a POJO that contains a nested immutable structure. Draft-js'
editorState
is a great example. Within sCU, the plaindeepEqual
function is used on it & breaks.Why this is needed:
Example:
reduxForm({form: 'linkChanger', immutables: ['editorState']})
can bump the docs if you like it