-
-
Notifications
You must be signed in to change notification settings - Fork 518
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.
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
feat: isolate parent and child frames when handling requests #2324
Conversation
@@ -155,6 +160,10 @@ async function handleRequest(event, requestId) { | |||
async function resolveMainClient(event) { | |||
const client = await self.clients.get(event.clientId) | |||
|
|||
if (activeClientIds.has(event.clientId)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix.
@@ -41,6 +43,11 @@ export function printStartMessage(args: PrintStartMessageArgs = {}) { | |||
console.log('Worker scope:', args.workerScope) | |||
} | |||
|
|||
if (args.client) { | |||
// eslint-disable-next-line no-console | |||
console.log('Client ID: %s (%s)', args.client.id, args.client.frameType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also improving the activation message to include the client ID that prompted it, as well as the client frame type (top-level/nested). This helps during debugging.
09fad7d
to
2b1128d
Compare
Also a good point that we still won't properly support this scenario:
Since a.1.1 doesn't call I'd say the expectation here is that the Unfortunately, there's no hierarchical relationship of the service worker clients. You also cannot know the actual client ID on the client to do any sort of logic on the library's side. See w3c/ServiceWorker#1556. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the quick fix!
Released: v2.5.0 🎉This has been released in v2.5.0! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
The problem
If the page calls
worker.start()
and has a child frame that also callsworker.start()
, the requests from the frame will only be resolved against the top-level parent'sworker
and its handlers.The solution
During the
resolveMainFrame
client lookup, if the eventclientId
is in the active client ids of the worker, return it. Don't lookup the clients because the top-level client will always match first, ignoring any underlying otherwise matching nested client ids.