fix(sw): return inner promise in activate cache cleanup#7928
Conversation
The cache.keys().then() chain inside the activate event handler was not being returned, so event.waitUntil() resolved immediately with undefined before any stale cache entries were deleted. This meant old cache entries from previous deployments would accumulate in users' browsers silently with no guarantee of cleanup. Fix by returning the inner promise chain and using Promise.all() to delete stale entries in parallel, ensuring event.waitUntil() only resolves once all deletions are complete.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@evenstensberg Please take a look |
|
@ryzrr Do you have an issue about it? |
|
@alexander-akait No existing issue for this one , I noticed it while going through the sw code and will be happy to open an issue first if you prefer that before raising a PR. |
|
@ryzrr No need, just a question |
| caches.open(cacheName).then((cache) => { | ||
| caches.open(cacheName).then((cache) => | ||
| // clean up those who are not listed in manifestURLs | ||
| cache.keys().then((keys) => { |
There was a problem hiding this comment.
To be fair, the inner Promise doesn't need to be returned. But indeed the outer Promise will resolve eagerly when the inner one hasn't finished resolving yet.
There was a problem hiding this comment.
Thanks for the clarification ! My intention was to ensure that all cache delete operations are awaited so event wait until resolves only after the cleanup finishes.
The cache.keys().then() chain inside the activate event handler was not being returned, so event.waitUntil() resolved immediately with undefined before any stale cache entries were deleted. This meant old cache entries from previous deployments would accumulate in users' browsers silently with no guarantee of cleanup. Fix by returning the inner promise chain and using Promise.all() to delete stale entries in parallel, ensuring event.waitUntil() only resolves once all deletions are complete.
4 participants
Summary
In the service worker's activate event handler, the inner cache.keys().then() promise was never returned from the outer .then() callback. this caused event.waitUntil() to resolve immediately with undefined before any stale cache entries were deleted. Old previous cache entries from previous deployment would be accumulated in users browsers with no guaranteed of cleanup. this fix returns the full promise chain and replaces the for loop with Promise.all() so all deletions run in parallel and event.waitUntil() only resolves once all stale entries are gone.
What kind of change does this PR introduce?
fix
Did you add tests for your changes?
No, the service worker runs in a browser context outside the Jest test environment.
Does this PR introduce a breaking change?
No
If relevant, what needs to be documented once your changes are merged or what have you already documented?
N/A
Use of AI
N/A