Skip to content

service worker removal #173609

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.

Sign up for GitHub

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

Jump to bottom
Closed
SydneyBao wants to merge 13 commits into from
Closed

service worker removal #173609

SydneyBao wants to merge 13 commits into from

Conversation

@SydneyBao
Copy link
Contributor

@SydneyBao SydneyBao commented Aug 12, 2025 •
edited by mdebbar
Loading

Create a service worker that removes the old cached service worker.

Related issue: 156910

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@SydneyBao SydneyBao requested a review from matanlurey as a code owner August 12, 2025 07:54
@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Aug 12, 2025
@SydneyBao SydneyBao mentioned this pull request Aug 12, 2025
Closed
9 tasks
gemini-code-assist[bot]
gemini-code-assist bot reviewed Aug 12, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the automatic service worker registration from Flutter web applications, replacing it with a cleanup worker designed to remove old caches from previous versions. The changes are well-contained within flutter_tools, flutter.js, and the relevant test files. The migration strategy appears sound. I have identified one critical issue in the new test implementation and one medium-severity issue regarding a duplicated test case in the web test suite.

@bkonyi bkonyi requested review from bkonyi and mdebbar and removed request for matanlurey August 12, 2025 14:44
@mdebbar
Copy link
Contributor

mdebbar commented Aug 13, 2025

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Aug 13, 2025
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request removes the automatic service worker registration from Flutter's web loader and replaces the default service worker with a cleanup worker that removes old caches and unregisters itself. The changes are well-implemented and the accompanying test verifies the cleanup behavior. I have a few suggestions to improve test realism and code clarity.

mdebbar
mdebbar reviewed Aug 14, 2025
View reviewed changes
@SydneyBao SydneyBao requested review from a team as code owners August 14, 2025 21:47
@github-actions github-actions bot added a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems platform-android Android applications specifically platform-ios iOS applications specifically framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: internationalization Supporting other languages or locales. (aka i18n) platform-fuchsia Fuchsia code specifically f: scrolling Viewports, list views, slivers, etc. f: cupertino flutter/packages/flutter/cupertino repository platform-windows Building on or for Windows specifically d: examples Sample code and demos f: routes Navigator, Router, and related APIs. platform-linux Building on or for Linux specifically a: desktop Running on desktop labels Aug 14, 2025
@github-actions github-actions bot removed a: desktop Running on desktop f: focus Focus traversal, gaining or losing focus team-android Owned by Android platform team team-ios Owned by iOS platform team d: docs/ flutter/flutter/docs, for contributors labels Aug 22, 2025
@SydneyBao
Copy link
Contributor Author

SydneyBao commented Aug 22, 2025 •
edited
Loading

@SydneyBao Are the files that you modified expected or did you have a bad merge?

fixed!

@ash2moon
Copy link
Contributor

ash2moon commented Aug 25, 2025

Hi @SydneyBao,

It looks like the tests are running into some issues. I suspect the large number of commits in this PR, possibly from an earlier merge conflict, might be causing unexpected problems.

To help resolve this, could you please squash all your changes into a single commit? This will clean up the commit history and should help the tests run correctly.

Here is a safe way to do this:

  1. Create a new branch from your current one to ensure no work is lost:

    # Make sure you are on your `final_service_worker_removal` branch first
git checkout -b final_service_worker_removal_squashed

  2. Combine all commits into a single set of changes:

    git reset --soft upstream/master

  3. Create a new commit with all the combined changes:

    git commit -m "feat: Remove service worker"

After that, please open a new pull request with this new branch, and check to see if the tests are running properly now.

@SydneyBao
Copy link
Contributor Author

SydneyBao commented Sep 5, 2025

Hi @SydneyBao,

It looks like the tests are running into some issues. I suspect the large number of commits in this PR, possibly from an earlier merge conflict, might be causing unexpected problems.

To help resolve this, could you please squash all your changes into a single commit? This will clean up the commit history and should help the tests run correctly.

Here is a safe way to do this:

  1. Create a new branch from your current one to ensure no work is lost: 
    # Make sure you are on your `final_service_worker_removal` branch first
git checkout -b final_service_worker_removal_squashed
  2. Combine all commits into a single set of changes: 
    git reset --soft upstream/master
  3. Create a new commit with all the combined changes: 
    git commit -m "feat: Remove service worker"

After that, please open a new pull request with this new branch, and check to see if the tests are running properly now.

Hi @SydneyBao,

It looks like the tests are running into some issues. I suspect the large number of commits in this PR, possibly from an earlier merge conflict, might be causing unexpected problems.

To help resolve this, could you please squash all your changes into a single commit? This will clean up the commit history and should help the tests run correctly.

Here is a safe way to do this:

  1. Create a new branch from your current one to ensure no work is lost: 
    # Make sure you are on your `final_service_worker_removal` branch first
git checkout -b final_service_worker_removal_squashed
  2. Combine all commits into a single set of changes: 
    git reset --soft upstream/master
  3. Create a new commit with all the combined changes: 
    git commit -m "feat: Remove service worker"

After that, please open a new pull request with this new branch, and check to see if the tests are running properly now.

Hi @ ash2moon. Thank you! I have created a new pr here: #174991

@ash2moon ash2moon mentioned this pull request Sep 8, 2025
Closed
9 tasks
@mdebbar
Copy link
Contributor

mdebbar commented Sep 19, 2025

@SydneyBao the failures were because of formatting and some tests needed to be removed. I fixed those, let's see if it works now.

@mdebbar mdebbar force-pushed the final_service_worker_removal branch from 3f0952e to 4dc1ea9 Compare September 19, 2025 19:59
@mdebbar
Copy link
Contributor

mdebbar commented Sep 19, 2025

Some of the commits that resulted from the merge conflict were causing weirdness with CLA signing.

I squashed everything in one commit and that made the CLA check happy.

@mdebbar mdebbar mentioned this pull request Oct 10, 2025
Merged
@mdebbar mdebbar closed this Oct 10, 2025
@mdebbar
Copy link
Contributor

mdebbar commented Oct 16, 2025

This is being worked on here: #176834

github-merge-queue bot pushed a commit that referenced this pull request Oct 17, 2025
@mdebbar
[web] Self-cleaning service worker (#176834)
f94a942 
Introduce a self-cleaning service worker to replace the old one.

Previous attempts (#170918,
#173609) had (accidentally?)
disabled the loading of the service worker in `flutter.js`. This PR
preserves the logic for loading the service worker, and it prints a
deprecation warning.

Towards #156910
Closes #106225
reidbaker pushed a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
@mdebbar @reidbaker
[web] Self-cleaning service worker (flutter#176834)
bbbd8d8 
Introduce a self-cleaning service worker to replace the old one.

Previous attempts (flutter#170918,
flutter#173609) had (accidentally?)
disabled the loading of the service worker in `flutter.js`. This PR
preserves the logic for loading the service worker, and it prints a
deprecation warning.

Towards flutter#156910
Closes flutter#106225
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdebbar mdebbar mdebbar left review comments

@bkonyi bkonyi Awaiting requested review from bkonyi

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

engine flutter/engine related. See also e: labels. platform-web Web applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SydneyBao @mdebbar @camsim99 @ash2moon