mhaagens / react-mobx-react-router4-boilerplate

React, React-Router 4, MobX and Webpack 2-boilerplate with async routes.
560 stars 137 forks source link

Recommended way to add multiple stores #29

Closed gilesbutler closed 7 years ago

gilesbutler commented 7 years ago

Hi @mhaagens

Thanks for the awesome boilerplate!! I'm just wondering what would you recommend as the best way to add multiple stores. I've been reading issue #1 where you implemented the DataFetcher and I see that it relies on injecting the default store you setup @inject(['store']).

I'd like to fetch data for a different store but can't figure out the best way. I'm currently trying to pass through a store to the DataWrapper's @inject decorator but when I do the component then isn't passed through. Appreciate any help or tips you can offer :)

Thanks

orouz commented 7 years ago

this is what i did:

1) under stores folder, create as many stores you need. (as in- uiStore, authStore, etc). export each store as a singletone (as in- const store = new Store(); export default store

2) inside your stores folder, have an index.js file that exports all the stores in this folder.

3) from index.js, use import * as stores from 'your-stores-path

4) pass {...stores} down as props to the <Provider> from <App>

5) when you need a certain store in a certain component, use @observer(['store1','store2'])

this seems to work nicely for me. hope it helps! :)

gilesbutler commented 7 years ago

Hi @orouz, thanks for that, unfortunately this still doesn't work with the DataWrapper component that comes with this boilerplate. Or have you found a way to make it work?

From what I can see the DataWrapper still needs to know which store to call fetchData on...

export default function DataWrapper(Component) {
  @inject(['store']) @observer
  class DataFetcher extends React.Component {
    constructor(props) {
      super(props);
      this.store = this.props.store;
    }
orouz commented 7 years ago

@gilesbutler hey! here's quick solution for that:

export default function DataWrapper(store) {
return (Component) => {
  @observer([store])
  class DataFetcher extends React.Component {
    constructor(props) {
      super(props);
      this.store = this.props[store];
    }
   // ... the rest is the same
}

and use it like this: @DataWrapper('store') (seems like it has to be the first decorator though)

note that DataWrapper assumes the store has a fetchData function. it would probably be better to extract it from the store into the DataWrapper component instead of duplicating it across stores. it also assumes data is stored at store.items / store.item.

sorting it out could be nice. i could try send a PR. any thoughts on this @mhaagens ?

gilesbutler commented 7 years ago

Thanks @orouz :) so @observer([store]) produces a warning in mobx 4.0...

Mobx observer: Using observer to inject stores is deprecated since 4.0. Use @inject("store1", "store2") @observer ComponentClass or inject("store1", "store2")(observer(componentClass)) instead of @observer(["store1", "store2"]) ComponentClass

I fixed it by doing this @inject([store]) @observer.

I agree about fetchData so extracted it into the DataWrapper...

async fetchData(pathname) {
  const { data } = await axios.get(`http://localhost:3000${pathname}`);
  return data.length > 0 ?
    this.store.setData(data) :
    this.store.setSingle(data);
}

It now calls this.store.setData() or this.store.setSingle() which don't have any assumptions about how your data is stored. So in your store you can have something like...

@action setSingle(data) {
    this.todo = data;
}

I renamed the clearItems() method to clearData()' which makes a bit more sense as we havesetData(). That way theDataWrapper` can call...

componentWillUnmount() {
    this.store.clearData();
}
orouz commented 7 years ago

@gilesbutler this looks good! thanks :)

gilesbutler commented 7 years ago

Thanks @orouz, I'll close this issue now.

Still keen to hear @mhaagens thoughts and take on it :)

yjose commented 7 years ago

Hi friends, I want to use Hot reload with multiple singleton stores any idea @gilesbutler ??

gilesbutler commented 6 years ago

Sorry @yjose only just seen this!

I'm afraid I don't know the best way to implement that

DunkeeFunk commented 6 years ago

this is a terrible idea ... if your app is going to be large and complex you are in for a world of pain and weeks back tracking

gilesbutler commented 6 years ago

Hey @DunkeeFunk do you mean @yjose's question or the main issue?

DunkeeFunk commented 6 years ago

No your solution .. im working with a similar set up and it gets very clumsy passing around a big store with mobx ... I am trying to figure out a way of separating out the stores even further.. there has to be a cleaner way to do this

gilesbutler commented 6 years ago

Ah gotcha, I haven't had too many problems tbh but our app isn't the biggest, yet!