Skip to content
This repository has been archived by the owner on Dec 15, 2018. It is now read-only.

Bring back middleware :( #96

Merged
merged 3 commits into from
Oct 7, 2016
Merged

Bring back middleware :( #96

merged 3 commits into from
Oct 7, 2016

Conversation

tptee
Copy link
Contributor

@tptee tptee commented Sep 30, 2016

This is the only way to allow consumer middleware to dispatch router actions.

/cc @divmain @baer

@dpwrussell
Copy link

Looks good.

Not sure if you want to think about a related problem at the same time though.

I implemented some middleware to do split routes where a request which hits a route with a certain property would be intercepted, the bundle loaded and then on successful load, would replay the action. The problem with this is that it is difficult to access routes or matchRoute from the middleware as the store accessible in the middleware does not have access to these properties.

The only way I've found to do this is to build the middleware with a closure, like this.

const middleware = routes => store => next => action => {
...
}

but then updating the route (Like #88 suggests) is tricky as you'd really need to replace store.routes, but there is no access to store to do that replace and as the routes are a closure and not accessed from the store, even if it was updated, the closure would not change unless that routes was a container object for the actual routes.

Ideas very welcome!

@tptee
Copy link
Contributor Author

tptee commented Oct 5, 2016

@dpwrussell can you add your comments to #88?

@tptee tptee force-pushed the feature/middleware-sadness branch from 11e676f to 440355e Compare October 6, 2016 04:56
@divmain
Copy link
Contributor

divmain commented Oct 7, 2016

This PR looks good! A pity it is necessary, but ¯\_(ツ)_/¯.

@tptee tptee merged commit 871e121 into master Oct 7, 2016
@tptee tptee deleted the feature/middleware-sadness branch October 7, 2016 17:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants