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

[wip] added ability to rewrite the route #12

Closed
wants to merge 4 commits into from
Closed

[wip] added ability to rewrite the route #12

wants to merge 4 commits into from

Conversation

jedireza
Copy link
Contributor

This allows us to essentially swap out the route handler without changing the URL.

The current use case is to support 404/500 pages. I have a PR for integrating fluxible-router into www.fluxible.io to help demonstrate. See: https://github.com/yahoo/fluxible.io/pull/120

  • test for REWRITE_ROUTE
  • better docs for RouteStore

@yahoocla
Copy link

CLA is valid!


debug('handleNavigateStart route not changed');
},
handleRewriteRoute: function (payload) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private

@redonkulus
Copy link
Contributor

Minor comments but 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 99.15% when pulling 165d98e on jedireza:rewrite-route into 53a97a5 on yahoo:master.

@mridgway
Copy link
Collaborator

mridgway commented May 1, 2015

I'm starting to wonder if this should be handled differently. It's basically a special case for something that isn't really route related. Others have asked for 404 handling within the router, although 500 handling doesn't really fit within the router at all. I wonder if this should be handled outside of the router entirely.

@jedireza
Copy link
Contributor Author

jedireza commented May 1, 2015

Maybe we can support a not found route configuration option and leave the 500s up to the app. This would be similar react-router's NotFoundRoute.

@jedireza
Copy link
Contributor Author

jedireza commented May 1, 2015

Closing for now. Moving these ideas to #14.

@jedireza jedireza closed this May 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants