todo-mvc: Use declarative DSL #56
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Sep 30, 2016
Merged
Store actions are never retrieved from the app factory so they don't have to be registered. Do configure them though.
Unlike other UI actions, the filter action is only referenced from the route. It does not need to be registered, but it must still be configured.
The router dispatches for the initial URL, but dispatches won't work until the app has started.
Declare the routes in a separate module to reduce clutter in the main module. It also helps make it clearer that the router is only started once the app is ready. Pass the app instance to routes via the dispatch context. This allows the routes to resolve the filter action before creating the filter route. Create a single route for all three filters, using parameter validation. The filter action must now be registered with the app and no longer has to be configured separately.
Ensure the TodoFooter widget resolves the clearCompleted action via the registry. Ensure the TodoItem widget resolves the actions via the registry.
This utilizes the new createWidget support in app and avoids having to register the item with the app.
Register the store declaratively, then bind actions once the app is ready. Widget actions can now be resolved via the app, so they need to be registered rather than configured. Quick refactor so the widget actions can resolve their store. Todo actions must now be configured after the app is started so they can resolve the todo store.
Refactor UI actions to resolve store actions upon configuration. Rename widget actions to avoid name clashes.
Better use of destructuring, ensure clear promise chains.
Use the types in store and widget actions.
Only register actions that are used declaratively. Since actions are code it's easier to refer to them directly from other actions / widgets. Now that stores live in their own module they're also easy to refer to, without going via the app.
Better typing of do() arguments. Return promises. Whitespace.
novemberborn
force-pushed
the
todo-declaratively
branch
from
ca3222a
to
1d447e1
Compare
|
The corresponding |
novemberborn
commented
Oct 5, 2016
| export const toggleAll = createAction({ | ||
| do({ checked: completed }: { checked: boolean }) { | ||
| return todoStore.get() | ||
| // <any> hammer since the store isn't typed to return an iterator (which it does). |
There was a problem hiding this comment.
Can remove this hammer after updating to the latest dependencies.
There was a problem hiding this comment.
Actually it seems to return an IteratorShim and I still need the hammer. @kitsonk ?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I may have gotten a bit carried away with this… apologies :)
todo-itemcustom element is registered declaratively, though maybe it should be done inmain.ts