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

fix: Added @action decorator to updateRoute and resetRoute methods of… #3

Closed
wants to merge 1 commit into from

Conversation

aap82
Copy link

@aap82 aap82 commented Dec 14, 2017

… RouterStore as they directly m

@coveralls
Copy link

Coverage Status

Coverage remained the same at 74.419% when pulling 895e52c on aap82:aap82 into e44133c on LeonardoGentile:master.

1 similar comment
@coveralls
Copy link

coveralls commented Dec 14, 2017

Coverage Status

Coverage remained the same at 74.419% when pulling 895e52c on aap82:aap82 into e44133c on LeonardoGentile:master.

@LeonardoGentile
Copy link
Owner

Thanks for this 👍
I will take a look and test it this weekend

@LeonardoGentile
Copy link
Owner

I would say this was not necessary because the two methods updateRoute and resetRoute are always called from other methods already decorated with @action, isn't it?

Do you have the need to call these two methods from outside the store instance?

@aap82
Copy link
Author

aap82 commented Dec 15, 2017

You're right that it's probably not necessary, but it just made sense to me to have it wrapped in the event that I or anyone else want to use it directly. But no, I do not have the need for it at the moment.

On a side note though, I think that these

  @observable route = null;
  @observable previousRoute = null;
  @observable transitionRoute = null;

should be actually be

  @observable.ref route = null;
  @observable.ref previousRoute = null;
  @observable.ref transitionRoute = null;

because the route properties themselves are never mutated, as they are replaced in whole. So, there isnt really a need to mark the properties observable, just the reference. Thoughts?

@LeonardoGentile
Copy link
Owner

I'm closing this as updateRoute and resetRoute are intended as helpers and always called from other @action decorated methods.

I will implement the @observable.ref soon

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

Successfully merging this pull request may close these issues.

3 participants