mobxjs / mobx-react

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

Conditional useObserver hook #864

Closed AjaxSolutions closed 4 years ago

AjaxSolutions commented 4 years ago

This is a feature request. I'm looking for a way to use the useObserver hook conditionally.

Depending on the state of my component, I'd like to be able to suspend and then resume the useObserver hook.

danielkcz commented 4 years ago

No hooks can be used conditionally. Use <Observer> component for conditional reactivity.

Also, if you are attempting to optimize by avoiding unnecessary re-renders, consider restructuring your stores so you can control which changes will cause rendering.

AjaxSolutions commented 4 years ago

@FredyC This is what I had in mind. https://twitter.com/DavidKPiano/status/1228700861024604160 Is there a way to use hooks such as the useObserver hook in a way similar to "subscribable" objects. https://codesandbox.io/s/subscribable-context-e4j31

danielkcz commented 4 years ago

Streams are obviously a whole different paradigm. Feel free to use those or even combine those together (reactions pushing data to streams which you can filter).

MobX is about declarative observability so you never have to doubt that component will be up to date every time. When you introduce conditionals and have a bug in there, it will be really hard to track down.

My advice: Don't optimize before you measure. Unless re-renderings are causing a bottleneck to your app code, it's not worth the trouble.

AjaxSolutions commented 4 years ago

OK, I'll close this ticket.

mweststrate commented 4 years ago

note that useObserver is already a subscription mechanism, and way way smarter than any stream subscription, for example observables in unused branches won't be subscribed to, whereas stream subscriptions are much more coarse grained and dumber. So formulating the problem as generic as you do, it sounds to me like you are asking for something that is worse than what you have currently with useObserver. Now I don't know your use case, but make sure you are not asking for this based on a theoretical exercise, but rather on measured performance. Otherwise by trying to pull some clever trick might very well be making things worse, not better.

On Sat, May 9, 2020 at 12:55 PM Les Szklanny notifications@github.com wrote:

Closed #864 https://github.com/mobxjs/mobx-react/issues/864.

— 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/864#event-3319317897, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBA5LYWKRJIG4PSSKGLRQVABRANCNFSM4M4XYAUA .

AjaxSolutions commented 4 years ago

Thank you for your answer @mweststrate .

for example observables in unused branches won't be subscribed to

Could you please explain what you mean by "unused branches" and how they are handled?

danielkcz commented 4 years ago

@AjaxSolutions I believe he means something like this. To be honest, I did not know about this either. That basically gives you ability for conditional observability.

const store = observable({
  mode: 'in',
  inBranch: 10,
  outBranch: 20
})

export default function App() {
  console.log('RENDER')

  React.useEffect(() => {
    store.outBranch = 30
  }, [])

  return useObserver(() => (
    <div>{store.mode === 'in' ? store.inBranch : store.outBranch}</div>
  ))
}

https://codesandbox.io/s/angry-sunset-doer5?file=/src/App.js

Note: initial 2 renders is normal, happens even if you comment out mutation

AjaxSolutions commented 4 years ago

Very interesting. Thank you for the codesandbox @FredyC .