Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Object assign gives error when bundling react native package #78

Closed
arealmaas opened this issue Jun 21, 2016 · 15 comments
Closed

Object assign gives error when bundling react native package #78

arealmaas opened this issue Jun 21, 2016 · 15 comments
Assignees
Labels

Comments

@arealmaas
Copy link

arealmaas commented Jun 21, 2016

Using react native and this line: https://github.com/apollostack/react-apollo/blob/master/src/connect.tsx#L158 gives me the following error in the react native simulator:

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.

Anybody else receiving this?

@arealmaas
Copy link
Author

changed from

var storeState = this.store.getState();
this.state = Object.assign({}, storeState)

to

var storeState = this.store.getState();
this.state = storeState.toJS()

And it started working again. Seems like an issue with Object.assign and react native, prooobably not related to this lib :))

@stubailo
Copy link
Contributor

@jbaxleyiii perhaps we should use lodash.assign?

@jbaxleyiii
Copy link
Contributor

jbaxleyiii commented Jun 21, 2016

@stubailo sounds good too me. I can change that out and open a PR this evening for it, unless you or someone else wants to get to it sooner?

Also, welcome back!

@stubailo
Copy link
Contributor

Hey good to be back! So relaxing :) I can try, but I have a lot of meetings so it might be much later today.

@jbaxleyiii
Copy link
Contributor

haha same here 👍

@arealmaas
Copy link
Author

Does assign even work with immutable Map?

@arealmaas
Copy link
Author

arealmaas commented Jun 22, 2016

nvm.. The issue seems to be the use of immutable reducers.. Seems like react-apollo doesn't treat immutable data very well :)

@jbaxleyiii
Copy link
Contributor

@AlmaasAre can you show me your usage? I'd love to improve this lib to support immutable data! Are you using immutable.js?

@arealmaas
Copy link
Author

@jbaxleyiii Yes sure, thanks! Yes i am! When using the connect function it fails. Tries to deep copy state by using Object.assign which i believe doesn't work well with immutablejs. I also got some errors of wrong usage of Object.assign:

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 get this error by just using connect from apollo.

Dunno what needs to be done tho. Check if the state is immutable before assigning? not sure

@jbaxleyiii
Copy link
Contributor

@AlmaasAre yep! I need to add in a check for immutablejs 👍

@aweary
Copy link
Contributor

aweary commented Jun 29, 2016

Does it make more sense to provide a way for the user to map their storeState to connect's local state? If you just add a check for immutable.js you'll still have the same problem when users are using other immutable libraries.

Instead maybe connect can accept a function like getStateFromStoreState that passes in storeState and lets the user parse and return a plain object representing their state.

@stubailo
Copy link
Contributor

stubailo commented Jul 6, 2016

Apollo Client now supports React Native as of 0.3.27! Going to close this issue because most of the conversation is about Immutable.js and not React Native.

@stubailo stubailo closed this as completed Jul 6, 2016
@zol zol removed the help wanted label Jul 6, 2016
@davidyaha
Copy link

I think that with the 0.5.25 version of apollo-client that uses object spread and polyfills with Object.assign, this bug came back..
In my implementation this happens on refetch.
Also I think this issue belongs in apollo-client rather than here.

@helfer
Copy link
Contributor

helfer commented Jan 10, 2017

@davidyaha is this still the case for you with 0.7.1? If so, please open an issue on apollo-client! ❤️

@davidyaha
Copy link

Not sure.. I will update next week probably and if it reappears I'll open a new issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants