mobxjs / mobx-react

React bindings for MobX
https://mobx.js.org/react-integration.html
MIT License
4.85k stars 350 forks source link

observer component re-renders even though observed value doesn't change #889

Closed bengry closed 4 years ago

bengry commented 4 years ago

Intended outcome

When an observed value doesn't change, don't re-render a component wrapped with an observer.

Actual outcome

observer-wrapped component re-renders even though observed value doesn't change.

How to reproduce the issue

See repro here: https://codesandbox.io/s/stoic-sara-v1znu?file=/src/index.tsx

  1. Click the Set param value button
  2. Check console, there's a new entry even though there shouldn't be: | [mobx.trace] 'observerView' is invalidated due to a change in: 'MyModel@11.params'

Some more details: When clicking the button, an action fires, which is supposed to do nothing - it creates a new object that's structurally identical to the previous one. I expect that the @comptued.struct (paramsInternal) will catch this and prevent further propagation. I then read this, which I tried - see the params @computed, to no avail..

Versions

mobx: 5.15.4 mobx-react: 6.2.3 react & react-dom: 16.13.1

mweststrate commented 4 years ago

In your example someProp isn't read by the computed, but by the component. The struct only works if you create non-observable data structures in your computed.

I fixed that here: https://codesandbox.io/s/practical-dream-o07hb?file=/src/index.tsx

Somehow the bug persisted though as long I had that trace() in there, so that needs some investigation.

Note that the cleaner solution to your problem is to do this._params.someProp = prop

On Mon, Jul 27, 2020 at 4:22 PM Ben Grynhaus notifications@github.com wrote:

Intended outcome

When an observed value doesn't change, don't re-render a component wrapped with an observer. Actual outcome

observer-wrapped component re-renders even though observed value doesn't change. How to reproduce the issue

See repro here: https://codesandbox.io/s/stoic-sara-v1znu?file=/src/index.tsx

  1. Click the Set param value button
  2. Check console, there's a new entry even though there shouldn't be: | [mobx.trace] 'observerView' is invalidated due to a change in: ' MyModel@11.params'

Some more details: When clicking the button, an action fires, which is supposed to do nothing

mobx: 5.15.4 mobx-react: 6.2.3 react & react-dom: 16.13.1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx-react/issues/889, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBCGJHA7MRIYZ5Y3ESTR5WLTDANCNFSM4PI4CPLA .

MoonBall commented 4 years ago

@bengry The View component don't re-render although the trace is printed. The log of [mobx.trace] only remind that the reaction maybe run agin(see trace code).The reaction doesn't rerun if it's dependencies are not changed(see shouldCompute).

bengry commented 4 years ago

In your example someProp isn't read by the computed, but by the component. The struct only works if you create non-observable data structures in your computed.

Now that you mention it, it does make sense. Is it mentioned in the docs that you need to read the individual values? In my real-world scenario I ended up doing a spread on the params, which should work just as well, for any number of properties on the _params object. Please correct me if I'm wrong here though.

@bengry The View component don't re-render although the trace is printed. The log of [mobx.trace] only remind that the reaction maybe run agin.

I stand corrected, I just put a console.count in there and it does indeed not called 👍. Good to know that a trace() log does not necessarily mean a re-computation. Is that also true for other stuff (i.e. a trace() inside a computed that depends on another observable (or a computed which at the end derives from one..). i.e. the dependant computed won't be re-evaluated even though the trace() placed inside of it is printing?

mweststrate commented 4 years ago

@bengry it depends, if things of the observed by the computed change, the computed has to recompute, if it than concludes it didn't produce a different value than before, it won't propage further, etc.