service-workers icon indicating copy to clipboard operation
service-workers copied to clipboard

Add "clientId" to fetch event

Open jRichardeau opened this issue 5 years ago • 5 comments

Hi,

When using self.trigger("fetch", request) in service-worker-mock we can only pass the request to the listener while in real browser the fetch callback also receives a clientId in the event.

Is it possible to add this ?

I think this should be in the utils/eventHandler.js file.

We could add a third argument to the trigger function that will be deconstruct in the callback event objet.

I mean self.trigger("fetch", request, {clientId: 123})

Thank you

jRichardeau avatar Feb 11 '20 17:02 jRichardeau

Hey! Thanks for looking into a solution. I think the ideal solution would be having the second argument actually match the FetchEvent init value (which arguably should have been done to start with). Since init.request has to be a Request or a string, we could overload the second argument and check if it's an object, Request, or string. Then it and keep backwards compatibility.

self.trigger("fetch", { request, clientId: 123) });

Thoughts? If you want to put up a diff, i'd be happy to review!

zackargyle avatar Feb 11 '20 17:02 zackargyle

My bad I think this is already implemented in the 2.x version, but this version does not work for my case so i'm using 1.x version. Do you think we could update 1.x version with this feature ? If yes which branch should I use ? Thank you

jRichardeau avatar Feb 11 '20 18:02 jRichardeau

I made the changes but I can't push my new branch to the repository.

remote: Permission to zackargyle/service-workers.git denied to jRichardeau.
fatal: unable to access 'https://github.com/zackargyle/service-workers.git/': The requested URL returned error: 403

May I did something wrong ?

jRichardeau avatar Feb 12 '20 17:02 jRichardeau

Can you try forking the repo, submitting the commits to your own branch, and then submitting a pull request to this repo with that commit?

zackargyle avatar Feb 12 '20 17:02 zackargyle

@zackargyle can you please look at #132 I need you to merge the pull request

jRichardeau avatar Mar 24 '20 13:03 jRichardeau