Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using mobx-react-router with mobx-state-tree #665

Closed
dbertella opened this issue Feb 16, 2018 · 16 comments
Closed

Using mobx-react-router with mobx-state-tree #665

dbertella opened this issue Feb 16, 2018 · 16 comments

Comments

@dbertella
Copy link

I opened an issue in that library already but I thought it can be more interesting to have it here.
Is it possible in your opinion to use mobx-react-router.
What I need is basically change route programmatically inside the mst actions using react-router.

What I'm doing (or trying to do) is:

import createBrowserHistory from 'history/createBrowserHistory';
import { RouterStore, syncHistoryWithStore } from 'mobx-react-router';
import Store from 'stores'

const browserHistory = createBrowserHistory();
const routingStore = new RouterStore(); // from mobx-react-router

const store = Store.create({ router: routingStore }) // Store is my main store in mst

const history = syncHistoryWithStore(browserHistory, routingStore); // from mobx-react-router

....

My main store is something like this:

const Store = types
  .model('Store', {
    auth: types.optional(AuthStore, {}),
    user: types.optional(UserModel, {}),
    router: types.optional(types.frozen)
  })

But then the syncHistoryWithStore is causing me an error Cannot assign to read only property 'history' of object '#<RouterStore>' because I guess the types.frozen don't accept mutability, am I correct?
Is there a work around, a better way / better type for this?

@alisd23
Copy link

alisd23 commented Feb 17, 2018

What about if you moved the store creation to after the syncHistoryWithStore line?
Does it still complain about immutability?

@dbertella
Copy link
Author

dbertella commented Feb 19, 2018

I just tried this briefly but it looks like is working! I'll keep this updated!
EDIT: now I got this error, not 100% sure is relevant but I think so (I think mst doesn't like the return type of the normal mobx @action)
EDIT 2: The problem here was that I was doing yield of store.push() that it wasn't returning anything, it was probably working as it is but I'm working on a mst friendlier version if you are interested

Uncaught (in promise) Error: [mobx-state-tree] Only promises can be yielded to `async`, got: [object Object]

It happen when .push method is called in a mst action, I'm trying to translate your code in a mst friendly one, let's see if I can get somewhere

@robinfehr
Copy link
Contributor

@dbertella didn't fully get the status out of the conversation, can this issue be closed?

@dbertella
Copy link
Author

Hi sorry I was distracted from many other things, the update is, it's working as it is as far as I know, so I guess we can close this issue. I was trying to rewrite mobx-react-router in a more mst way but for some reason I couldn't achieve the same functionality so I need to try again soon, might need to learn more about mobx and mobx-state-tree first. I'll keep you updated if I came up with a better solution.

@linonetwo
Copy link

Confirmed, it's working.

Just

// router.js
import createHistory from 'history/createBrowserHistory';
import { RouterStore, syncHistoryWithStore } from 'mobx-react-router';

const routingStore = new RouterStore();
const browserHistory = createHistory();
export const history = syncHistoryWithStore(browserHistory, routingStore);
export default routingStore;
// store.js
import Router from './router';

const Model = types.model({
  router: types.optional(types.frozen),
});
export const store = Model.create({ router: Router });

@dbertella
Copy link
Author

@linonetwo if you change route with a Link from react-router is it updating the reference on the mst store or it "just" happen when using the internal store.router.push method?

@linonetwo
Copy link

linonetwo commented Feb 26, 2018

@dbertella Both. To use <Link>, I also use @withRouter from react-router-dom.

@dbertella
Copy link
Author

I'm missing something because I can change route without a problem but onSnapshot doesn't get call and I don't have a new snapshot

@dbertella
Copy link
Author

dbertella commented Feb 28, 2018

@alisd23 I ended up writing my own implementation of mobx-react-router in mst, this is basically it.
I removed subscribe and unsubscribe methods because returning {...history} was causing problems. The Route were not updating for some reason. (I even tryed wrapping them in a withRouter Hoc but nothing changed)

This version seems to work perfectly for me

// RouterStore
import { types } from 'mobx-state-tree'

const Location = types.model('Location', {
  key: '',
  pathname: '',
  search: '',
  hash: '',
  state: types.frozen,
})

export const syncHistoryWithStore = (history, store) => {
  store._updateHistory(history)

  // Handle update from history object
  const handleLocationChange = location => {
    store._updateLocation(location)
  }

  history.listen(handleLocationChange)
  handleLocationChange(history.location)

  return history
}

export const RouterStore = types
  .model('RouterStore', {
    location: types.optional(Location, {
      key: '',
      pathname: '',
      search: '',
      hash: '',
      state: {},
    }),
  })
  .actions(self => {
    let history
    return {
      _updateLocation(newState) {
        self.location = newState
      },
      _updateHistory(initialHistory) {
        history = initialHistory
      },
      push(location) {
        history.push(location)
      },
      replace(location) {
        history.replace(location)
      },
      go(n) {
        history.go(n)
      },
      goBack() {
        history.goBack()
      },
      goForward() {
        history.goForward()
      },
    }
  })

// routes.js
import createBrowserHistory from 'history/createBrowserHistory'
import { RouterStore, syncHistoryWithStore } from 'stores/RouterStore'

const browserHistory = createBrowserHistory()
const routingStore = RouterStore.create({})

export const history = syncHistoryWithStore(browserHistory, routingStore)
export default routingStore
// index.js
const store = Store.create({ router: routingStore })

@alisd23
Copy link

alisd23 commented Feb 28, 2018

Hmm the { ...history } code was removed from mobx-react-router in version 4.0.1. You must be using an old version. Could you try upgrading then see if it works out of the box?

@dbertella
Copy link
Author

It doesn't update the store in my app even with the latest version.

I create a simple sandbox with mobx-react-router: https://codesandbox.io/s/l4mn1o12jm that should log the snapshot in the console and it doesn't seem to work. @linonetwo can you check if I made any stupid mistake?

This version instead https://codesandbox.io/s/913n1q60xw (with the mst transpiled version of mobx-react-router) works as expected

@marcofugaro
Copy link
Contributor

I tried wiring up mobx-react-router and mobx-state-tree as well, the routing works but it doesn't trigger the onSnapshot for me either, not even when calling router.push().

@marcofugaro
Copy link
Contributor

Basically the problem is that this line is updating only a property of a t.frozen object and not the object itself.
A solution would be to update the entire object like the example in #161, or to create a real mobx-state-tree branch like in the solution proposed from @dbertella

@alisd23
Copy link

alisd23 commented Mar 9, 2018

For anyone looking, I have now released an mst-react-router package, which hopefully should solve this problem.

@calvellido
Copy link

Hey, faced this problem just this morning and by using @alisd23 package everything works a breeze! Thank you very much.

@jadbox
Copy link
Contributor

jadbox commented Aug 20, 2018

@alisd23 I started to get this typescript error that mst renamed property Snapshot. What was it renamed to? alisd23/mst-react-router#10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants