Allow multiple event handlers to call respondWith()

[frehner]

Forked from the discussion here: #89

Some key comments from @domenic :
#89 (comment)
#89 (comment)

And highlights below as well

Currently only one handler can call respondWith():

To answer your specific questions:

But what happens if an earlier event listener calls respondWith?

As currently specced, calling respondWith() or preventDefault() means that any further calls to respondWith() will fail with an "InvalidStateError" DOMException.

Some additional thoughts from @tbondwilkinson

Of the smaller QOL improvements: I think allowing multiple calls to event.respondWith seems reasonable to me, but it begs the question whether you'd want to add the ability to get the current promise, so that you could .then() it, rather than just add to the Promise.all

Change the updating of location.href and appHistory.current to synchronously happen after all handlers have finished running (if respondWith() was called by any of them), instead of happening synchronously as part of respondWith(). This helps avoid the problem where future handlers see appHistory.current === appHistory.destination.

Oh, this seems like something we'd want to do, regardless of the outcome here.

[frehner]

If we did consider the asynchronous case to be highly important, then I'd like to reconsider the decision in #46 / #19 to make all same-document navigations synchronous. In other words, if delaying action until an async determination is made is a key goal of app history, then we should just delay the whole navigation to be asynchronous, instead of doing something kludgy like telling people to synchronously intercept or cancel the navigation but later reissue it (like discussed in single-spa/single-spa#762 (comment)).

Regarding this comment, one interesting piece of feedback from #66 is that both Vue and Angular navigations are async (all Vue navigations, and Angular navigations by default). Does that feedback affect this decision?

[domenic]

Not really, since they are frameworks that handle all navigations, and so they can synchronously decide to always call respondWith().

[domenic]

Quoting #89 (comment):

There are several smaller surface improvements we could make to help multiple navigate handlers work better together. The current design, of respondWith() preventing further respondWith() calls with no signal, is probably not optimal. Some ideas:

• Update event.canRespond in response to respondWith() calls. Right now it's static and reflects the interceptability of the event according to The Table; we could make it dynamic instead. Then, future handlers could get a more accurate sense of whether respondWith() works.

• Automatically perform event.stopImmediatePropagation() whenever respondWith() is called. Then, future handlers wouldn't see the navigate event at all.

• Allow multiple calls to event.respondWith(). Only the first has the interesting impact of converting the cross-document navigation into a same-document navigation. The others add their promises to a list, which gets Promise.all()ed to delay the navigatesuccess and navigateerror events.

• Change the updating of location.href and appHistory.current to synchronously happen after all handlers have finished running (if respondWith() was called by any of them), instead of happening synchronously as part of respondWith(). This helps avoid the problem where future handlers see appHistory.current === appHistory.destination. Edit: this was done in Move URL/history updating out of respondWith() #103.

The last of these is done. I'm interested in feedback on which, if any, of the first three of these we should do.

[domenic]

Discussing this a bit offline with @tbondwilkinson, we're thinking that allowing multiple calls to respondWith(), and Promise.all()ing them, is a good idea.

During that discussion, we thought there were two alternatives: Promise.all()ing them so they run in parallel, or doing a .then() chain so they run in series. But upon reflection I realized this was incorrect. You pass a promise to respondWith(), and that promise is for work that's already started. We'd have to change the semantics of respondWith() to take a promise-returning function if we wanted to get sequential behavior.

I'm going to draft a Promise.all() version now, since that's a relatively-straightforward change. However, I'd be interested to hear if the sequential version is worth investigating, and whether it'd unlock new use cases. Ultimately I think you could still do the sequential version with monkeypatching, or with coordination between the event handlers. But would making it the automatic by-default pattern be helpful? I don't think it directly accomplishes the use case outlined in #89, but maybe it'd help in some other way...

[domenic]

Hmm, if we do this, we may want to rename respondWith(), since its service worker counterpart has the second behavior ("Automatically perform event.stopImmediatePropagation() whenever respondWith() is called. Then, future handlers wouldn't see the navigate event at all.").

I guess waitUntil() is an OK name...

[domenic]

I guess waitUntil() is an OK name...

I'm no longer convinced. Although after #126 navigateEvent.pleaseNameMe(p) has similar semantics to fetchEvent.waitUntil(p) in terms of how they treat promises, they are different in many other regards.

In particular, fetchEvent.waitUntil() buffers other fetch events until the current one completes. And it generally lets the fetch go through unimpeded. Whereas, navigateEvent.pleaseNameMe() does not have any such buffering semantics, and it performs the important side effect of converting a cross-document navigation into a same-document one.

