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

Service worker breaks AMP validation #2264

Closed
asendia opened this issue Aug 22, 2021 · 1 comment · Fixed by #2265
Closed

Service worker breaks AMP validation #2264

asendia opened this issue Aug 22, 2021 · 1 comment · Fixed by #2265
Labels
service worker Stuff related to service workers

Comments

@asendia
Copy link
Contributor

asendia commented Aug 22, 2021

Describe the bug

Issue: Creating src/service-worker.ts breaks AMP validation in Google Webmaster because svelte-kit injects custom <script> tag to install the service-worker.
webmaster error

Expected: svelte-kit should inject amp-install-serviceworker if config.kit.amp is true or let users to customize it.

PR: #2265

Reproduction

  1. Create src/service-worker.ts
  2. Set config.kit.amp = true in svelte.config.js
  3. There will be no AMP validation error during development, but it will result on AMP entries being removed from google with the following error in Google webmaster dashboard: Custom JavaScript is not allowed.
    Example: https://github.com/asendia/salmonfit/tree/f5aea01869760c83d57403a8cad3e1bfdee8e7aa

Logs

No response

System Info

System:
    OS: macOS 11.5.1
    CPU: (8) arm64 Apple M1
    Memory: 228.31 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.1.0 - ~/.nvm/versions/node/v16.1.0/bin/node
    Yarn: 1.22.10 - ~/.nvm/versions/node/v16.1.0/bin/yarn
    npm: 7.11.2 - ~/.nvm/versions/node/v16.1.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.159
    Firefox: 91.0.1
    Safari: 14.1.2
  npmPackages:
    @sveltejs/adapter-netlify: ^1.0.0-next.29 => 1.0.0-next.29 
    @sveltejs/kit: next => 1.0.0-next.154 
    svelte: ^3.34.0 => 3.42.1

Severity

serious, but I can work around it

Additional Information

Workaround that I did: ugly postinstall script to modify ssr.js: https://github.com/asendia/salmonfit/blob/3d9b0316a85c6164db01946529bc8b15d0ec31b8/scripts/replace-sw-amp.js

@ignatiusmb ignatiusmb added the service worker Stuff related to service workers label Aug 23, 2021
@benmccann
Copy link
Member

@asendia what do you think about #2281 as a fix for this? It allows you to turn off automatic service worker registration so that you can handle it yourself

I'm not terribly familiar with either AMP no service workers, so I don't fully understand the issue you're facing

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

Successfully merging a pull request may close this issue.

3 participants