WICG / webpackage

Web packaging format
Other
1.23k stars 118 forks source link

<script type=webundle> should fire a load (or error) event? #697

Open hayatoito opened 3 years ago

hayatoito commented 3 years ago

The main issue is #670.

It's unclear whether <script type=webbundle> should fire a load (or error) event or not. And if they should be fired, when should <script type=webbundle> fire them?

For references, the other <script type=>-ish elements' behaviors are:

We might want to align the event semantics of <script type=webbundle> with Bundle Preloading so that we can converge them in the future.

hayatoito commented 3 years ago

cc: @felipeerias

hayatoito commented 3 years ago

As a workaround, until we can have a consensus, users can try to fetch a (small) subresource immediately after inserting the script element into a document.

For example:

const script = document.createElement("script");
script.type = "webbundle";
script.textContext = JSON.stringify({ .... });
document.body.append(script);
fetch(a_tiny_subresource_in_bundle).then((_) => {
  // Alternative of a load event listener of a script element
}).catch((error) => {
  // Alternative of an error event listener of a script element
})

I'm not 100% sure whether this workaround is always applicable, however, this is fine in most cases.

Any ideas or opinions are welcome!

cjtenny commented 3 years ago

One thing I'm a little confused about, what do you mean by 'it fetches a bundle many times in its lifetime'? Our intention was that each bundlepreload section would result in a single request with a Bundle-Preload header; there would be no additional Bundle-Preload requests from the script element. After the page has sent a Bundle-Preload request, regular, non-<script type=bundlepreload> requests for the specified resources will then be served by the browser from the received bundle - blocking until it is available.

To review:

In my mind, then, it might make sense to send:

The load event would be separate from the load events sent by later references to the bundled resources, which would not be sent from the script element.

What do you think? Does that match the behavior of other script type elements? I'm not familiar with speculation rules, but I'll take a look.

cjtenny commented 3 years ago

As a workaround, until we can have a consensus, users can try to fetch a (small) subresource immediately after inserting the script element into a document. ... I'm not 100% sure whether this workaround is always applicable, however, this is fine in most cases.

If resources can be served from the bundled response before the whole bundle has been received, then it would be possible for this to fire a load event but the bundle itself to fire an error event - I don't know if that's good or bad. Otherwise, I think it would be a perfect workaround. Perhaps another subresource with less overhead than a script could be a little faster and more accurate for timing, but it probably doesn't matter much.

hiroshige-g commented 3 years ago

Basically, <script> load and error events indicate successful/failing resource loading, while parse errors go to Window's error events (e.g. window.onerror). Inline <script type=webbundle> can trigger resource loading, so it might make sense to fire load/error events based on whether the loading of webbundle files was successful. Inline <script type=importmap> doesn't fire load/error events, because inline import maps itself doesn't trigger resource loading. Similar discussion for firing load events for inline <script type=module> is at https://github.com/whatwg/html/issues/6421.

cjtenny commented 3 years ago

Got it, thank you for clarifying! It seems then like the first event I proposed should go to window.onerror, and the other 3 make sense for the script events. Does that sound reasonable? Are there any implications to that which could cause problems?

hayatoito commented 3 years ago

Thanks for the comments! It seems what we want to do is:

This sounds reasonable to me at first glance. At least, this would be consistent with the direction at https://github.com/whatwg/html/issues/6421.

@hiroshige-g If there are any implications which could cause problems, please let us know that.

cjtenny commented 3 years ago

Yeah, and I suppose defining "bundle loading failed" sub-cases is still a bit up in the air / might depend on bundle-preloading semantics. We can explore further what that is separately from the question of whether to fire the event, what do you think?

hayatoito commented 3 years ago

Yeah, and I suppose defining "bundle loading failed" sub-cases is still a bit up in the air / might depend on bundle-preloading semantics. We can explore further what that is separately from the question of whether to fire the event, what do you think?

Agreed. I guess defining "bundle loading failed" might be tricky on the bundle-preloading case. So we can explore that further as another issue.

hiroshige-g commented 3 years ago

What should occur when:

  1. The first <script type=webbundle> was added with source = foo.wbn.
  2. The loading of foo.wbn succeeded or failed.
  3. Then the first element was replaced with the second <script type=webbundle> with source = foo.wbn.

IIUC Step 3 doesn't trigger new network request to foo.wbn. (This is intentional for successful cases. Also good for failed cases?) Should the load/error event be fired at Step 3 for the second element?

hayatoito commented 3 years ago

A good question.

For those who are not familiar with the context, see the requirement 2 of <script type=webbundle> design doc, which might be helpful to understand the context more.

IIUC Step 3 doesn't trigger new network request to foo.wbn. (This is intentional for successful cases. Also good for failed cases?)

Yeah, step 3 doesn't trigger new network request. We've not decided the behavior for failed cases. Let's assume step 3 doesn't trigger new network request for a failed case too.

Given this assumption,

Should the load/error event be fired at Step 3 for the second element?

I don't have a strong preference. The behaviors of the Step3 would be one of the followings?

1st one fired a load 1st one fired an error
A fire a load fire an error
B fire nothing fire nothing
C fire a load fire nothing
D fire nothing fire an error

Either A or B looks consistent to me. I am slightly in favor of A if we consider "re-using" is just an internal optimization and want to minimize the visible side effect.

irori commented 3 years ago

+1 to A.

I don't like C and D. From the user's point of view, it is difficult to use if it's not guaranteed that either load or error will be fired.

hiroshige-g commented 3 years ago

+1 to A. It is consistent with other cases where we fire load (and error) events without any fetch requests due to caching in the HTML spec layer:

Also it would keep the invariant of one load or error event for every <script type=webbundle>, which might be easier to understand for users and web developers.

We've not decided the behavior for failed cases.

Yeah, whether we reuse the failed cases or not is a separate issue to be determined.

cjtenny commented 3 years ago

What should occur when:

1. The first `<script type=webbundle>` was added with source = `foo.wbn`.

2. The loading of `foo.wbn` succeeded or failed.

3. Then the first element was replaced with the second `<script type=webbundle>` with source = `foo.wbn`.

IIUC Step 3 doesn't trigger new network request to foo.wbn. (This is intentional for successful cases. Also good for failed cases?) Should the load/error event be fired at Step 3 for the second element?

For bundle-preloading, step 3 depends on the contents of the element. If the resources specification contains new resources, a new request would be issued and a new load event should be fired. For consistency, I'm uncertain whether it'd be better to fire an event or not if no new resources are requested.

Sorry for the delay, have been busy with other things - will be around more starting in ~two more weeks. @felipeerias can clarify further while I'm out!

hiroshige-g commented 2 years ago

My spec PR https://github.com/WICG/webpackage/pull/721 fires load/error events for every <script type=webbundle>, which is consistent with the comments here and corresponds to A in https://github.com/WICG/webpackage/issues/697#issuecomment-953701564.