Better code ergonomics for URL rewriting use cases

[domenic]

(Moved from slightlyoff/history_api#23 by @SetTrend, with me synthesizing and summarizing.)

This example, which was formerly in the readme, is an instance of a somewhat-common pattern, of wanting to do slight tweaks to the URL before navigating. I'll quote it here in full (translated to the window.navigation API):

navigation.addEventListener("navigate", e => {
  const url = new URL(e.destination.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");

    e.preventDefault();
    location.href = url;
  }
});

However, as noted there,

TODO: it feels like this should be less disruptive than a cancel-and-perform-new-navigation; it's just a tweak to the outgoing navigation.

To expand on this, code like the above will generate a second navigate event. Which, actually, this code doesn't guard against: so as written this will cause an infinite loop. Oh dear.

I also suspect that sometimes you want to do something like this, but with state, not URLs. For example, something like

navigation.addEventListener("navigate", e => {
  if (!e.destination.getState()) {
    // We probably got here via an <a> click or something.
    const state = figureOutStateForURL(e.destination.url);
    
    e.preventDefault();
    navigation.navigate(e.destinationEntry.url, { state });
  }
});

This example, like the previous one, is unfortunate; canceling the navigation (with e.preventDefault()) then starting a new one feels wrong for this sort of minor tweak.

The question is just what syntax would be better. Here are some ideas we've seen so far:

Expand transitionWhile()

Right now transitionWhile() takes a promise for nothing; the browser is only interested in how long the promise takes to settle, and whether it fulfills or rejects. We could expand it to take a promise for { url, state }, so that you'd write the above examples as

navigation.addEventListener("navigate", e => {
  const url = new URL(e.destinationEntry.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");
    
    e.transitionWhile({ url });
  }
});

navigation.addEventListener("navigate", e => {
  if (!e.destination.state) {
    // We probably got here via an <a> click or something.
    const state = figureOutStateForURL(e.destination.url);
    
    e.transitionWhile({ url: e.destination.url, state });
  }
});

This would also allow asynchronous calculation of the new URL and state, which I could imagine being useful.

Make destination mutable

This ties into larger questions about mutability (#7), so it could come in a few variants. But a simple version is to just have NavigationDestination's url and state properties be mutable. Then the above examples would become

navigation.addEventListener("navigate", e => {
  const url = new URL(e.destination.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");
    
    e.destination.url = url;
  }
});

navigation.addEventListener("navigate", e => {
  if (!e.destination.state) {
    // We probably got here via an <a> click or something.
    const state = figureOutStateForURL(e.destination.url);
    
    e.destination.setState(state);
  }
});

Introduce a new beforenavigate event where these things are mutable

Here the idea is to separate navigations into two phases: preparing for the navigation, where you might want to tweak the URL and state; and processing the navigation, where you don't want to change those but instead you simply want to act on it.

This could look a lot like "Make destination mutable", or maybe we'd put the url and state properties directly on the event object.

This has a lot of consequences though that'd need to be thought through. Like, does beforenavigate get the same information as navigate? And, we'd need to reevaluate each of our navigate examples to figure out whether they'd be better off using beforenavigate. I'm not sure there would always be a right answer, which would make this API hard to use.

---

A tricky thing to consider is what happens when such mutations change the "type" of the navigation. The affiliate links example takes a cross-document cross-origin navigation, and just produces a slightly-different cross-document cross-origin navigation. But, what about a case like this?

appHistory.addEventListener("navigate", e => {
  e.destination.url = "https://some-other-origin.example.com/";
});

history.pushState(null, "", "/same-origin-page");

Here we're starting with a same-origin same-document navigation. But the navigate handler wants to update it to a cross-origin navigation, and cross-origin navigations must be cross-document. (You can't have your Document object claiming to be someone else's origin!)

We could probably just throw in such cases, I suppose. You could always do a proper redirect in your navigate handler, i.e. canceling the current navigation and starting a new one via location.href, if you wanted this.

The reverse version:

navigate.addEventListener("navigate", e => {
  e.destination.url = "/same-origin-page";
});

location.href = "https://some-other-origin.example.com/";

does not have this problem; it transforms a cross-document cross-origin navigation into a cross-document same-origin navigation, which is fine. (It's just like link-rewriting.) But, we could also throw on that sort of thing for simplicity...

[tbondwilkinson]

I do think that the destinationEntry that's passed into the navigate event should NOT be the same Entry that you would read from currentEntry because in some ways it's prospective - it hasn't yet been serialized to history, and specifically if you do:

appHistory.addEventListener("navigate", e => {
  console.log(e.destinationEntry === appHistory.currentEntry);
});

this should be false - and I would expect it to be false.

So I like the approach of passing a separate instanceof Object in destinationEntry, and I like the idea of making it mutable. But I think that the promise you pass to respondWith should also be able to resolve to that entry, or should be the entry itself.

So in total:

appHistory.addEventListener("navigate", e => {
  const modifiedEntry = e.destinationEntry;
  const url = new URL(modifiedEntry.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");
    
    modifiedEntry.url = url.toString();
    e.respondWith(Promise.resolve(modifiedEntry));
  }
});

[domenic]

(Note: I recently renamed e.destinationEntry to e.destination to cut down on verbosity.)

this should be false - and I would expect it to be false.

Yep, that's already the plan. It's definitely a different object. The question is, whether it should be a different class. Right now AppHistoryEntry's url and state properties are semi-immutable; they can only be changed via appHistory.update(), for the current entry only. If we want to make them mutable (so that e.destination.url = ... works), then it'd be a new class.

So I like the approach of passing a separate instanceof Object in destinationEntry, and I like the idea of making it mutable. But I think that the promise you pass to respondWith should also be able to resolve to that entry, or should be the entry itself.

Hmm, this seems like it's mixing two paradigms to me. If you're already doing e.destination.url = ..., then the browser knows you mutated the URL; there's no need to tell it via the additional e.respondWith(Promise.resolve(modifiedEntry)) (or, equivalently, e.respondWith(Promise.resolve(e.destination)).

Similarly, if you need to do some async work to figure out the new URL, the mutable e.destination paradigm for that would be:

e.respondWith((async () => {
  const stuff = await doStuff();
  e.destination.url = stuff;
})());

[tbondwilkinson]

It's just for me a question of the timing.

If we say, for instance:

appHistory.addEventListener("navigate", e => {
  e.destination.url = ...;
  e.respondWith((async () => {
    await new Promise(resolve => setTimeout(resolve, 10));
    e.destination.url = ...;
    await new Promise(resolve => setTimeout(resolve, 10));
    e.destination.url = ...;
    await new Promise(resolve => setTimeout(resolve, 10));
    e.destination.url = ...;
  })());
});

Would you expect the URL to update on each new assignment to e.destination.url? I think that's reasonable, actually, I just want to make sure we're intentional. Because otherwise, if we only want to let the URL change at the end when the Promise resolves, then I think having to explicitly resolve the intended entry makes it clearer that THAT'S the triggering action.

[domenic]

Great point. I don't like the look of that; causing many URL updates per navigation is tricky. So that inclines me more toward "expand respondWith". @annevk also was somewhat in favor of that alternative when we discussed offline.

I do want to figure out #19 first since that impacts exactly when the URL updates. I'll come back to this after we have a model there...

[domenic]

After #56 gave us more clarity on when respondWith() works, re-using that no longer makes sense. In particular we want to allow URL rewriting for cross-origin <a> clicks, but we don't allow respondWith() for that case. We would end up with two very different meanings for the same function.

Before we come up with a new solution, we need to clarify, in a similar style to #56, exactly when this kind of URL/state modification is allowed. Here's a first pass: it's allowed for

• Any non-browser-UI-initiated non-back/forward navigation, e.g. a link click or form submission or location.href set or history.pushState().
• Browser-UI non-back/forward fragment changes.

This is very similar to when navigate is cancelable. However, navigate cancelation is allowed for history.back() / appHistory.back() and friends, which I think we should not allow URL rewriting for, because it's too confusing what that means.

With this on hand, here's a new menu of solutions:

A new event with mutable e.destination

This event fires only when the above conditions hold.

Note that making state mutable requires adding setState() to e.destination. (We wouldn't add it to AppHistoryEntry generally.)

appHistory.addEventListener("beforenavigate", e => {
  const url = new URL(e.destination.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");
    e.destination.url = url;
  }
});

appHistory.addEventListener("beforenavigate", e => {
  if (!e.destination.getState()) {
    e.destination.setState(figureOutStateForURL(e.destination.url));
  }
});

A new event with an update() method

This event fires only when the above conditions hold. Better names welcome for update().

appHistory.addEventListener("beforenavigate", e => {
  const url = new URL(e.destination.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");
    e.update({ url });
  }
});

appHistory.addEventListener("beforenavigate", e => {
  if (!e.destination.getState()) {
    e.update({ state: figureOutStateForURL(e.destination.url) });
  }
});

A new method and boolean in navigate

We'd add update() and canUpdate (better names welcome). canUpdate is true in the above conditions, and update() throws when the above conditions don't hold. I think in most cases people wouldn't need to check canUpdate, but it seems important to have.

If you called update() after calling respondWith(), it would throw. If you called it asynchronously, it would throw. (Also, I guess canUpdate would flip to false in such cases.)

appHistory.addEventListener("navigate", e => {
  const url = new URL(e.destination.url);
  if (url.hostname === "store.example.com") {
    url.queryParams.set("affiliateId", "ead21623-781e-442f-a2c4-6cc1b2a9fda2");
    
    // No need to check canUpdate as cross-origin navs that reach `navigate` are always canUpdate.
    e.update({ url });
  }
  
  // Rest of your navigate handler goes here...
});

appHistory.addEventListener("navigate", e => {
  // Probably in a real app no need to check canUpdate since the app will put
  // state in all back/forward entries.
  if (!e.destination.getState() && e.canUpdate) {
    e.update({ state: figureOutStateForURL(e.destination.url) });
  }
  
  // Rest of your navigate handler goes here...
});

Mutable e.destination in navigate

You can pretty much figure out how this works by analogy. I think we'd still need canUpdate, as the setters would throw in some cases.

---

I like "A new event with an update() method" the best.

[tbondwilkinson]

A new event with an update() method

Also my favorite, except I think the name update is quite bad. modify? redirect?

[domenic]

Redirect would be so good but doesn't make sense for the state-change-only case IMO. Modify is not bad.

[annevk]

One thing that will likely help in clarifying this is to have a clear description of the primitives you end up exposing here. I guess the distinction you are drawing above is between "normal" navigation and "history" navigation. And then there's a difference between that being same origin and cross origin (though this only matters for "normal" as "history" is by definition same-origin as far as this API is concerned?).

It seems to me that respondWith() could still work (assuming that a delayed answer is acceptable) if you would also allow this kind of rewriting same-origin. And through getters (such as canRespond) you indicate what kind of object repondWith() can work with for a given event.

At least, from a high-level there's a set of primitive operations that you allow to be intercepted and responded to, but not all responses are acceptable everywhere. Perhaps these sets are completely non-overlapping and therefore two different APIs is better, but it's not clear that is the case?

[domenic]

Still haven't processed all the comments and thoughts here yet, but some discussion with @csreis pointed out that the explainer's current solution of e.preventDefault(); location.href = ... is extra-undesirable for other reasons. It loses important data like form submission data, same-siteness (for Sec-Same-Site), and various other things browsers bundle with navigations.

[annevk]

Right, I would expect a similar API to the fetch event in service workers. You get a NavigationRequest and you can create a new one to respond with. And then there's various policies about what is acceptable to read from these and what you get to set when creating a new one, to match security expectations.