Closed aldendaniels closed 9 years ago
I need to be able to pause a transition while data loading is occurring
I don't think you'd actually want to do this. For the user, the page would be non-responsive while you're fetching data (i.e. they would click the back button and nothing would happen for a while).
@mjackson - this simulates the browser's native behavior - if I click a link on a page, then I'm left looking at the current page until the target page loads.
You would, of course, want to show a loading indicator during the transition phase, to keep the UI from feeling unresponsive.
@aldendaniels
We used this approach and it turned out to be a disaster. For example, while data is loading, what does Back button do? Does it cancel transition? If not, how can you cancel a transition that's taking too long (e.g. bad mobile connection). So does Back cancel pending transition? But that looks very weird on mobile, where "Back" is a slide gesture.
My advice: change URL right away. Changing it later = bad UX. I've learned it hard way.
@gaearon - Thanks for weighing in.
The browser natively does exactly what you're describing as being problematic on mobile - if you slide to go back during a pending transition then you slide "back" onto the same page you were on (cancelling the transition). Since this is native behavior, why not simulate it?
Are there other complications I'm missing for an SPA?
What don't you like about doing something like this, @aldendaniels?
history.listen(function (location) {
fetchTheData(location, function (data) {
renderThePage(location, data);
});
});
The current implementation of transition hooks is minimal on purpose. It only addresses the specific use case of confirming transitions away from a route. If you need to run some logic after a user hits a route, you can already easily do that.
There are a couple of issues:
renderThePage()
, I'd need some kind of proxy history object. Actually, the proxy history object is a decent option, but it would have to duplicate a lot of logic that's already built into history.The middleware layer provides an extensible hook for any kind of async transition operation, whether it's data loading, saving changes, or prompting a user before navigating away. And it's not coupled to React or React router, so it can be used (and tested) in any context.
@aldendaniels You make some great points :)
Changing the URL immediately is (IMO) an anti-pattern
There are actually two subtly different use cases here: programmatic and "manual" transitions. When you transition programmatically (i.e. using history.pushState
and/or history.replaceState
) then yes, we have the chance to defer updating the URL until data is loaded. However, when the user hits the back/forward buttons the URL always updates immediately (at least on desktop), regardless of whether or not the page is loaded.
Since there are two different behaviors here, I chose to go with the latter so we're always consistent. However, I'm hesitant to say that I know this is always the right decision because there is currently an unresolved issue in the code where we 1) update the URL, 2) cancel the transition and then 3) need to put the URL back (see #8 for more discussion). So you may have a point here and we may need to figure out how to support both behaviors, assuming we can't resolve that issue another way.
Hitting the back button will reload data for the previous view, instead of canceling the transition
Doesn't that completely depend on how fetchTheData
works? Many client-side apps these days employ a caching layer that lets them request the same data without incurring extra network requests. For example, your data fetching layer could be smart enough to know "I've already seen this location
object. Just return whatever we fetched the last time".
I'd need some kind of proxy history object
That's exactly the approach I'm advocating here. To be more clear, you don't have to give the <Router>
a history
object. Instead, you can give it a location
:
history.listen(function (location) {
fetchTheData(location, function (data) {
React.render(<Router location={location}>...</Router>, node);
});
});
You could pretty easily wrap this up in your own history object.
function createDataFetchingHistory(options) {
var history = createHistory(options);
function listen(listener) {
return history.listen(function (location) {
fetchTheData(location, function (data) {
listener(location, data);
});
});
}
return {
...history,
listen
};
}
var history = createDataFetchingHistory(...);
history.listen(function (location, data) {
// wahoo!
});
Thanks for the detailed response! There's some good stuff here.
when the user hits the back/forward buttons the URL always updates immediately (at least on desktop),
When I manually click a link (testing Chrome desktop on mac) then the URL does NOT update immediately - it waits for an initial response from the target page. With back/forward buttons the URL does appear to update instantly, but the the browser serves a cached version of the page so the entire page load is instant.
Many client-side apps these days employ a caching layer that lets them request the same data without incurring extra network requests.
Yes, but a custom caching layer shouldn't be a requirement for this. If an in-progress transition is cancelled via the back-button, then no transition occurred so no data loading should occur. Hitting the back button during an in-progress transition should render the transition a no-op.
To be more clear, you don't have to give the
a history object. Instead, you can give it a location:
Ooh, nice, I did not know this. This makes implementing my own middleware layer outside of history
much easier. Nonetheless, I can't replicate the desired behavior fully without access to history internals via something like a middleware layer.
Imagine this scenario:
/route0/
/route1/
/route1/
/route2/
What should happen is that the user is taken back to /route0
, because the transition to route1
never happened - it was interrupted. What's actually going to happen is that the back button take the user back to /route1/
because it was synchronously added to history.
I don't see a clean way around this without a middleware layer built into history.
UPDATE - Added missing step "5" to my scenario above:
User hits the back button
I think you can manage that scenario by redirecting push
calls to replace
if there is a pending transition.
I don't understand the need for a middleware API though. Wouldn't it be enough to include the functionality in the onChange listener? Something like
history.listen((location, performTransition) => {
renderSpinner();
loadData(location, (data) => {
performTransition();
// URL is changed now
render(location, data);
});
});
With the listen callback being guaranteed to be called immediately after a push/replace, one could manage pendingLocation
or renderedLocation
by himself. The middleware API could be built on top of it as far as I can tell.
@taurose - I think building the middleware layer on top of history would require rewriting most of createHistory.js
because you'd have an alternate view of reality with a different history and different back button behavior.
...so yes, I could intercept transitions, maintain my own separate source-of-truth history object, and map POP and PUSH events to REPLACE when they intercept pending transitions. But this is a lot of effort duplication.
HistoryJS already has the concept of a pending transition - used to support prompting users before navigation. Middleware simply extends this concept to support other use-cases by making transitions interruptible. The added complexity seems minimal to me and the gain seems significant.
This said, I don’t seem to be striking a chord here and you’re right, I can accomplish what I need in other ways.
@mjackson - Re-read your post in the light of day. Realized that I'd missed the import of what you were saying about manual transitions via back/forward buttons being immediate by necessity.
It’s true that the URL will update immediately when the user navigates via the forward/back button. This doesn’t mean (per-se), however, that the transition needs to be treated as synchronous. You can still treat the transition as “pending” after the URL has changed, thus getting the desired back-button behavior.
I think you misunderstood my proposal. You wouldn't have to keep your own history stack or redirect PUSH/REPLACE. Unless you call performTransition
, the URL wouldn't be changed, just like in your proposal. You'd only have to manage concurrency yourself to avoid rendering or fetching data for superceded locations.
var middlewares = [];
var addMiddleware = (middleware) => middlewares.push(middleware);
var removeMiddleware = ...;
var pendingLocation = null;
var curLocation = null;
history.listen((location, performTransition) => {
pendingLocation = location;
async.series(middlewares, (middleware, next) => {
if (pendingLocation !== location) ...
middleware(location, curLocation, next);
}), (err) => {
if (pendingLocation !== location) return;
pendingLocation = null;
performTransition();
curLocation = location;
render(location);
});
});
I think you can manage that scenario by redirecting push calls to replace if there is a pending transition.
There's probably still some work to be done in the transitionTo
function to support this, @taurose. Perhaps instead of throwing we should just change the action
of the nextLocation
to be a replace there...
Sorry @taurose, you're right, I had misunderstood your proposal. Thanks for elaborating.
And to @mjackson's point - this does add the requirement of interruptible transitions - but that's not a bad thing IMO.
I think that the design you're describing is (or at least could be) a middleware layer - it's just a different API. I'd envision this working just like Express' API, where listeners are evaluated serially:
// Verify auth.
history.listen((location, fnNext) => {
verifyAuth()
.then(fnNext)
.catch(() => fnNext({ // Redirect - Downstream listeners will not be called.
path: '/login',
params: {next: location.pathName}
});
});
// Load data.
history.listen((location, fnNext) => {
fetchData().finally(fnNext);
});
// Render.
React.render(document.body,
<Root> // Shows top-level loading indicator/overlay.
<Router history={history} />
</Root>
);
Is this what you were envisioning?
And of course you could still use this to replace the current registerTransitionHook()
stuff:
history.listen((location, fnNext) => {
showCustomComfirmPrompt('Are you sure?', fnNext); // false aborts
});
Almost :) . I actually had a single listener in mind, so you would write something like this
history.listen((location, performTransition) => {
confirm(location)
.then(checkAuth)
.then(loadData)
.then(performTransition)
.then(render);
});
Yeah, it would be pretty similar to the middleware API. One advantage I see is that you have the ability (or at least it's clearer how to) cancel ongoing async operations asap since the callback can be guaranteed to always be called immediately. Also, it's not opinionated about when to change the URL and how to run async operations (serial vs parallel).
Hmm. I agree that a single listener is all that's needed.
If you do this though, you'll want to lock it down so you can ONLY have one listener - which is different from today. Otherwise, what happens if you do register multiple async listeners? Does the first one win?... or the last one?
If you're going to support multiple listeners, then I think serial execution is a good approach - and one with strong precedence (Sinatra/Express).
Also, supporting multiple listeners has the advantage that its easy for multiple 3rd-party libraries to process requests without stomping on each other ...again, like the myriad of Express middleware available.
Checkout how visionmedia's Page.js library does this:
page(path, callback[, callback ...])
Defines a route mapping path to the given callback(s). Each callback is invoked with two arguments, context and next. Much like Express invoking next will call the next registered callback with the given path.
Context
Routes are passed Context objects, these may be used to share state, for example ctx.user =, as well as the history "state" ctx.state that the pushState API provides.
Personally, I like this approach.
Also, it's not opinionated about when to change the URL and how to run async operations (serial vs parallel).
I don't think supporting multiple listen()
calls makes this more opinionated - you can still register a single listener and parallelize internally.
So were are we with this? I'm happy to update my PR with whatever API is decided on... and I can work around the existing API if the decision is to keep the API unchanged. Whatever the outcome, I'd like to move forward (or not) as soon as there is consensus.
I can work around the existing API
Let's go with this for now, since I'm still not quite able to see where our API falls short.
Often when I'm working around someone else's API limitations, it helps me solidify my thinking about how that API needs to be changed so I can do what I need to do more easily. If that happens here, please do follow up and let us know.
Closing for now, but I'm happy to re-open later if you feel you'd like to continue this discussion, @aldendaniels.
@mjackson - fair enough. I'll be using your suggestion:
history.listen(function (location) {
fetchTheData(location, function (data) {
React.render(<Router location={location}>...</Router>, node);
});
});
The drawback is that the back button will misbehave, but I can live with that for now.
Hey @aldendaniels - just wanted to give you an update here. After thinking this through for a while and working to build the new react router API on top of this lib, I decided to make transition hooks async as you suggested here. The work was done in ae8dd6fc596cbe76bc8facb92bf141a87df4cfb1 and should be published in a minor version bump (since the API is backwards-compat, i.e. you can still return
if you want).
@mjackson - excellent, thanks for the heads up!
From a quick glance at the change, it looks like you're not running the transition hooks for the very 1st transition (e.g. the first time .listen()
is called). This is problematic for the data-loading/authentication use case.
@mjackson Just checking in again. See my previous comment. Would love to know your plans hear - I'm also happy to make a PR.
I'd like to hook into history to do data loading. To do this, I need to be able to pause a transition while data loading is occurring.
I think this can be simulated today using
registerTransitionHook()
and a customgetUserConfirmation()
handler that doesn't actually show a user confirmation dialog. This is of course a hack.A better solution would look something like this:
The transition would simply freeze until
fnNext
orfnAbort
are called.If the direction sounds good, then I could submit a PR for this.