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

Why is autorun doesn't run when observable props has changed? #546

Closed
leesiongchan opened this issue Sep 9, 2016 · 4 comments
Closed

Why is autorun doesn't run when observable props has changed? #546

leesiongchan opened this issue Sep 9, 2016 · 4 comments

Comments

@leesiongchan
Copy link

leesiongchan commented Sep 9, 2016

Provided I have a code like below, why is this.props.event.location wouldn't be triggered when changed?

@observer
class Location extends Component {
  componentDidMount() {
    autorun(() => {
      if (this.props.event.location) {
        this.fetch(this.props.event.location);
      }
    });
  }

  @action
  async fetch(locationIri) {
    const location = await new Promise(resolve => {
      setTimeout(() => {
        resolve({
          '@id': '/locations/1',
          formattedAddress: 'PRC de Schorre, Schommelei, 2850 Boom, Belgium',
        });
      }, 1000);
    });

    runInAction(() => {
      this.location = location;
    });
  }

  @observable location = {};

  render() {
    return (
      <div>
        Address: {this.location.formattedAddress}
      </div>
    );
  }
}

@observer
class Event extends Component {
  componentDidMount() {
    this.fetch();
  }

  @observable event = {};

  @action
  async fetch() {
    const event = await new Promise(resolve => {
      setTimeout(() => {
        resolve({
          name: 'Tomorrowland',
          location: '/locations/1', // IRI
        });
      }, 1000);
    });

    runInAction(() => {
      this.event = event;
    });
  }

  render() {
    return (
      <div>
        <Location event={this.event} />
      </div>
    );
  }
}
@jonhester
Copy link

From a cursory glance I'm not sure exactly what the issue is, but that seems a bit antipattern for react, which may be making things harder for you. I'd rather have a new component for each event and location rather than reusing the same one.

I'd set up an event store and a location store, separate from your react code. You can still fetch events and locations on demand, but you won't have to refetch if the same event or location comes up again. Personally, I'd keep the autorun in a store and not the react component. The docs actually have a really good example that includes a reaction, which is like an autorun.

https://mobxjs.github.io/mobx/best/store.html

@urugator
Copy link
Collaborator

urugator commented Sep 9, 2016

I don't think that event.location is observable, try to change this:

@observable event = {};

into this:

@observable event = {
  location: null
};

Also I don't think you ever change event.location anywhere. You're changing only this.event and this.location, which is not what you're observing for in autorun.

@andykog
Copy link
Member

andykog commented Sep 12, 2016

@leesiongchan, I see that your location object is initially empty. Mobx won't react on adding props. If you want that behaviour, use map instead. See Common pitfalls & best practices: object.someNewProp = value

@andykog
Copy link
Member

andykog commented Sep 12, 2016

@leesiongchan, feel free to reopen

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

4 participants