Skip to content
This repository has been archived by the owner on Sep 9, 2021. It is now read-only.

feat: add support for ServiceWorkers #14

Closed
wants to merge 3 commits into from
Closed

feat: add support for ServiceWorkers #14

wants to merge 3 commits into from

Conversation

AsaAyers
Copy link

@AsaAyers AsaAyers commented Jan 4, 2016

Fixes #11

@AsaAyers AsaAyers mentioned this pull request Jan 4, 2016
@opatut
Copy link

opatut commented Jan 8, 2016

May I, at this point, say thank you for implementing this? Cheers 🎉 👍

@markdalgleish
Copy link

I'd love for this to get merged in so I can deprecate https://github.com/markdalgleish/serviceworker-loader.

@AsaAyers
Copy link
Author

This PR is pretty straight forward, but I think the more important part is #15. I don't know what useful things people would do with a service worker that don't include the list of assets from webpack that need to be cached. #15 introduces a magic __assets__ array.

#15 functions, but really needs some help before it's ready to merge. Any help would be greatly appreciated.

@AsaAyers
Copy link
Author

AsaAyers commented Apr 7, 2016

@markdalgleish Are you able to help get both of my PRs merged? #15 still needs some help. I've had these PRs open for 3 months now and you're the only webpack member I've seen a response from.

@patrickdet
Copy link

this loader works well for me so far. love it. thx @AsaAyers

constructor = "require(" + JSON.stringify("!!" + path.join(__dirname, "createInlineWorker.js")) + ")(" +
var constructor
if(query.service) {
constructor = "navigator.serviceWorker.register(__webpack_public_path__ + " + JSON.stringify(workerFile) + ", options);"
Copy link

Choose a reason for hiding this comment

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

Can you add detection here? Worker is pretty much defined everywhere, serviceWorker isn't

maybe something like
navigator.serviceWorker ? navigator.serviceWorker.register(__webpack_public_path__ + " + JSON.stringify(workerFile) + ", options) : Promise.reject('No support')

Copy link
Author

Choose a reason for hiding this comment

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

ok, now it rejects if navigator.serviceWorker is missing.

@AsaAyers
Copy link
Author

@markdalgleish I see you're a webpack member now. Are you able to help with this and #15?

@HyeonuPark
Copy link

HyeonuPark commented Sep 1, 2016

Is there some example to use this with SW? I don't think the way below is correct.

myServiceWorker = require('worker!./sw.js')

navigator.serviceWorker.register(myServiceWorker)

Edit: I thought it's already merged so I was finding it's usage in master branch. Sorry for confusing you!

@AsaAyers
Copy link
Author

AsaAyers commented Sep 1, 2016

If you want a regular worker the code is:

myServiceWorker = require('worker!./sw.js')

new myServiceWorker()

On my branch you create a service worker using:

myServiceWorker = require('worker?service!./sw.js')

new myServiceWorker()

var constructor
if(query.service) {
constructor = "('serviceWorker' in navigator)" +
"? navigator.serviceWorker.register(__webpack_public_path__ + " + JSON.stringify(workerFile) + ", options);" +

Choose a reason for hiding this comment

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

I'm getting a syntax error in the generated code on this line, caused by the semi-colon before the ternary operator colon:

If you remove it, it seems to work correctly.

? navigator.serviceWorker.register(__webpack_public_path__ + " + JSON.stringify(workerFile) + ", options) " 

@michael-ciniawsky michael-ciniawsky changed the title Add support for Service Workers feat: add support for ServiceWorkers Jul 4, 2017
@michael-ciniawsky
Copy link
Member

Duplicate of #32 ?

@michael-ciniawsky michael-ciniawsky added this to the 0.9.0 milestone Jul 4, 2017
@michael-ciniawsky michael-ciniawsky added this to Feature in Dashboard Jul 4, 2017
@AsaAyers
Copy link
Author

I'm making https://github.com/pulls more useful to me today by closing my old PRs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Dashboard
Feature
Development

Successfully merging this pull request may close these issues.

None yet

8 participants