Skip to content

Make Route more extensible #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ryanflorence opened this issue Jul 2, 2014 · 8 comments
Closed

Make Route more extensible #65

ryanflorence opened this issue Jul 2, 2014 · 8 comments

Comments

@ryanflorence
Copy link
Member

For cases like #52 it would be lovely to provide interesting extension points.

We could start by first making everything in Route a RouteMixin, and then export Route as nothing more then a component that uses it. This would allow people to start adding their own custom behavior to their Route config.

@mjackson
Copy link
Member

mjackson commented Jul 3, 2014

I'm not opposed to this, though it seems like #52 should be solved by using a transition.redirect( ... ) in the willTransitionTo hook of the route handler. I'm not sure it represents a good use case.

@mjackson
Copy link
Member

mjackson commented Jul 3, 2014

To clarify, the PostPage handler from #52 could have looked like this:

var PostPage = React.createClass({

  statics: {

    paramsAreValid: function (params) {
      // ...
    },

    willTransitionTo: function (transition, params) {
      if (!PostPage.paramsAreValid(params))
        transition.redirect('404');
    }

  }

});

Of course, this requires you to create your own paramsAreValid method for each route handler.

But also, consider the case where paramsAreValid succeeds but you still have a 404. Still going on the example from #52, assume the id param is indeed numeric but the id isn't found in the database. You still need to redirect to the 404 in that case. So it weakens the argument for param validation based on regex IMO.

@ryanflorence
Copy link
Member Author

Ignoring #52 (since it has been solved quite nicely with https://github.com/bjyoungblood/rnr-constrained-route) I still wonder if there's some interesting stuff we just aren't thinking about yet that could happen if people could make their own <Route>.

But until we have a strong use-case that can't be solved elegantly with the existing api, happy to leave this sitting here and pick it up then.

@madebyherzblut
Copy link

@rpflorence Can you post an example how this could solve the context problem discussed in #57?

@ryanflorence
Copy link
Member Author

var ContextualRoute = React.createClass({
  mixins: [ Router.RouteMixin ],

  getDefaultProps: function() {
    return { context: { foo: 'bar' } }
  }
});
<ContextualRoute handler={App}/>
<!-- instead of -->
<Route context={context} handler={App}/>

@ryanflorence
Copy link
Member Author

I don't think we need to do anything fancy here anymore, the way we've implemented <Redirect/> shows that making custom route config components is trivial. https://github.com/rackt/react-router/blob/master/modules/components/Redirect.js

Unless @spicyj sees something terrible about treating a plain old function like a component ... I felt a little sneaky when I wrote that code.

@sophiebits
Copy link
Contributor

Actually, yes. Plain functions will soon cease to work in JSX because of a handful of changes we're planning to make soon – you'll hear more details from @sebmarkbage when they're ready.

@ryanflorence
Copy link
Member Author

Would like to know how to accomplish what we have with Redirect so we can change the implementation before those changes break it.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants