Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(pwa): only count clients in scope #760

Merged
merged 1 commit into from
Nov 24, 2022
Merged

fix(pwa): only count clients in scope #760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions pwa/src/service-worker/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,23 @@ export async function getClientsInfo(event) {

// Include uncontrolled clients: necessary to know if there are multiple
// tabs open upon first SW installation
const clientsList = await self.clients.matchAll({
includeUncontrolled: true,
})
const filteredClientsList = await self.clients
.matchAll({
includeUncontrolled: true,
Copy link
Member

@tomzemp tomzemp Nov 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would setting includeUncontrolled: false work here?
From https://developer.mozilla.org/en-US/docs/Web/API/Clients/matchAll:

includeUncontrolled: A boolean value — if set to true, the matching operation will return all service worker clients who share the same origin as the current service worker. Otherwise, it returns only the service worker clients controlled by the current service worker. The default is false.

It seems like client starting with the same scope would be the same as a client controlled by the current service worker?

Copy link
Contributor Author

@KaiVandivier KaiVandivier Nov 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a weird thing: we do sometimes want to include uncontrolled clients that are still in the scope of the service worker, because there are some cases where a client has an active service worker but the client is not controlled, for example during the first installation of the service worker in an app, or after a 'hard reload'. I have no idea why the includeUncontrolled option goes on to include all clients on that domain though, even outside the service worker scope 🤷

Just to be sure, I did an experiment and logged the different results of matchAll; here's what it looks like with one uncontrolled tab and another one right after a hard reload -- neither are controlled, but both are in scope and of interest to getClientsInfo:
Screen Shot 2022-11-05 at 1 37 41 PM

(So in conclusion, we need the includeUncontrolled option, but that means we need to filter out the clients that are out of this SW's scope)

})
.then((clientsList) =>
// Filter to just clients within this SW scope, because other clients
// on this domain but outside of SW scope are returned otherwise
clientsList.filter((client) =>
client.url.startsWith(self.registration.scope)
)
)

self.clients.get(clientId).then((client) => {
client.postMessage({
type: swMsgs.clientsInfo,
payload: {
clientsCount: clientsList.length,
clientsCount: filteredClientsList.length,
},
})
})
Expand Down