-
-
Notifications
You must be signed in to change notification settings - Fork 10.6k
Rewrite, version 1.0 #1158
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
Rewrite, version 1.0 #1158
Conversation
In regards to the transition changes, will it be possible to define
|
Wow, lots of changes 😄 . My thoughts on this:
|
@rickharrison in onChange you can walk the |
@taurose Thank you for your thoughtful response :) You have some great questions. I'll try to answer them here.
This is the big question. Honestly, I was trying not to think of the answer to this question and get everything else right before even considering it. I find that if I can narrow down the requirements, a lot of other things become clearer. So ya, the short answer is I don't know yet. ;) The idea was that anything that needs to be loaded asynchronously could be done inside the
Indeed. Asynchronously loading route config is only something that you need on really large apps, and even then only client-side, so we are adding a lot of complexity for a very specific use case. However, this is a use case that almost all of our large scale users have (i.e. Facebook, Twitter) so it's worth solving. Edit: Just saw the 2nd part of this question. If you need to link to a route that you load asynchronously, you need to use an absolute URL in your
Discarding data for a sec, the only time we need to asynchronously load route config and components is on the client where we have state. In other words, you can't asynchronously load route config and components on the server (but you shouldn't need to). Data is another beast entirely. Ideally we would have the same API for loading data on both clients and servers. See above.
Currently, nothing. I think we probably need to keep the
AFAICT there are two things we need to know in order for
With HTML5's Which part of that isn't reliable? |
@taurose One thing I tried my best to do with this is get the push/replace/pop semantics right. You had commented that our use of "pop" wasn't quite right in the past, and I think I know now what you meant by that. |
Edit: I updated the original PR above with a better idea. // The "transition manager" would be a thing that you could easily plug
// into your router's onChange hook. It could be responsible for managing
// any # of things including fetching data (and cancelling outbound requests
// for data when the routes change mid-flight), updating scroll position, etc.
var TransitionManager = createTransitionManager();
var Router = createRouter({
routes: ...,
onChange: TransitionManager.handleRouterChange
});
... |
Thanks for the detailed response. I still don't quite understand the async routes use case. Why isn't it enough to have async handlers and a decentralized route config?
I think you shouldn't store Either way, there's some browser inconsistencies I encountered working on #843, especially related to hash navigation. Clicking a |
@ryanflorence As someone who has upgraded with every version, having some backwards compatibility functions would be super helpful and much appreciated! :) Also, I'd be glad to help out with creating those and testing them with my apps. Perhaps we can chat about the approach in IRC sometime. |
It looks like a great amount of work. I have couple of questions:
|
Aaaaaah, you're absolutely right. ;) We can fix that with a As for random browser breakages, maybe there's no way around those... |
@vladap we're gonna ship 1.0 within a week. there, i said it. now we gotta do it ;) |
The problem with That's why, for the PR I mentioned, I opted to use an ID to identify the current visit/session, whose length could then be stored in sessionStorage (or even localStorage). The Example browser history:
sessionHistory: [ { id: 1, length: 3}, { id: 2, length: 2} ] |
|
Re data fetching, Michael and I spent a couple hours this morning on it (and several hours in times past, at his house, at a burger joint, over screen shares), it will be similar to what we do in the callback of |
Smashing it out of the park again! |
Just to be clear: if we will call our own static methods in |
@mjackson: one week... haha, it is even faster then I can possibly adapt. Thanks for the great work. |
"The willTransitionTo and willTransitionFrom transition hooks have been removed." This is troubling. We use var Authenticated = React.createClass({
// Redirect unauthenticated users
statics: {
willTransitionTo: function(transition, params, query, next) {
var currentUser = SessionStore.currentUser();
if (!currentUser) {
transition.redirect('login', null, { query: transition.path });
}
next();
}
},
render: function() {
return <RouteHandler/>
}
} I may have missed it, but what's the intended replacement for interception in these type of scenarios? |
@uberllama everything you used to be able to do with transitions you will still be able to do, the upgrade guide will have detailed information on how to update your app, and maybe even a backwards compat module. |
@ryanflorence I'll stay tuned. Is there anything ready I can look at in the interim? |
Looks great, nice work all. Could you clarify what nested routes look like in the non-jsx form? Would it be something like? var Router = createRouter(
{
component: App,
childRoutes: [{
name: 'home',
component: Home,
childRoutes: [{
name: 'profile',
component: Profile
}]
}]
}
); |
@jeffbski ya, you've got it. :) |
Great, I like it. |
5067ccb
to
494268c
Compare
state = JSON.parse(value); | ||
} catch (e) { | ||
// Invalid state in AsyncStorage? | ||
state = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably at least add a log statement here. If it has an invalid AsyncStorage, the app is probably hosed anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@duncanfinney sure, a log statement is a fine idea. Although, I didn't see console.log
anywhere in the React Native docs, so I'm still a little unsure how to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.log and console.warn go to the console/Xcode logs; console.error should cause a redbox.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent. Thanks for the info @spicyj !
WHY YOU PEOPLE AWAKE? |
👍 |
👍 Awesome! Looking forward to see v1.0 |
Thanks :) If you're adventurous:
We know we are missing some stuff, a bit more work and we'll have a beta that we will believe to be "complete". You may find the Also we'll have an upgrade guide with the beta when it comes. |
+1 Installing… |
@ryanflorence There is this area called Europe :D |
@johanneslumpe I'd have to see it for myself... |
Because someone told us there'd be a react-router beta tonight... 😉 |
Awesome, congrats!!! 👍 |
This might just be an idiot filter, but [email protected] doesn't seem to contain the compiled sources in /lib |
So far I've added the |
alright, I've fixed it, you can
and then require it as usual, npm was using |
👍 👍 👍 👍 |
What's the intended way to handle data fetching for index components? My routes look like: React.render((
<Router history={BrowserHistory} createElement={createElement}>
<Route path="/" component={App} indexComponent={Dashboard}>
<Route name="welcome" path="welcome" component={Welcome} />
</Route>
</Router>
), document.body); My setup is very similar to Relay. My It seems like the docs use |
I'm working around this for now by using |
I have a question regarding server-side rendering: how do we pass data to the App? I've read through the v1.0.0-beta3 doc and this thread, but could not get it to work. The 'Top-level API' section above mentions the following:
But it's not clear that how is this function supposed to be implemented? Later in this thread mentions
|
Hey, sorry if I missed this (I did read the thread, I really did), but is there an approximate timeline for react-router 1.0.0? Thanks! |
I'm also interested in the timeline. There are some great changes in this rewrite and I want to use them :) |
Same. I've been putting off implementing my auth re-direction forever because 1.0 makes it infinitely easier to do. Just want an idea whether or not I actually am going to have to do it the hard way or I can just wait until 1.0 ships |
I to am interested in the timeline, really itching for that 1.0.0 stable release ;) |
@ryanflorence @mjackson any rough approximation? +/- a few weeks or a month even? |
+1 |
Will this router be able to render a server side partial html view (that is not a react component) tostring? |
NOTE: DO NOT MERGE YET
This PR is a complete rewrite of the router with the following goals in mind:
<RouteHandler>
API... and a bunch of other stuff that we've learned from various issues over the past year.
Here's a summary of the various things you can do:
Top-level API
The
props
arg here contains 4 properties:location
: the currentLocation
(see below)branch
: an array of the routes that are currently activeparams
: the URL paramscomponents
: an array of components (classes) that are going to be rendered to the page (see below for component loading)The
branch
andcomponents
are particularly useful for fetching data, so you could do something like this:Inside your
App
component (or any component in the hierarchy) you usethis.props.children
instead of<RouteHandler>
to render your child route handler. This eliminates the need for<RouteHandler>
, context hacks, and<DefaultRoute>
since you can now choose to render something else by just checkingthis.props.children
for truthiness.<NotFoundRoute>
has also been removed in favor of just using<Route path="*">
, which does the exact same thing.Non-JSX Config
You can provide your route configuration using plain JavaScript objects; no JSX required. In fact, all we do is just strip the props from your JSX elements under the hood to get plain objects from them. So JSX is purely sugar API.
Note the use of
childRoutes
instead ofchildren
above. If you need to load more route config asynchronously, you can provide agetChildRoutes(callback)
method. For example, if you were using webpack's code splitting feature you could do:Which brings me to my next point ...
Gradual Path Matching
Since we want to be able to load route config on-demand, we can no longer match the deepest route first. Instead, we start at the top of the route hierarchy and traverse downwards until the entire path is consumed by a branch of routes. This works fine in most cases, but it makes it difficult for us to nest absolute paths, obviously.
One solution that @ryanflorence proposed was to let parent routes essentially specify a function that would return
true
orfalse
depending on whether or not that route thought it could match the path in some grandchild. So, e.g. you would have something like:Now, if the path were something like
/users/5
the router would know that it should load the child routes ofHome
because one of them probably matches the path. This hasn't been implemented yet, but I thought I'd mention it here for discussion's sake.Async Component Loading
Along with on-demand loading of route config, you can also easily load components when they are needed.
Rendering Multiple Components
Routes may render a single component (most common) or multiple. Similar to
ReactChildren
, ifcomponents
is a single component,this.props.children
will be a single element. In order to render multiple components,components
should be an object that is keyed with the name of a prop to use for that element.Note: When rendering multiple child components,
this.props.children
isnull
. Also, arrays as children are not allowed.Props
Aside from
children
, route components also get a few other props:location
: the currentLocation
(see below)params
: the URL paramsroute
: the route object that is rendering that componentError/Update Handling
The router also accepts
onError
andonUpdate
callbacks that are called when there are errors or when the DOM is updated.History/Location API
Everything that used to be named
*Location
is now called*History
. A history object is a thing that emitsLocation
objects as the user navigates around the page. ALocation
object is just a container for thepath
and thenavigationType
(i.e. push, replace, or pop).History objects are also much more powerful now. All have a
go(n)
implementation, andHTML5History
andHistory
(used mainly for testing) have reliablecanGo(n)
implementations.There is also a
NativeHistory
implementation that should work on React Native, tho it's a little tricky to get it working ATM.Transition Hooks
The
willTransitionTo
andwillTransitionFrom
transition hooks have been removed in favor of more fine-grained hooks at both the route and component level. The transition hook signatures are now:route.onLeave(router, nextState)
route.onEnter(router, nextState)
Transition hooks still run from the leaf of the branch we're leaving, up to the common parent route, and back down to the leaf of the branch we're entering, in that order. Additionally, component instances may register hook functions that can be used to observe and/or prevent transitions when they need to using the new
Transition
mixin. Component instance-level hooks run before route hooks.Anyway, apologies for the long-winded PR, but there's a lot of stuff here! Please keep comments small and scoped to what we're doing here. I'd hate for this to turn into a huge thread :P
Edit: Added data-loading example.
Edit: Added transition hooks.
Edit: Added props for named children, disallow arrays.
Edit: Added
addTransitionHook
/removeTransitionHook
API.Edit: Renamed
Router.run
=>Router.match
Edit: Removed static transition hooks
Stuff we still need:
Ok. Stuff we still need here:
Move routerWillLeave hook to instance lifecycle method instead of staticAdd component-level API for observing/preventing transitionscanGo(n)
support (@taurose can you help determine if the current impl. is legit or not? if not, let's just remove it)COME ON! LET'S GET THIS MERGED AND SHIP 1.0!!!!