Skip to content

fix(service-worker): correctly determine client ID on navigation requests#42607

Closed
gkalpak wants to merge 2 commits intoangular:masterfrom
gkalpak:fix-sw-navigation-client-id
Closed

fix(service-worker): correctly determine client ID on navigation requests#42607
gkalpak wants to merge 2 commits intoangular:masterfrom
gkalpak:fix-sw-navigation-client-id

Conversation

@gkalpak
Copy link
Copy Markdown
Member

@gkalpak gkalpak commented Jun 19, 2021

The ServiceWorker assigns an app-version to a each client to ensure that all subsequent requests for a client are served using the same app-version. The assignment is done based on the client ID.

Previously, the ServiceWorker would only try to read the client's ID off of the FetchEvent's clientId property. However, for navigation requests the new client's ID will be set on resultingClientId, while clientId will either be empty or hold the ID of the client where the request initiated from. See also related discussions in w3c/ServiceWorker#870 and w3c/ServiceWorker#1266.

In theory, this could lead to the navigation request (i.e. index.html) being served from a different app-version than the subsequent sub-resource requests (i.e. assets). In practice, the likelihood of this happening is probably very low though, since it would require the latest app-version to be updated between the initial navigation request and the first sub-resource request, which should happen very shortly after the navigation request.

This commit ensures that the correct client ID is determined even for navigation requests by also taking the resultingClientId property into account.

@gkalpak gkalpak added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: service-worker Issues related to the @angular/service-worker package labels Jun 19, 2021
@ngbot ngbot bot modified the milestone: Backlog Jun 19, 2021
@google-cla google-cla bot added the cla: yes label Jun 19, 2021
@pullapprove pullapprove bot requested a review from AndrewKushnir June 19, 2021 14:52
@mary-poppins
Copy link
Copy Markdown

You can preview be5e838 at https://pr42607-be5e838.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

You can preview b18287d at https://pr42607-b18287d.ngbuilds.io/.

@gkalpak gkalpak requested a review from IgorMinar June 19, 2021 15:53
@gkalpak gkalpak removed the request for review from IgorMinar June 22, 2021 07:23
gkalpak added 2 commits June 22, 2021 16:09
…ests

The ServiceWorker assigns an app-version to a each client to ensure that
all subsequent requests for a client are served using the same
app-version. The assignment is done based on the client ID.

Previously, the ServiceWorker would only try to read the client's ID off
of the `FetchEvent`'s `clientId` property. However, for navigation
requests the new client's ID will be set on [resultingClientId][1],
while `clientId` will either be empty or hold the ID of the client where
the request initiated from. See also related discussions in
w3c/ServiceWorker#870 and w3c/ServiceWorker#1266.

In theory, this could lead to the navigation request (i.e. `index.html`)
being served from a different app-version than the subsequent
sub-resource requests (i.e. assets). In practice, the likelihood of this
happening is probably very low though, since it would require the latest
app-version to be updated between the initial navigation request and the
first sub-resource request, which should happen very shortly after the
navigation request.

This commit ensures that the correct client ID is determined even for
navigation requests by also taking the `resultingClientId` property into
account.

[1]: https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent/resultingClientId
@gkalpak gkalpak force-pushed the fix-sw-navigation-client-id branch from b18287d to fac611a Compare June 22, 2021 13:14
@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 22, 2021
@mary-poppins
Copy link
Copy Markdown

You can preview fac611a at https://pr42607-fac611a.ngbuilds.io/.

@dylhunn dylhunn closed this in 9498da1 Jun 22, 2021
dylhunn pushed a commit that referenced this pull request Jun 22, 2021
…ests (#42607)

The ServiceWorker assigns an app-version to a each client to ensure that
all subsequent requests for a client are served using the same
app-version. The assignment is done based on the client ID.

Previously, the ServiceWorker would only try to read the client's ID off
of the `FetchEvent`'s `clientId` property. However, for navigation
requests the new client's ID will be set on [resultingClientId][1],
while `clientId` will either be empty or hold the ID of the client where
the request initiated from. See also related discussions in
w3c/ServiceWorker#870 and w3c/ServiceWorker#1266.

In theory, this could lead to the navigation request (i.e. `index.html`)
being served from a different app-version than the subsequent
sub-resource requests (i.e. assets). In practice, the likelihood of this
happening is probably very low though, since it would require the latest
app-version to be updated between the initial navigation request and the
first sub-resource request, which should happen very shortly after the
navigation request.

This commit ensures that the correct client ID is determined even for
navigation requests by also taking the `resultingClientId` property into
account.

[1]: https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent/resultingClientId

PR Close #42607
@gkalpak gkalpak deleted the fix-sw-navigation-client-id branch June 22, 2021 16:29
gkalpak added a commit to gkalpak/angular that referenced this pull request Jul 7, 2021
Update `@angular/*` packages to latest 12.1.x versions (mainly to take
advantage of recent ServiceWorker improvements, such as angular#42607
and angular#42622).
atscott pushed a commit that referenced this pull request Jul 7, 2021
Update `@angular/*` packages to latest 12.1.x versions (mainly to take
advantage of recent ServiceWorker improvements, such as #42607
and #42622).

PR Close #42776
atscott pushed a commit that referenced this pull request Jul 7, 2021
Update `@angular/*` packages to latest 12.1.x versions (mainly to take
advantage of recent ServiceWorker improvements, such as #42607
and #42622).

PR Close #42776
@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: service-worker Issues related to the @angular/service-worker package cla: yes target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants