Mobx-state-tree: production build seems to shrink asynchronous actions name (middleware)

[fabienjuif]

version: 1.1.0 / 1.0.0

I have an action named login, at start I can catch it (type action) :

{
  fullname: "/auth/login",
  name:"login",
  path:"/auth",
}

At the end I can't catch it (type flow_return) :

{
  fullpath: "/auth/t",
  name: "t",
  path: "/auth",
}

I saw this bug using trampss-mst-onaction.
But I wanted to try with a simpler middleware to see if the error comes from me :

  addMiddleware(store, (call, next) => {
    const { type, name } = call
    console.log('bug ?', { type, name })

    next(call)
  })

And it seems that this is the build version of mobx-state-tree :(

[mweststrate]

It is probably not related to mst itself but the uglifier that minimizes
the function name of your flow. How is login declared?

On vr 27 okt. 2017 18:42 Fabien JUIF [e-mail] wrote:

[image: image]
https://user-images.githubusercontent.com/17828231/32114837-e296908a-bb44-11e7-8804-6575b978765c.png

version: 1.1.0 / 1.0.0

I have an action named login, at start I can catch it (type action) :

{
fullname: "/auth/login",
name:"login",
path:"/auth",
}

At the end I can't catch it (type flow_return) :

{
fullpath: "/auth/t",
name: "t",
path: "/auth",
}

I saw this bug using trampss-on-action.
But I wanted to try with a simpler middleware to see if the error comes
from me :

addMiddleware(store, (call, next) => {
const { type, name } = call
console.log('bug ?', { type, name })

next(call)

})

And it seems that this is the build version of mobx-state-tree :(

[image: image]
https://user-images.githubusercontent.com/17828231/32115285-81085cf2-bb46-11e7-92b7-575b789d5739.png

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx-state-tree/issues/492, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhG3Fp3C8_YCAu85cB3Ti4O_LJXXBks5swgffgaJpZM4QJUnq
.

[fabienjuif]

export default types
  .model({
    user: types.maybe(User),
  })
  .actions(self => ({
    setUser: (user) => {
      localStorage.setItem('user', JSON.stringify(user))

      self.user = user
    },
  }))
  .actions(self => ({
    init: () => {
      const raw = localStorage.getItem('user')
      if (raw) self.setUser(JSON.parse(raw))
    },
    login: process(function* login(auth) {
      // query
      const query = gql`
        query {
          user (email: "${auth.email}", password: "${auth.password}") {
            id
            token
          }
        }
      `
      const { data: { user } } = yield client.query({ query, fetchPolicy: 'network-only' })

      // update store and localstorage
      if (user) self.setUser(user)
    }),
    logout: () => {
      localStorage.removeItem('user')
      self.user = null
    },
  }))
  .views(self => ({
    get token() { return self.user && self.user.token },
    get logged() { return !!self.token },
  }))

I thank about the uglifier, but then how the type action retrieve the right name ?

[fabienjuif]

Ok https://github.com/mobxjs/mobx-state-tree/blob/master/packages/mobx-state-tree/src/core/flow.ts#L43

The name for asynchronous actions is not the field name declared into _mobx-state-tree_ types.actions, but this is the function name provided to flow decorator:

    login: flow(function* /* this is it -> */ login(auth) { }

So this is why uglifier shrink it :(

[fabienjuif]

@mweststrate @mattiamanzati

Wild idea, what if we declared asynchronous action this way ?

const Store = types
  .model({})
  .actions(self => ({
    login: function* login() { /* ... */ },
  })

And then types.actions decorate himself the action if it detects this is a generator ? (at instanciation). So it can catch the good name !

Then into flow implementation we can add a parameter, the name (which is the field from the object returned by the types.actions callback) :

export function flow(name: string, asyncAction: any): any {
    return createFlowSpawner(name, asyncAction)
}

Advantages :

• We don't import an other API as mobx-state-tree user (flow is not needed to be public anymore)
• We declare actions almost the same way (synchronous and asynchronous ones)
• Uglifier shrink is avoided

Drawbacks:

• flow API should be changed
• this is a breaking change (?)

edit:
TL&DR: move the flow decorator responsability from developer to mobx-state-tree, and then it will patch the uglifier bug.

edit2: warning added to trampss-mst-onaction

[fabienjuif]

Something like this: https://github.com/fabienjuif/mobx-state-tree/pull/1/files

I have to test it, I am really not sure about this.

[mattiamanzati]

We had something like that, the problem is that unfortunatelly if generator gets transpiled the check we used wont work :/
Please have a look at #456 , we could attach to the returned function a hidden method that will be called when the function will be attached to the MST instance.
So we will know the exact name used in the return of the .actions()

[fabienjuif]

That's just what i try here @mattiamanzati I think : https://github.com/fabienjuif/mobx-state-tree/pull/2/files

edit: as I come to the transpiled conclusion :( https://github.com/fabienjuif/mobx-state-tree/pull/1/files#r148020321

edit2:

• Add a field $mst_flow into flow decorator
• createFlowSpawner returns a new function which take the name
• and then in model.ts :

if ((action as any).$mst_flow) {
  action = (action as any).spawner(name)
}

[mweststrate]

btw, from top of my head you can use flow("name", function *() {}) to
preserve name, and otherwise maybe we should add that?

Op di 31 okt. 2017 om 16:07 schreef Fabien JUIF [e-mail]:

That's just what i try here @mattiamanzati
https://github.com/mattiamanzati I think :
https://github.com/fabienjuif/mobx-state-tree/pull/2/files

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx-state-tree/issues/492#issuecomment-340792440,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhH9HVkuxjMyxwBaW-Zz7JyzF7h8Jks5sxzfHgaJpZM4QJUnq
.

[fabienjuif]

@mweststrate I prefer not to specify name 3 times (action field, into flow parameter, and as a named function)

I think with this start of solution it could work: https://github.com/fabienjuif/mobx-state-tree/pull/2/files

I will read #456 as @mattiamanzati suggests

[mattiamanzati]

@fabienjuif I'll write some pseudocode tomorrow, or maybe even an implementation :)

[mweststrate]

I agree :) (although there is no reason anymore to name the function, but
still)

Op di 31 okt. 2017 om 16:11 schreef Fabien JUIF [e-mail]:

@mweststrate https://github.com/mweststrate I prefer not to specify
name 3 times (action field, into flow parameter, and as a named function)

I think with this start of solution it could work:
https://github.com/fabienjuif/mobx-state-tree/pull/2/files

I will read #456 https://github.com/mobxjs/mobx-state-tree/issues/456
as @mattiamanzati https://github.com/mattiamanzati suggest

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx-state-tree/issues/492#issuecomment-340793827,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhHN6bVjRuL4yVAUG7_jqMwfIS6Z-ks5sxzi8gaJpZM4QJUnq
.

[mweststrate]

:+1:

Op di 31 okt. 2017 om 16:13 schreef Michel Weststrate <[email protected]

:

I agree :) (although there is no reason anymore to name the function, but
still)

Op di 31 okt. 2017 om 16:11 schreef Fabien JUIF <[email protected]

:

@mweststrate https://github.com/mweststrate I prefer not to specify
name 3 times (action field, into flow parameter, and as a named function)

I think with this start of solution it could work:
https://github.com/fabienjuif/mobx-state-tree/pull/2/files

I will read #456 https://github.com/mobxjs/mobx-state-tree/issues/456
as @mattiamanzati https://github.com/mattiamanzati suggest

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx-state-tree/issues/492#issuecomment-340793827,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABvGhHN6bVjRuL4yVAUG7_jqMwfIS6Z-ks5sxzi8gaJpZM4QJUnq
.

[fabienjuif]

@mattiamanzati It will be great if you look at my second PR linked above and tell me what's wrong with this idea (so I can understand 😊)

[mattiamanzati]

https://github.com/fabienjuif/mobx-state-tree/pull/2/files#diff-060ef979aa0afd5ee7c8fe9cbd2b69edR43
The problem is that you are returning a object instead of a function, so this would break :)

.actions(self => {
   const internalSave= flow(function*(){

   })

  return ({
    save: flow(function*(){
       // do stuff...
       yield internalSave()
    })
  })
}

[mattiamanzati]

@fabienjuif https://github.com/mobxjs/mobx-state-tree/issues/456#issuecomment-339961999 replace "action" with flow, and you'll basically get the gist :)

[fabienjuif]

Ok I understand it now :) Thank you !

[mattiamanzati]

Ideally I would love a "createDecorator(fn)" api that returns a new function, and copy the metadata from the previous fn (like we do with $mst_middlewares)

[fabienjuif]

We could decorate the function returned by createDecorator(fn), and inject the action name via model.ts like I did with my second PR.

But your snippet still not works with this idea:

.actions(self => {
   const internalSave= flow(function*(){

   })

  return ({
    save: flow(function*(){
       // do stuff...
       yield internalSave()
    })
  })
}

How do internalState knows the action name?
What is the action name here ? internalState or save?

I'm getting confused.

[mattiamanzati]

In that case internalSave wont emit any middleware event, as it is not an exported function :)

[hmedney]

Interested in this one also - in preserving async action names in uglified builds. Would it be an appropriate use of MST to implement logic that depends on whether certain actions have been invoked, just by capturing action names via addMiddleware()? Any risks/downsides to this approach?

For example, knowing whether a particular async action had ever been invoked successfully (or is currently running) can enable the presentation layer to either show a loading message or display any existing data already in the store.

I think my net question is whether preserving the names of async actions is a design goal for MST and whether building logic based on call objects received through addMiddleware() is sound practice.

Thanks, and thanks for creating this awesome library!

[fabienjuif]

@hmedney this is all about the lib I created here and this is why I dig this issue :)

https://github.com/alakarteio/k-mst-onaction

[hmedney]

Hi @fabienjuif ah, nice - like the global function approach. I was going down a different path of capturing all the async actions in a MST tree and then observing changes in the tree.

@mweststrate @mattiamanzati It's somewhat implicit in this issue, but I was wondering if you could confirm before I go too much further down this road - is basing logic on async action events via addMiddleware() a sound practice in MST? Whether certain async actions have run, have never run, are running, or have error'd can be useful for the view to show a busy indicator, existing state, or an error message in a component rendering state from an async source.

[mweststrate]

Closing this issue as it is inactive for quite a while. Best open new issues for new questions.