Nested Models, and actions dependencies

[fabienjuif]

Hi there :)

I use mobx-state-tree for a project, and I would like to use it as my main store.
I have nested models and I would like to avoid having a graph definition, and instead I would like to have a tree (composition).
So I tried something that I explain below, but I don't like it. I ask some question at the end, feel free to ask more if you don't understand 👍

Store

Actions with getRoot

What I do right now, while my user needs to login is :

• I don't like the idea that my Auth model has to know the Router model
• Because I don't like my store to be a graph

Actions with onAction from Store model

So I tried to use onAction into the Store model, listening to login then calling goToList :

• I like this way of writing actions dependencies, because each leaf on my tree are independants
• This is easier to test, easier to understand, and all I have to do is to write dependencies into onAction
• We may say this is a bit like a redux-saga implementation

What's the problem ?

The problem is that this onAction way of writing my actions dependencies works well with synchronous actions, but not with asynchronous one (?? this is to confirm)

• login action is asynchronous
• goToList is synchronous
• My login action is triggered and caught by onAction at the beginning, not a the end of the process
• I redirect to my List screen (via goToList) before my user is logged, that is not fine here.

What's the solutions ?

• I read about this issue : Mutation events for custom JSON patch #481
  • I don't like the idea of listening a value
  • I would prefer to listen to action
  • If I listen to a value, I have to check (and then write code) what changed, how it changed, before triggering an other action
  • This is a bug source for me

Question

• Is it a good pattern to have nested models ?
• Should I move all my actions into the root model ?
• Is it a good idea to move from a graph pattern to a tree pattern ?
• How do you handle actions dependencies between models ?

Thank you !

[mattiamanzati]

Uhm, have you considered using addMiddleware instead of onAction? It will give you more control over async actions :)

[fabienjuif]

Yes I can try that 😃
I would like to know what you think about the approach

[mattiamanzati]

I think that both are valid ones, the latest one is more agnostic about how the data is shaped, ideally you can implement a service-command pattern based on that :)

[mikecann]

I personally like the look of it as a tree @fabienjuif, better than a graph like you described in your last one.

I would definitely be interested to see if you managed to get it working with async. Perhaps it could be a blog post?

[mikecann]

I did have one thought tho.

Wouldnt it be more "reactive" if your root Store listened to a "isLoggedIn" property on your AuthStore then redirected the Routes Store to "gotoList" when "isLoggedIn" value becomes "true" ?

Im not sure if thats a better approach than listening to the action itself however...

[fabienjuif]

Hi @mikecann I will post here my progress.

About property checking and reacting to them, I though about it, but I don't like it neither:

• if the model itself doesn't need it I think this is a bad practice to valuate boolean just "in case someone listen to me" and it add noises to the model itself don't you think ?
• I have the feeling that listening to action is
  • more bulletproof
  • more futurproof
  • and allow us to share models without thinking about others.

[mweststrate]

Note that you can setup reactions in the afterCreate of your RouterStore, to listen indeed to something from which you can deduct that the user is now logged in. It might feels as closer coupling indeed as your approach, but the advantage is that it is also typecheckable :) Which is a downside of your proposal.

That being said, the thing you propose should work just fine as well

[luisherranz]

In the Flux/Redux world, this is usually solved using two actions for async actions. Actually, three actions: LOGIN_REQUESTED, LOGIN_SUCCEED, LOGIN_FAILED.

Router can subscribe (listen) only to LOGIN_SUCCEED actions and act accordingly.

If you don't want to pollute your stores with app logic, you can leave that to external actors. Like you mentioned, something similar to redux-saga, but you'd need an external dispatcher.

This is just an idea about a possible implementation without using an external dispatcher:

const Auth = types.model('Auth')
.props({
  logging: false,
  error: types.maybe(types.string),
  userId: types.maybe(types.string),
})
.actions(self => {
  const loginRequested: () => { self.logging = true; login(); };
  const loginSucceed: userId => { self.logging = false; self.userId = userId };
  const loginFailed: error => { self.logging = false; self.error = error };

  const login = flow(function* () {
        try {
           const userId = yield fetch(...); // your login logic
           self.loginSucceed(userId);
        } catch (error) {
           self.loginFailed(error);
        }
  });
  return {
    loginRequested,
    loginSucceed,
    loginFailed,
  }
}));

Then in the middleware, you capture the auth.loginSucceed events and launch router.gotoList() actions.

[mattiamanzati]

Just note that
loginRequested,
loginSucceed,
loginFailed,
won't dispatch middleware actions unless they are called as self.loginRequested. See #456

[luisherranz]

Thanks @mattiamanzati. I have edited loginSucceed and loginFailed.

auth.loginRequested() should be called from your Login Screen.

[mikecann]

@luisherranz in your example, wouldnt that then expose "loginSucceed" and "loginFailed" to the outside world thus breaking encapsulation? I know this is sort of what is required here but is this a good idea for clean code?

[fabienjuif]

@luisherranz yes this is one of the possibles implementations of the Auth Model.

But I don't see how it preserve us from doing a middleware that listen to actions 🤔
I have the feeling the issue is the same.

[luisherranz]

@luisherranz in your example, wouldnt that then expose "loginSucceed" and "loginFailed" to the outside world thus breaking encapsulation? I know this is sort of what is required here but is this a good idea for clean code?

@mikecann Yes, in a Redux/Flux system all actions are public!

@luisherranz yes this is one of the possibles implementations of the Auth Model.
But I don't see how it preserve us from doing a middleware that listen to actions 🤔
I have the feeling the issue is the same.

@fabienjuif The whole point was that having a separate loginSucceed can make things easier. Router has to listen to login attempts somewhere. It may be in a middleware or somewhere else.

That said, I think the ideal scenario (if going the Flux route) would be having an external dispatcher and manage the logic in async/generator functions, like redux-saga.

[fabienjuif]

I wrote a middleware that I can use like this :

  const dispatch = (action, tree) => {
    const { fullpath, ended } = action

    if (fullpath === '/auth/login' && ended) {
      tree.ui.router.goToList()
    }
  }

  addMiddleware(store, trampssReact(dispatch))

It works well right now 😄

I still have a little issue (maybe it's not): fullpath has "/auth/login" twice :

• once as an action (when it is triggered and before the process start
• and once it ends (process_return).

That's why I added ended parameter in action to catch the end of the action.

I will create a new repo with this first middleware implementation.
This is a simple code base, but I have to clean it first :)

I come back soon.

[fabienjuif]

Voila: https://github.com/Trampss/trampss-mst-onaction

In fact, after all the digging this is just a simpler API of a middleware 😛

The function given to onAction will receive two arguments :

• the action
• the tree (attached)

The action has :

• path
• name
• fullpath (path + name)
• ended (if present and set to true, this is an asynchronous action that is ended)
• args

[luisherranz]

Looks great @fabienjuif!

[mikecann]

@fabienjuif thats cool! Would love to see some tests that it works under all situations (errors, async, non-async) etc

[fabienjuif]

Right now @mikecann it works with

• async actions (the dispatch function will be triggered at both start and end of the asynchronous action, at the end the boolean ended is set to true)
• sync actions (you don't need to look at ended)

I use it right now for theses both cases in my project.

Errors -> this is something to dig, I haven't test this yet

[luisherranz]

Is there a way to know if the action is async in the first place? Something like { async: true, ended: false }.

[fabienjuif]

Yes it would be great, but I don't see how I can have this information the first time the action is triggered.

In the call parameter given to middlewares, async function and sync function seems to have same signature the first time they are triggered. I know this is an asynchronous action at its ends because the call type is process_return, but I didn't find the kind of information from the first trigger.

@mattiamanzati can you confirm that ?

edit:

right now I try to write some helpers to match actions :)

[mweststrate]

@fabienjuif you can detect it by checking in your action middleware whether a process_spawn was emitted after calling next but before you return from the middleware that intercepted the action.

The action tracking middleware makes that process much simpler btw. Give me some time to publish that (you can see how it is used in the Undo/Redo middleware

https://github.com/mobxjs/mobx-state-tree/blob/master/packages/mobx-state-tree/src/middlewares/create-action-tracking-middleware.ts

[fabienjuif]

I changed the API a bit in this PR : unirakun/k-mst-onaction#6 (comment)

It uses helpers, I'm not really sure about this.
edit:

On this PR I try an array implementation : unirakun/k-mst-onaction#7

What do you think is better as a user perspective ?

[wbercx]

@fabienjuif I do like the array implementation! Do you have any thoughts/preferences for dealing with map keys or array indexes being part of the fullpath?

// For example, if prospects was a map and each prospect had a `save` action:
take.ended('/prospects/287bc0ae-6518-4a55-ad59-430372338ef3/save', () => ...)
take.ended('/prospects/51e8fc96-2160-433c-b009-b59cdf7c5a20/save', () => ...)

I think it would be handy to be able to capture the save on any prospect.

[mattiamanzati]

@wbercx I see that @fabienjuif added support for regexps, maybe he can add support for https://github.com/pillarjs/path-to-regexp

take.ended('/prospects/:id/save', () => ...)

[fabienjuif]

I could :)
I am waiting for a review for the "new API" PR to be merged ;)

[fabienjuif]

I have the feeling the main issue from this topic is resolved.
If you want to help me improve the first version of the middleware, we can discuss into its github repository ? https://github.com/Trampss/trampss-mst-onaction

I close this topic so the *mobx-state-tree issue tab is less flooded :)

If you don't agree, feel free to argue ;)