Skip to content

Ensure state is merged into meta props as plain object (#1877, #1955) #2336

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

Merged
merged 1 commit into from
Dec 27, 2016
Merged

Conversation

esbenp
Copy link
Contributor

@esbenp esbenp commented Dec 26, 2016

cb3abb5 added functionality that merged the state into the meta object for fields. However, when using Immutable JS structures the state will be an Immutable.Iterable. When developing in React Native the Object.assign call here cb3abb5#diff-7eb826f425b082db29894569d83899f8R68 causes an exception as described in #1877 and #1955.

One of the sources for assign has an enumerable key on the prototype chain. This is an edge case that we do not support. This error is a performance optimization and not spec compliant.

I am assuming this is because the Object.assign polyfill is different in the React Native environment (any confirmation on this?). I tested the webpack polyfill in the Immutable example and it pretty much just transfers properties from one object to another without any checks. I am assuming this is why this error has not had any more attention since it does not happen in browser environments.

EDIT: Found the polyfill https://github.com/facebook/react-native/blob/master/packager/react-packager/src/Resolver/polyfills/polyfills.js#L56

As for the fix I added a toJS method to the structure types. This also means I had to expand the first argument in createFieldProps and createFieldsProps to be an object instead of just getIn. I am not familiar with the code base so any pointers and directions for style is much appreciated. It should be noted that I have not tested this solution in React Native environment (only a dirty hack that accomplishes the same). And I have not tested all the examples either but tests are passing.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
#1877, #1955)

cb3abb5 added functionality that merged the state
into the meta object for fields. However, when using Immutable JS structures this
will be an Immutable.Iterable. This causes an exception in environments where
the Object.assign polyfill does not allow this (for instance in React Native)
@codecov-io
Copy link

codecov-io commented Dec 26, 2016

Current coverage is 100% (diff: 100%)

Merging #2336 into master will not change coverage

@@           master   #2336   diff @@
=====================================
  Files          61      61          
  Lines        1267    1269     +2   
  Methods         0       0          
  Messages        0       0          
  Branches        0       0          
=====================================
+ Hits         1267    1269     +2   
  Misses          0       0          
  Partials        0       0          

Powered by Codecov. Last update c3aa263...4eb1d47

@erikras
Copy link
Member

erikras commented Dec 27, 2016

Looks good.

@erikras erikras merged commit bf698c8 into redux-form:master Dec 27, 2016
@lock
Copy link

lock bot commented Jun 1, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants