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

[mobx6] @action.bound decorator not binding for function reference #3215

Closed
ssstelsss opened this issue Dec 6, 2021 · 2 comments
Closed

Comments

@ssstelsss
Copy link

ssstelsss commented Dec 6, 2021

expected behaviour is this.count in this.renderCell will be 123, because this.renderCell was decorated with action.bound. But when I use reference on decorated function context of class is loose for this function.

For "mobx": "4.4.2" it worked correctly. But when we migrate our project on 6 version the bug appeared.

May be you can explain this behaviour.

export class App extends React.Component {
  @observable
  private items = [
    {
      title: "Test1",
      render: this.renderCell
    },
    {
      title: "Test2",
      render: this.renderCell
    }
  ];

  @observable
  count = 123;

  constructor(props) {
    super(props);
    makeObservable(this);
  }

  @action.bound
  renderCell(item) {
    console.log(this);
    // this.count is undefined in this scope
    return (
      <div key={item.content}>
        <div>{item.content}</div>
        <div>{+this.count}</div>
        <br/>
      </div>
    );
  }

  render() {
    return <ComponentWithRender items={this.items} />;
  }
}

https://codesandbox.io/s/mobx-action-bound-bug-ulred?file=/src/App.tsx

@urugator
Copy link
Collaborator

urugator commented Dec 6, 2021

This is because the property items is initialized before makeObservable(this), which applies the decorator and binds the method. So when items is initialized, this.renderCell is not yet bound and what's more it's not even action yet.
Either move the initialization after makeObservable:

constructor(props) {
  super(props);
  makeObservable(this);
  this.items = [
    {
      title: "Test1",
      render: this.renderCell
    },
    {
      title: "Test2",
      render: this.renderCell
    }
  ];
}

or bind them by yourself render: this.renderCell.bind(this)
or user arrow fn render: (...args) => this.renderCell(...args)

More importantly you shouldn't apply action to render functions - action applies untracked, so the observables accessed in renderCell won't be tracked. actions are intended for state mutations and the main goal is to batch these mutations.

@ssstelsss
Copy link
Author

This is because the property items is initialized before makeObservable(this), which applies the decorator and binds the method. So when items is initialized, this.renderCell is not yet bound and what's more it's not even action yet. Either move the initialization after makeObservable:

constructor(props) {
  super(props);
  makeObservable(this);
  this.items = [
    {
      title: "Test1",
      render: this.renderCell
    },
    {
      title: "Test2",
      render: this.renderCell
    }
  ];
}

or bind them by yourself render: this.renderCell.bind(this) or user arrow fn render: (...args) => this.renderCell(...args)

More importantly you shouldn't apply action to render functions - action applies untracked, so the observables accessed in renderCell won't be tracked. actions are intended for state mutations and the main goal is to batch these mutations.

Thank you for your answer. It's really helpful!

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

No branches or pull requests

3 participants