inject with custom mapper function is not reactive

[nassimbenkirane]

I read about 'mobdux' and I wanted to give this pattern a try, using inject to decouple the store values and the component. Unfortunately in some cases, the view does not update with the store changes

Simple example :

When using inject with a custom function like so :

const ListView = inject(({ store }) => ({
  list: store.state.list,
  addItem: store.addItem,
}))(({ list, addItem }) =>
  <div>
    <button onClick={addItem}>Add Item</button>
    <ul>
      {list.map(elem =>
        <li>
          {elem}
        </li>,
      )}
    </ul>
  </div>,
)

with list being a list of strings, the changes on store.state.list do not reflect on the view.

In order to have the view react to the changes on the todo list, I have to use observer on top of inject :

const ListView = inject(({ store }) => ({
  list: store.state.list,
  addItem: store.addItem,
}))(observer(({ list, addItem }) =>
  <div>
    <button onClick={addItem}>Add Item</button>
    <ul>
      {list.map(elem =>
        <li>
          {elem}
        </li>,
      )}
    </ul>
  </div>,
))

I'm not sure what is the expected behaviour, but I think it is better if the view updates automatically with inject

For now I am using a custom function connect :

const connect = mapStoresToProps => WrappedComponent => (
    inject(mapStoresToProps)(observer(WrappedComponent))
)

so that we don't have to manually call observer each time.

I am guessing it might also be related to this comment on the 'mobdux' post that uses inject internally : https://medium.com/@vlopezsalvador.22/great-article-cameron-fletcher-could-you-give-me-a-hint-about-this-issue-i-am-having-6cdef64f5db8

Anyway, great work on Mobx ! Amazing DX 👍

[urugator]

The problem is that store.state.list subscribes for a change of the list property, but not for a change of the array (value) itself. It would work if you would do:
this.state.list = [...this.state.list.slice(), "newItem"]; (slice is used because spread syntax won't work with ObservableArray)
Always remeber to call array.slice() or mobx.toJS(array) before passing an array to a non-observer:

inject(({ store }) => ({
  list: store.state.list.slice() // or toJS(store.state.list),
  addItem: store.addItem,
}))

This would work as well, but it's a bit "artificial/hacky" solution:

inject(({ store }) => ({
  listLength: store.state.list.length, // now we have subscribed for a change of the array itself 
  list: store.state.list,
  addItem: store.addItem,
}))

The difference between the slice and toJS is that toJS recursively accesses all observables and turns them into plain JS structures, loosing the "observability", but causing the component being subscribed for all these accessed observables. The slice only turns the array into plain JS array, causing subscription for the array itself only and maintaining the "observability" of the array elements.

[nassimbenkirane]

Hello @urugator, thanks for the feedback.

Indeed, converting the observableArray to a plain old javascript array would work. BTW I think that is the way 'mobdux' works.

However, even if the inject function was not designed to be used way, I do think that providing a method in the public API that would inject + observe would be great.

If I understand correctly, it is the reason why inject was made reactive to the mapper function in the first place, according to #111 and the SO post linked in the issue.

I am thinking of a Redux-like connect function , as mobdux is doing, or as used in mobx-react tests themselves.

What do you think ?

[urugator]

Imho connect like approach goes against the MobX philosophy.
MobX automatically manages depedency graph based on how the data are actually being used, so you don't have to worry about manual (re)subscription.
If you introduce mapper, even though mapper itself is a reactive function, you're manually stating dependencies without actually knowing if and how they're being used by the component.
I understand that's basically the point, because you want to decouple the component from a store, but at the same time it's also the source of the problem.
For a generic connect like solution, I think that only reasonable assumption about the prop usage is to presume that the whole object subgraph is needed (mobx.toJS(prop)). Which is what mobdux does afaict.
However it kinda kills the component composability or at least the performance benefits that MobX can offer (ability to re-render only the child component).
I really don't think that inject should be used to remove the need for observer.

If it's just about being lazy to write inject and observer, then helper function doing both makes sense. Whether it should be part of the package, I dunno. Speaking for myself, I woudn't mind having it, but I don't need it (at least for now). Should there be a decorator version as well?

[nassimbenkirane]

Thanks for you answer !

You're probably right. In the end, I guess it's about being lazy to write inject and observer together :).

I'm a big fan of how Mobx automagically handles subscription for us. At the same time, most of the examples I found (and most of the components I wrote) use directly values from the store in the components. In our codebase, this results in parsing and formatting in render functions and our components tend to become less and less readable.

IMHO it seems great in terms of separation of concerns to have a layer of abstraction so that we can better decouple the view and the stores.

I guess there are (at least) two ways to implement a connect like functionality with mobx-react

connect(({mainStore}) => ({
    list: mainStore.state.list
}))({list}) => (
    <ul>...</ul>
))

1. One way is to provide non-observable values in inject function, and use those plain values in the view. That is what Mobdux does. The previous code would be roughly equivalent to :

inject(({mainStore}) => ({
    list: mainStore.state.list.peek() // or Mobx.toJS
}))({list}) => (
    <ul>...</ul>
))

2. Another way is to pass observable values to the view and make it an observer

inject(({mainStore}) => ({
    list: mainStore.state.list // Mobx ObservableArray
}))(observer({list}) => (
    <ul>...</ul>
)))

I can see how using the 1st approach would go against mobx's philosophy because we would be declaring the subscriptions manually.

With the second approach, though, I think it's not about declaring subscriptions, but rather to abstract them for the view as readable variable with names that make sense. It seems to provides a very good solution to the challenge of separating the view and the data, while keeping the views reactive.

Do you see disadvantages in using the 2d way, or should I expect to run into problems using this pattern ?

P.S. I guess the @connect decorator would make sense too

[urugator]

If you use it only as an adapter to bridge between store and component API, I wouldn't expect any problems.
However the component API must be still designed with the observables in mind (otherwise the observer wouldn't make sense).
For example remember that once you "unwrap" the observable, you loose the observability, therefore you can't pass it to descendant components as an observable object.
You may actually want to do something like:

inject(store => {
  name: computed(() => store.name) // store.name is string
})
// now you can still pass the "name" alone to descendants while keeping it observable

So you can get rid of the direct dependency on store, but you won't get rid of the depedency on observables.

Whether you need such abstraction is up to you.
My personal advice with abstractions in general is to introduce them only if you actually need them (this need usually emerges during development). Don't use
(seems|would|could) - (cool|great|nice) - (in terms of|for the sake of) - (some generic principle)
as a reason.

this results in parsing and formatting in render functions and our components tend to become less and less readable.

Can you elaborate? Post an example perhaps?

[mweststrate]

this results in parsing and formatting in render functions and our components tend to become less and less readable.

For clarity; it is not the code literally in the render method that is tracked; it is all the data that is accessed during render. So it is perfectly fine to have all that code in your own, completely UI anaware utility libraries. The only thing that matters is that they are invoked during render

[mweststrate]

Closing as original question seems answered.