mobxjs / mobx-react

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

Feature Request: hook instead of observer #786

Closed tomitrescak closed 5 years ago

tomitrescak commented 5 years ago

Been Mobx user for several years and I got too comfortable with your awesome solution, desiring even more comfort ;) I apologise. The problem I have is the necessity to wrap components with the observer function or use <Observer /> component.

Recently I stumbled upon Overmind, which intends to do similar, but in MUCH more complex way, so Mobx is still the kind.

YET

I really love the useOvermind hook, that is created by the createHook(store) function.

With this hook, no observer is necessary and code is much cleaner. Would you consider something like this for MobX React?

Thanks

tomitrescak commented 5 years ago

Ha! I found a nifty example online:

https://blog.mselee.com/posts/2019/06/08/using-mobx-with-react-hooks-typescript/

You can close this if you do not wish to add this into core. I think it is really useful!

danielkcz commented 5 years ago

I don't know Overmind and I don't think I want to dive deep into it. Can you summarize it? Is it providing some sort of reactivity as well or why do you compare it to MobX?

To be honest, when mobx-react-lite was starting, I had a similar concern to yours, but sadly we weren't able to find a proper solution.

Then there was other glimpse of hope, but nothing viable came out of it either.

We even tried approach through macros, but that also turned out the wrong way around.

In the end, it's just a matter of getting used to it. It's surely a MobX tradeoff, but nothing too horrible in hindsight imo.

danielkcz commented 5 years ago

Hm, I am not sure part of that article made you happy, the useObserver? Be careful about it. It's essentially different syntax for <Observer>, it won't see any observables outside its boundary.

Also, we have official docs, no need to search the internet for that :) https://mobx-react.js.org/recipes-context

danielkcz commented 5 years ago

Ok, I actually read through the article and there is indeed an interesting solution through a selector function 💡

import React from 'react';
import { useObserver } from 'mobx-react-lite';

export const useStoreData = <Selection, ContextData, Store>(
  context: React.Context<ContextData>,
  storeSelector: (contextData: ContextData) => Store,
  dataSelector: (store: Store) => Selection
) => {
  const value = React.useContext(context);
  if (!value) {
    throw new Error();
  }
  const store = storeSelector(value);
  return useObserver(() => {
    return dataSelector(store);
  });
};

I haven't tried, but it indeed looks like a promising solution.

I wonder who the author is, there isn't much info about him/her, I would like to give some kudos.

@mweststrate What do you think? Do you see some caveats in such a solution? It also effectively removes the limitation of destructuring if the selector picks on actual primitive values 😮

tomitrescak commented 5 years ago

Yah, I played with it a bit and there are two things:

PROS:

CONS

danielkcz commented 5 years ago

Yea obviously no solution is perfect, here you are trading off the verbosity for a slightly less magical code, observer logic is hidden away so a newcomer won't be surprised too much.

  • not as simple as just accessing the value from the store,

True, but you can freely destructure in the selector and assemble the new object from various other stores. It's probably very close to connect from Redux (as I remember it).

  • you need to access all observable values before all your code, since you are using hooks

Also true, in more complex components it could be a fair annoyance to synchronize two places in the code.

  • possibly no conditional flow ?

Not sure what do you mean by that. Observer won't allow for that either. I mean if you have a condition in rendering, it will trigger rerender no matter if that condition got evaluated.

urugator commented 5 years ago

The whole essence of mobx is that the subscription happens based on actual usage. Once you introduce any kind of selector/connect function you can sort of throw it away as it's is no longer responsible for resolving dependencies. The idea really is that dependencies are defined by the "derivation" (render function) - so that if you modify the derivation, the subscriptions will naturally adapt in a way that there won't be any missing or redundant subscriptions.

I mean if you have a condition in rendering, it will trigger re-render no matter if that condition got evaluated.

But it won't re-render if observable behind the condition is modified and the condition remains falsy:

render() {
  if (state.a) { // falsy
    // anything accessed inside this block is exluded from subscription until state.a is modified    
  }
}
danielkcz commented 5 years ago

Yea, you are right @urugator (as most of the time). It would introduce confusing behavior unless you would write selectors to pick up on every primitive value. That's especially problematic for arrays, it would require usual hacks like Array.from and similar.

Oh well, I guess we really cannot get rid of observer :) There is at least some consolation price that we can now get a warning when we forget the observer..

lock[bot] commented 4 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.