Deferred routes support #182
Conversation
… of deferred routes
|
This PR resolves issue #163 |
|
@taion Even though the function is quite small, I think it'd be a good idea to cover it with tests, because there are many possible combinations of normal and deferred routes being nested in each other and it's not very straightforward as to in what order their callbacks will be executed |
|
I've been slammed all week, sorry... I'm going to try to find some time to look this weekend. |
|
@taion Any updates on this? |
|
soon, sorry – still trying to catch up after the long weekend |
|
Sure, no problem. I'll try to reply as soon as I get a response from you |
|
I'm really sorry for the delay here. I'm blocking off time specifically to merge this for this week. |
|
That'd be great! |
|
sorry, i need a few more days... >.< |
src/ResolverUtils.js
Outdated
| return { | ||
| routeData, | ||
| prevPromise: isPromise(routeData) ? routeData : prevPromise, | ||
| prevParentPromise: parentPromise, |
There was a problem hiding this comment.
@NSLS This is a small behavior change. It seemed odd for defer to change the parent promise route based on whether there was a previous promise. This makes it so that defer without any previous promises is a no-op, rather than making the children of that route with defer behave as if they all had defer.
| * Generate route data according to their getters, respecting the order of | ||
| * promises per the `defer` flag on routes. | ||
| */ | ||
| getData(match, routeMatches) { |
There was a problem hiding this comment.
Moved this here because it's a property of the default resolver, and it's not relevant to other resolvers
There was a problem hiding this comment.
Yep, makes sense
| // For a deferred route, the parent promise is the previous promise. | ||
| // Otherwise, it's the previous parent promise. | ||
| const parentPromise = routeMatch.route.defer | ||
| ? Promise.all(ancestorRouteData) |
There was a problem hiding this comment.
This is a little different... I think it makes more sense to wait on all parent promises, instead of just the most immediate one.
There was a problem hiding this comment.
I think my intention there was that some child routes would not have to wait for the promises they don't care about.
For example, one could have a root route, which fetches some general data, such as recent news maybe, which are required on each page. This promise does not hold any other route from fetching its' own data.
Then, you could have another route, let's say /admin, which requires the user to be authenticated, and hence it fetches the auth state in its own getData. And finally, the route which is included into /admin, e.g. /admin/news, needs to wait till authentication state will be loaded, but does not need to wait till recent news are fetched.
But if I understood correctly, in your example the chain would be
Fetch news => Fetch auth state => Fetch admin news
And in my desired behaviour it would be something like
Fetch news && (Fetch auth state => Fetch admin news)
Hope I could express this clearly :))
There was a problem hiding this comment.
I know what you mean, but I think that makes things a little unclear. What I mean is, suppose the route config looks like:
<Route {...} getData={getData1}>
<Route {...} getData={getData2}>
<Route {...} getData={getData3} defer />
</Route>
</Route>Suppose getData1 and getData2 both return promises.
Now suppose we change getData2 to no longer return a promise. With the earlier implementation, getData3 would switch to waiting on getData1 instead of getData2. This seems a bit more confusing then if it switched from waiting on both getData1 and getData2 to only waiting on getData1.
In the case above, I'd consider handling things like "recent news" with a named child route, perhaps, such that the hierarchy only includes immediate parents anyway... something like:
<Route {...} getData={getAuthData}>
{{
recentNews: <Route {...} getData={getRecentNews} />,
private: <Route {...} getData={getPrivateData} defer />,
}}
</Route>How does that sound?
There was a problem hiding this comment.
Ah, I see now. Then I haven't got any more objections. I think we can merge this
|
Thanks for your help here, and for your patience! |
Originally, found executes all
getDatacalls in parallel. Although it's good for the majority of use cases, sometimes it makes sense to delay several calls until it's parent request has been resolved. For example, one might want to load some data only after it's authentication has been determined, and throw a redirect otherwise.This PR adds a support for
deferprop on route configuration by replacing the oldgetRouteValuesfunction in resolver with a similargetRouteDatafunction.Without the
deferflag the behaviour of the resolvers should stay the same.When the
deferflag is specified, the resolver will look for latest parent route's promise and will delay the getter call on this route until the parent's route has been resolved.All the getter calls of the deferred route's children will be executed in parallel with the deferred route's callback, unless they are itself deferred.
Closes #163