Coder Social home page Coder Social logo

Transition Middleware about history HOT 24 CLOSED

remix-run avatar remix-run commented on April 25, 2024
Transition Middleware

from history.

Comments (24)

mjackson avatar mjackson commented on April 25, 2024

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).

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

@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.

from history.

gaearon avatar gaearon commented on April 25, 2024

@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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

@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?

from history.

mjackson avatar mjackson commented on April 25, 2024

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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

There are a couple of issues:

  1. Changing the URL immediately is (IMO) an anti-pattern. Native browser behavior, Ember (I think) and popular SPAs like Gmail and GitHub do it the other way.
  2. Hitting the back button will reload data for the previous view, instead of canceling the transition. Again, this feels like an anti-pattern.
  3. I'm using this with ReactRouter, which expects a history object, so I can't just call 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.

from history.

mjackson avatar mjackson commented on April 25, 2024

@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!
});

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

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:

  1. User starts at url /route0/
  2. User navigates to /route1/
  3. Data loader fetches data for /route1/
  4. Before data loading is complete, user navigates to /route2/
  5. User hits the back button

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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

UPDATE - Added missing step "5" to my scenario above:

User hits the back button

from history.

agundermann avatar agundermann commented on April 25, 2024

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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

@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.

from history.

agundermann avatar agundermann commented on April 25, 2024

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);
  });
});

from history.

mjackson avatar mjackson commented on April 25, 2024

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...

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

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?

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

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
});

from history.

agundermann avatar agundermann commented on April 25, 2024

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).

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

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.

from history.

mjackson avatar mjackson commented on April 25, 2024

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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

@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.

from history.

mjackson avatar mjackson commented on April 25, 2024

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 ae8dd6f and should be published in a minor version bump (since the API is backwards-compat, i.e. you can still return if you want).

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

@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.

from history.

aldendaniels avatar aldendaniels commented on April 25, 2024

@mjackson Just checking in again. See my previous comment. Would love to know your plans hear - I'm also happy to make a PR.

from history.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.