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.

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

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

Open
arenoir opened this issue Mar 23, 2019 · 12 comments
Open

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

arenoir opened this issue Mar 23, 2019 · 12 comments

Comments

@arenoir
Copy link

arenoir commented Mar 23, 2019

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
Copy link
Author

arenoir commented Apr 4, 2019

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.

@arenoir arenoir closed this as completed Apr 4, 2019
@ghost
Copy link

ghost commented Apr 4, 2019

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
Copy link
Author

arenoir commented Apr 4, 2019

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
Copy link
Author

arenoir commented Apr 4, 2019

@arenoir arenoir reopened this Apr 4, 2019
@ghost
Copy link

ghost commented Apr 4, 2019

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

@arenoir
Copy link
Author

arenoir commented Apr 5, 2019

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
Copy link
Contributor

eshtadc commented May 3, 2019

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
Copy link
Author

arenoir commented May 6, 2019

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

@eshtadc
Copy link
Contributor

eshtadc commented May 6, 2019

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

@eshtadc
Copy link
Contributor

eshtadc commented May 24, 2019

@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
Copy link

jfrux commented Nov 5, 2019

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
Copy link
Contributor

eshtadc commented Dec 18, 2019

@jfrux Is that using the PR that we were testing for this issue? #152

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

Successfully merging a pull request may close this issue.

3 participants