Skip to content

Modular api to satisfy #68#71

Closed
randallsquared wants to merge 1 commit intoBinaryMuse:masterfrom
randallsquared:modularApi
Closed

Modular api to satisfy #68#71
randallsquared wants to merge 1 commit intoBinaryMuse:masterfrom
randallsquared:modularApi

Conversation

@randallsquared
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

Fixed up some strings and comments which were misleading.

@randallsquared
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

Oh, okay. :)

@BinaryMuse BinaryMuse added this to the v1.5.0 milestone Oct 13, 2014
@BinaryMuse
Copy link
Copy Markdown
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
Copy Markdown
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.

2 participants