So, probably we should come up with a totally new name, not related to the events seen in service workers. Maybe... navigateEvent.convert(p), to put the emphasis on something rather different?

[frehner]

Naming things is hard!

evt.convert() seems like it may be confusing - are we converting the event itself? (No, it's the navigation that fired the event that we're converting, right?) So maybe something like...

evt.convertNavigation()

evt.spaNavigate()

evt.spaRoute() (route being a verb here... but if I have to explain it here, then it's probably not great either)

Yeah I don't know, I'm bad at this game. 🙂

[bathos]

Suggestion: event.transitionWhile().

• Places the relationship with a corresponding AppHistoryTransition, whose lifetime is determined by promises passed to this method, in the forefront. I think using the same vocabulary in both places would help to make the API surface more intelligible — “ah ... I can use appHistory.transition.rollBack() while a promise I pass to event.transitionWhile remains unsettled”).

• Avoids overspecifying a use case, compared to e.g. spaNavigate. An app doesn’t need to be “single page” to sometimes leverage this, and one of the nice things about AppHistory is its greater friendliness to things like third party widgets that want to use same doc nav without clobbering possible history usage of the embedder.

• It could be transitionUntil, but I think deliberate opting for verbWhile instead of verbUntil helps communicate that this isn’t following the same model as waitUntil. It also arguably makes it more clear that it can be called with multiple promises — transitionUntil kinda makes it sound like the transition will end when your promise settles, which might not be true if there were multiple promises.

---

Aside:

In particular, fetchEvent.waitUntil() buffers other fetch events until the current one completes.

This blew my mind a bit to learn just now. I never realized the event.waitUntil(open-cache-and-add-cloned-response-to-cache-here) pattern could potentially delay handling of the next fetch event even if you settle the respondWith promise before that waitUntil promise.

[domenic]

The only potential problem with transitionWhile() is that it doesn't make it clear that it performs a convert-cross-document-to-same-document action. In particular, I guess we would rename canRespond to canTransition, which feels a bit less clear.

So in my mind it's between transitionWhile() and convertNavigation(). Let's try a quick GitHub straw poll based on reactions to this post:

• 😄 for transitionWhile() / canTransition
• 🚀 for convertNavigation() / canConvert
• 👎 for neither of these being good and we should keep discussing.

[tbondwilkinson]

I'm not completely sold on these names.

My problem with "convert" is that it doesn't convert in all cases - sometimes the navigation is already a same-document navigation, like if it's a traversal or if it's history.pushState.

My proposal is transitionUntil() or like deferTransition() or extendTransition()

[domenic]

What did you think of @bathos's arguments for why verbWhile might be better than verbUntil?

[bathos]

I should qualify re: verbWhile / verbUntil that I realize it’s a pretty subtle distinction being made and neither word achieves instantaneous “I definitely know how THIS will behave!” nirvana. If folks think the existing usage of until is similar enough that the pattern-matching value of reuse (“I can guess how this works...”) is higher than the pattern-matching risk (“wait ... why doesn’t it work like...”) then I couldn’t disagree; I can only speculate about which is more important.

[domenic]

I found the multiple promises point pretty compelling.

[tbondwilkinson]

Fair enough, I'm fine with While

[jakearchibald]

@bathos

This blew my mind a bit to learn just now. I never realized the event.waitUntil(open-cache-and-add-cloned-response-to-cache-here) pattern could potentially delay handling of the next fetch event even if you settle the respondWith promise before that waitUntil promise.

It doesn't. waitUntil signifies that work relating to this fetch is ongoing and keeps the service worker alive. The service worker never forces fetches to be serial.

[bathos]

@jakearchibald thanks for the info — that’s exactly what my understanding had been prior to reading that and I was very surprised to learn otherwise (or think I was learning otherwise, rather).

Do you know what the quoted statement (“fetchEvent.waitUntil() buffers other fetch events until the current one completes”) was referring to? (i.e. was that itself incorrect or did i just misunderstand what it was saying?)

[jakearchibald]

The only time waitUntil delays fetch events is when it's called on the activate event.

This is because activate is the time the old service worker is gone - it's when you can perform migrations etc that couldn't be run while the old service worker was controlling pages, but that are needed before the new service worker fully takes over.

[bathos]

Thanks, that makes perfect sense.

(Though I still never totally trust my mental model of the lifecycle without verifying stuff against the spec, it’s probably gotten pretty solid at this point. However I doubt I’d have learned any of it in the first place if not for what was and probably still is the greatest dev intro ever written for a new platform API & its capabilities :)

[jakearchibald]

Ohh wow thanks! There's also https://web.dev/service-worker-lifecycle/ which covers some of the lifecycle stuff.