This repository was archived by the owner on Oct 26, 2018. It is now read-only.
[WIP] Tweak the API to handle initial state correctly #255
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 know this is going to be controversial..
I think it’s wrong that the middleware dispatches during its creation. We probably won't allow it in next versions of Redux anyway.
The fact that this dispatch happens too early also caused #252. When the initial state is specified to the store and we “listen to replays”, we want the state to be the source of truth in the absence of user action. So if we load a location from localStorage and pass it into the initial state, whether with devtools or not, we want the URL to be set from it.
Since we need to extract a method to cause the initial dispatch anyway, I thought I might as well unite these two methods and turn them into options:
I know not everybody is a fan of named arguments so I’m very open to suggestions here. My reasoning for uniting these two methods was the fact that:
Therefore having to call one or two methods just to get the thing working seems like overkill. Additionally, named arguments explain what actually happens in very straightforward two-way binding terms, which is what this library actually is: two-way bindings from URL to a state field.
If you think this API sucks I’ll be happy to rewrite this to your API of choice. The primary purpose of this PR is still to fix #252, and I added a couple of new tests verifying that it is indeed now fixed.
This PR updates tests, example, and first part of the README. I am happy to update the API usage part when/if we agree on the API.