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

Better documentation on action/view definitions and their impact on IDE introspection, navigation and refactoring #2155

Open
beepsoft opened this issue Feb 26, 2024 · 2 comments
Labels
docs or examples Documentation or examples related

Comments

@beepsoft
Copy link

(Creating this issue as a follow-up of this conversion in the mst-gql repo: mobxjs/mst-gql#445 (comment))

Another issue that may worth mentioning or explaining more in the docs.

If you declare the actions like this (version 1):

const M = types
  .model('MyModel', {})
  .actions(self => ({
      action1(){},
      action2(){}
  }))

... you won't get proper IDE introspection, navigation and refactoring support. For example, if you Cmd+Click on action1() to see where it is referenced, IntelliJ jumps here:

node_modules/mobx-state-tree/dist/types/complex-types/model.d.ts

export interface ModelActions {
    [key: string]: FunctionWithFlag;
}

You can also use this (version 2):

const M = types
  .model('MyModel', {})
  .actions(self => {
    function action1(){}
    function action2(){}
    
    return {
      action1,
      action2
    }
  })

This is the worst: Cmd+Click jumps in to the return{} and if you Cmd+Click inside return{} it jumps to the function definition no the usages.

And finally, you can assign the actions to a named variable, which you return (version 3):

const M = types
  .model('MyModel', {})
  .actions(self => {
      const actions = {
          action1(){},
          action2(){}
      }
      return actions
  })

In this case Cmd+Click on actions1() will correctly list or jump to its usages and can refactor.

This is a huge difference in DX!.

Version 1 and 2 are the default suggestions in the docs now: https://mobx-state-tree.js.org/concepts/actions

Version 3 is mentioned as a tip: https://mobx-state-tree.js.org/tips/typescript, however, it comes with a note in bold:

NOTE: the last approach will incur runtime performance penalty as accessing such computed values (e.g. inside render() method of an observed component) always leads to full recompute (see this issue for details). For a heavily used computed properties it's recommended to use one of above approaches.

I guess this only refers to views, but should be OK for actions and volatile.

@coolsoftwaretyler coolsoftwaretyler added the docs or examples Documentation or examples related label Feb 26, 2024
@coolsoftwaretyler
Copy link
Collaborator

Thanks @beepsoft! I know we have a call scheduled, so we can talk in-depth then. But just want to acknowledge we'll work on this.

I think I'd like to add a "Recipes" section to the website in the sidebar for things like this.

Would love to see a PR with a markdown file presenting these options and code examples, along with tradeoffs for each, and a recommendation.

@constgen
Copy link

constgen commented Apr 7, 2024

Just one more workaround

const M = types
  .model('MyModel', {})
  .actions(self => ({
      action1(){}
  }))
  .actions(self => ({
      action2(){}
  }))

Every action in a separate actions declaration also fixes Go To Definition

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

No branches or pull requests

3 participants