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

core: remove service-worker audit #15257

Merged
merged 9 commits into from
Jul 17, 2023
Merged

core: remove service-worker audit #15257

merged 9 commits into from
Jul 17, 2023

Conversation

adamraine
Copy link
Member

@adamraine adamraine commented Jul 12, 2023

Justification from internal email discussion:

  • A service worker is no longer required for PWA insatiability in Chrome
  • Even before the SW requirement was removed, this audit was mostly redundant with the installable manifest audit
  • Many users see this audit failing and add a pointless service worker just to get it to pass, even though this can hurt performance.

Edit: I may have jumped the gun on removing this audit. We should land #15253 first. Done

Keeping the ServiceWorker gatherer around like we do with IFrameElements. I'm open to removing completely though.

Some SW related drive by updates in core/audits/installable-manifest.js

#14909

@adamraine adamraine requested a review from a team as a code owner July 12, 2023 17:05
@adamraine adamraine requested review from connorjclark and removed request for a team July 12, 2023 17:05
requestedUrl: 'http://localhost:10503/offline-ready.html?slow',
finalDisplayedUrl: 'http://localhost:10503/offline-ready.html?slow',
audits: {
'service-worker': {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only audit we check in this smoke test, so it doesn't seem useful anymore.

@adamraine adamraine merged commit ff2deca into main Jul 17, 2023
24 checks passed
@adamraine adamraine deleted the rm-service-worker-audit branch July 17, 2023 19:43
@khmyznikov
Copy link

@adamraine how to re-enable this audit manually?

@adamraine
Copy link
Member Author

It's not possible ATM, but you could create a custom audit with the same implementation.

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

Successfully merging this pull request may close these issues.

4 participants