WICG / web-app-launch

Web App Launch Handler
Other
75 stars 28 forks source link

Queue-based API #20

Closed mgiuca closed 2 years ago

mgiuca commented 4 years ago

Based on a discussion today with @marcoscaceres @fallaciousreasoning @raymeskhoury @dominickn @alancutter (my first napkin spec!)

image

We're grappling with the following problems with launch event and file handling:

~ okay ~

~ deep breath ~

Here's the proposal (not battle-tested):

Therefore, the expected behaviour is that:

I will try to flesh this out more, consider more edge cases, and write actual code.

fallaciousreasoning commented 4 years ago

I'm broadly in favor of this. However I do have some concerns about tying this to a new data structure. I'm not super familiar with this, but don't datastructures generally belong in the realm of tc39. I don't know how much control we have to get something through, and it feels like it could take a while.

@raymeskhoury and I briefly discussed implementing LaunchEvents on top of observables, but were scared off because it hasn't progressed in a while (last commit was end of 2017).

If we do decide to take this kind of approach, I prefer the observable API to the LaunchEventQueue proposed above. It's more generic, it's more similar to existing JavaScript libraries (RxJS, zen-observable, fate-observable), and (I think), it has a much nicer API.

From the proposal

Lazy: Observables do not start emitting data until an observer has subscribed. So this is fine:

const observable = Observable.of("red", "green", "blue");
setTimeout(() => observable.subscribe({
    next(color) {
        console.log(color);
    }
}), 1000);

// Logs:
// red
// green
// blue

and specifically for launch events, we could do something like this:

// Note: This doesn't miss any launch events regardless of when we subscribe.
// This also forces apps to consider being launched multiple times, as opposed to
// the get/onpush model.
launchEvents.subscribe({
    next(launchEvent) {
        // Handle launch.
    }
});

so to sum up, I like the general idea, but I worry a bit about how hard speccing it will be (and I think we can do better than LaunchEventQueue).

(this is actually what we wanted to do, the current API was a compromise in order to get something semi-workable in).

mgiuca commented 4 years ago

Yeah if Observables was a thing, we would be able to use it, but I don't want us to block on it.

but don't datastructures generally belong in the realm of tc39

Generic ones do, but we can create a new class for our purposes inside our own API if we want to. Think of the web platform as a library that's being built on top of JS. If you're writing a library and you need a data structure that doesn't exist in the language, you could petition the upstream language developers to add the data structure you need, but it would also be OK to just write the data structure as part of your library.

If we're going to write our own, we wouldn't make it fully generalized (then we'll be the maintainers of a generic Observable class inside the File Handler spec). We would just make it do enough for file handlers.

I think we could do the subscribe thing (as opposed to event handlers) so you don't need to check the queue before you add your event listener to it.

fallaciousreasoning commented 4 years ago

Okay, I think I'm onboard. I might have a crack at updating the explainer with this.

What about making it look a bit like an observable from the outside (e.g. try and match roughly what is being proposed, but only with the bits we need)? It seems like a bit of an exercise in futility, as observables may never ship, or ship in a completely different form. Still, it'd be nice to not end up with two things that do more or less the same thing.

Also, do you think this belongs in the LaunchEvent explainer or the FileHandling explainer? I'm leaning towards FileHandling, just because it seems easier to explain with a concrete use case, and it means FileHandling won't depend on LaunchEvents. WDYT?

fallaciousreasoning commented 4 years ago

New crazy thought: Can we hide all of this observable black magic behind the event listener API? (i.e. we promise to fire the event once for each launch AFTER you attach a handler).

let launchNumber = 0;

// launch 1 happens now.
// launch 2 happens now.
setTimeout(() => {
  window.addEventListenerLaunch('launch', launchParams => {
    console.log(`Launched: ${++launchNumber}`);
  });

  // launch 3 happens now.
}, 5000);

// Output (after ~5 seconds):
// Launched: 1
// Launched: 2
// Launched: 3
marcoscaceres commented 4 years ago

That might unfortunately violate the principle that attaching event handlers and listeners shouldn’t have side effects. If you could attach the listeners and then call a function, then that would be ok tho.

fallaciousreasoning commented 4 years ago

Yeah, I thought something like that might be a problem :/

mkruisselbrink commented 3 years ago

I know this is an old issue, but I'd like to point out that some of the described problems shouldn't be a problem if browsers actually implement postMessage to clients as is spec'ed in the ServiceWorker spec.

I.e. specifically the race condition in

However, if the details are forwarded in a postMessage, there is a race condition in between the client appearing in the Clients list and the client adding a "message" event listener: the message is dropped on the floor as there is no listener.

does not exists. Per spec, messages send to a Client will be queued until either the client assigns an event handler to onmessage, or explicitly calls the startMessages() method (i.e. very similar to how MessagePort's start out paused). Now unfortunately I don't think that behavior is actually currently implemented anywhere, and some of the other reasons for a queue based API as described here might still be valid, but it might be good to explicitly mention why the existing queueing in Client.postMessage isn't good enough.

jeffposnick commented 3 years ago

CC: @philipwalton who looked into this in the context of https://github.com/GoogleChrome/workbox/blob/6d38919ebbc9664327e19ff00302d805c8166170/packages/workbox-window/src/Workbox.ts#L82-L85

I think he found that some browsers did the right thing?

philipwalton commented 3 years ago

My recollection (which seems to match my comment here as well), is that Chrome and Firefox both correctly implement postMessage buffering, at least well enough to reliably receive a message event sent from a fetch handler in the SW for a navigation request of a window client not yet created. (Also this test has been passing.)

alancutter commented 2 years ago

LaunchQueue is now in the spec draft.