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

Enable marking store as global, a.k.a - fallback for unknown changes. #6

Merged
merged 2 commits into from
Aug 24, 2016

Conversation

antitoxic
Copy link
Collaborator

That's an alternative to #3.

Very simple: use config to set a global: true which marks the store as default store for any uknown changes that spy() detects.

Use that store and it's associated remotedev to show those changes.

@antitoxic antitoxic mentioned this pull request Aug 24, 2016
@zalmoxisus
Copy link
Owner

zalmoxisus commented Aug 24, 2016

Looks great! Will it replace #5 or will complement that? As far as I understand, there're no problems to get the target there? For async actions we just pass the store in runInAction (which is the user's responsibility). Or there are some use case we aren't able to handle there?

@antitoxic
Copy link
Collaborator Author

I think #5 is different. There are a lot more use-cases to be done and I don't think most of them are possible. "Why not possible?" could be due to inability to get "path" or inability to find target if the observable is neither a store method nor an argument (the action could be bound to the observabe via bind() or just have access to it via external scope).

There are a lot of factors for #5. This pull request is way simpler and does the trick for most cases. I think it's good to merge because #5 has a lot more work to do.

@zalmoxisus zalmoxisus merged commit e5e4a5d into zalmoxisus:master Aug 24, 2016
@zalmoxisus
Copy link
Owner

Merged, will cut a release today. Thanks a lot for the contribution!

Was just wondering, whether we should close #5 or continue to work on it.

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.

2 participants