Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

NaN in props causes observer not to rerender in React 16 #363

Closed
andrewbranch opened this issue Nov 20, 2017 · 1 comment
Closed

NaN in props causes observer not to rerender in React 16 #363

andrewbranch opened this issue Nov 20, 2017 · 1 comment

Comments

@andrewbranch
Copy link
Contributor

mobx-react/src/observer.js

Lines 140 to 142 in bc10762

if (!isForcingUpdate && isObjectShallowModified(valueHolder, v)) {
valueHolder = v
skipRender = true

The logic that sets skipRender to true, skipping a forceUpdate, checks to see if the current props and next props are shallow equal to each other with isObjectShallowModified. The business logic as I understand it is basically “if props are shallow modified, don’t call forceUpdate because React will update the component on its own.”

However, this assumption appears to be wrong as of React 16 when the value of one of those props is NaN, because while NaN !== NaN, apparently React’s own handling of NaN has changed, and React does not in fact update the component. The result is that if you pass NaN as a prop to any observer component, it will never update as a reaction to observables changing. E.g.

@observer
class Foo extends Component {
  render() {
    return /* something that hits any number of observables */;
  }
}

// elsewhere
<Foo bar={NaN} /> // This instance of Foo will never update

Live example: https://www.webpackbin.com/bins/-KzQSm3N0aWfc_U2V1BZ


I'm happy to fix this myself, but need some guidance. If we make isObjectShallowModified compare values of NaN as “not modified,” this will cause double rerenders in React 15 and below. Is that an acceptable tradeoff, or should we detect which version of React is being used?

@mweststrate
Copy link
Member

@andrewbranch feel free to improve isObjectShallowModified to assume that NaN === NaN.

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

No branches or pull requests

2 participants