search

rtfeldman / seamless-immutable

Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.
BSD 3-Clause "New" or "Revised" License
5.37k stars 195 forks source link

Don't try to make React elements immutable #120

Closed davidblurton closed 8 years ago

davidblurton commented 8 years ago

Fixes #73

Allows you to map from an immutable to a React element.

davidblurton commented 8 years ago

Tests pass but error with 

Error:
Zuul tried to run tests in saucelabs, however no saucelabs credentials were provided.
See the zuul wiki (https://github.com/defunctzombie/zuul/wiki/Cloud-testing) for info on how to setup cloud testing.
shamrin commented 8 years ago

@davidblurton Interesting approach to solving #73.

Have you considered simply checking for isReactComponent property? It was sugggested here:

https://discuss.reactjs.org/t/how-to-determine-if-js-object-is-react-component/2825/5

davidblurton commented 8 years ago

The property isReactComponent doesn't seem to be defined on ES6 classes.

To check for classes you'd have to do React.Component.isPrototypeOf(object) but that involves taking a dependency on react.

shamrin commented 8 years ago

You are right, React elements no longer can be detected by a simple property check.¹ Your approach seems to be the correct way. It's even mentioned in "React Components, Elements, and Instances" blog post:

All React elements require an additional $$typeof: Symbol.for('react.element') field declared on the object for security reasons.

¹ _isReactElement property was removed, at the same time as $$typeof was introduced - before React v0.14.0-rc1.

davidblurton commented 8 years ago

I've added some tests for React elements and components. This along with the cycle detection should make things a lot smoother.

One problem I had was that the property that causes the issue, _owner, isn't filled in until the component is mounted in the dom. In other words, I was unable to reproduce the stack overflow in my tests. Instead I tested that React elements and components aren't modified.

CI build still failing with Zuul error.

rtfeldman commented 8 years ago

🎉 🎉 🎉 Thanks a ton @davidblurton!!!