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

Modular api to satisfy #68 #71

Closed
wants to merge 1 commit into from

Conversation

randallsquared
Copy link
Contributor

This implements a superset of the API mentioned in #68 with addActions/addStores supporting the current object format (and used by the Flux constructor), and addAction/addStore supporting the one-at-a-time notion.

Flux#addStore takes a name and a store, and adds the store to Flux.

Flux#addAction takes either an array of namespaces and an action function, or any number of namespaces as strings, and then an action function.

bindActions was changed to throw when encountering a collision.

I didn't check in any documentation, but I can do so if you'd prefer that to writing it yourself.

@randallsquared
Copy link
Contributor Author

Hm. I didn't realize it was going to want to merge five pointless commits. I can resubmit with just the latest, if that's a problem.

@BinaryMuse
Copy link
Owner

@randallsquared Thanks so much, this is awesome! I'm out of town soon but will take a closer look ASAP. Feel free to squash and force push over the branch.

@randallsquared
Copy link
Contributor Author

Fixed up commits.

Add new method on Dispatcher: addStore to support Flux#addStore
Add tests for new api functionality
New: Remove misleading comments in favor of descriptive variable naming
New: Edit wording of error messages for clarity
@randallsquared
Copy link
Contributor Author

Fixed up some strings and comments which were misleading.

@randallsquared
Copy link
Contributor Author

I see that #51 and #39 are also involved, here. The code I submitted doesn't warn as #39 would suggest, but throws instead. I'm not sure whether the right way would be to change that to warning, or to add a method removeAction to allow code that received the error to remove the action if required.

@BinaryMuse
Copy link
Owner

This looks great, @randallsquared, thanks again. #51 and #39 deal with the store API, so I don't think there's an issue here.

@randallsquared
Copy link
Contributor Author

Oh, okay. :)

@BinaryMuse BinaryMuse added this to the v1.5.0 milestone Oct 13, 2014
@BinaryMuse
Copy link
Owner

Thanks again for your work on this; this landed as e02a443 and the commits from #77.

@BinaryMuse BinaryMuse closed this Oct 13, 2014
@randallsquared
Copy link
Contributor Author

No problem!

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.

None yet

2 participants