dustingetz / react-cursor

Immutable state for React.js
1.03k stars 50 forks source link

Can we detect and warn on out-of-band mutations? #54

Closed dustingetz closed 8 years ago

dustingetz commented 9 years ago

Prevents #47. I'm not entirely convinced that it's possible to detect out-of-band mutations. I haven't thought it all the way through.

One idea, is that the cursor update methods (set, push, etc.) can hang on to a reference to the final value right before they pass it to react setState. Then in each subsequent invocation to a cursor update method, we can recursively check that the current cursor.value is reference-equal to the previous value at all nodes of the tree, before performing the update. This would have performance implications.

dustingetz commented 9 years ago

Background reading: https://github.com/facebook/react/issues/1753

dustingetz commented 9 years ago

Can we deep freeze react state? https://github.com/stample/atom-react/blob/master/src/utils/deepFreeze.js

dustingetz commented 9 years ago

We could also use Object.observe (recursively - so implement deepObserve)

dustingetz commented 9 years ago

@ezmiller, do you want to take a whack at an Object.freeze approach? We need to account for browser support, what can be polyfilled, and whether an O.freeze or O.observe or other approach is best.

ezmiller commented 9 years ago

Sorry, I've been away, but will catch up with all this soon.

dustingetz commented 9 years ago

O.O is looking pretty dead now https://esdiscuss.org/topic/an-update-on-object-observe

lijunle commented 9 years ago

I look into the code of React. They just shadow freeze the element.props, as said in its document.

However, there is a way to do deep freeze mentioned in MDN examples.

@dustingetz Which option do you prefer? Shadow or deep?

dustingetz commented 9 years ago

I prefer deep, and since we only need to re-freeze the leaf subtree that comes out of react.addons.update, i dont think it will be that slow either.

dustingetz commented 9 years ago

re-open for 2.0

dustingetz commented 8 years ago

Merged into 2.0 (now master)