DockYard / ember-service-worker

A pluggable approach to Service Workers for Ember.js
http://ember-service-worker.com
MIT License
236 stars 63 forks source link

sw-registration.js is fingerprinted incorrectly when rootURL is set. #148

Open arenoir opened 5 years ago

arenoir commented 5 years ago

I am trying to setup service workers for an ember 3.4 app. The app is served from a sub directory '/mobile/' so the config/environment.js rootURL is set to '/mobile/'.

In development the sw-registration.js script is added to the index.html page and mobile is prepended. However when deployed via ember-cli-deploy the sw-registration.js is fingerprinted and a incorrect '/mobile/' path is added. (I say incorrect because all other assets are not prefixed by a mobile path).

All assets are fingerprinted and served from a cdn. I am unsure how or if the app should fingerprint the sw-registration.js file.

arenoir commented 5 years ago

I believe it is correct to append the rootURL to the path as the sw.js and sw-registration.js files are not asset files but rather application files. The difference being asset files can be served from a cdn.

ghost commented 5 years ago

Yes, sw.js and sw-registration.js should have rootURL. Only sw.js is an 'application' file. It's totally harmless to fingerprint sw-registration and can be served from CDN.

arenoir commented 5 years ago

Okay good to know. If the sw-registration.js file is fingerprinted then shouldn't it be treated like every other asset file and the rootURL not appended?

arenoir commented 5 years ago

https://ember-cli.com/user-guide/#deployments

"the asset URLs will not use rootURL and will instead be: https://cdn.example.com/assets/vendor-3b1b39893d8e34a6d0bd44095afcd5c4.js."

ghost commented 5 years ago

Looks like you can update https://github.com/DockYard/ember-service-worker/blob/master/index.js#L47 to make that behaviour work.

arenoir commented 5 years ago

Thanks @martndemus for the suggestion. I started to refactor the add-on to omit the rootURL from the sw-registration.js src if the fingerprint prepend option is present. But because _getRootURL() function is used by the sw.js file as well, so I punted.

It turns out there is a config option that handles this better any how. By using the inline registrationStrategy we don't have to worry about the rootURL being appended to the fingerprint.prepend string.

    //ember-cli-build.js
    'ember-service-worker': {
      registrationStrategy: 'inline'
    }
eshtadc commented 5 years ago

I've taken a stab at adjusting this. Seems the current logic is fine unless we are dealing with a fingerprinted asset (sw-registration) in production. The serviceWorkerBuilder only uses the root url for the sw.js file so I made that more explicit. Otherwise, the rest of the calls are for sw-registration.js for different registration strategies. Still have tests to write, but figured I'd check to see if this seems like it's going in the right direction.

arenoir commented 5 years ago

@eshtadc it looks good to me. I will pull it into my application and give it a try.

eshtadc commented 5 years ago

Thanks @arenoir - would love to hear how it works for your scenario "in the wild".

eshtadc commented 5 years ago

@arenoir Have you had a chance to try this out? I've got a little time to work on it again and add tests, etc. if the approach is reasonable.

jfrux commented 5 years ago

My current issue related to this is that my rootURL is set like rootUrl: "", and prepend: "" and yet it's still coming out at /sw-registration-58fcbda0b51af71b8d090b023db785db.js

eshtadc commented 4 years ago

@jfrux Is that using the PR that we were testing for this issue? https://github.com/DockYard/ember-service-worker/pull/152