Fix navigation preload cache hit examples#44138
Merged
pepelsbey merged 2 commits intoMay 29, 2026
Merged
Conversation
This comment has been minimized.
This comment has been minimized.
Contributor
|
Preview URLs (2 pages)
(comment last updated: 2026-05-28 10:58:37) |
6d8e326 to
bd97d19
Compare
pepelsbey
requested changes
May 20, 2026
Member
pepelsbey
left a comment
There was a problem hiding this comment.
Looks good! Thank you 🙂
Though, I believe these Promise wrappers are not needed, as event.preloadResponse already a Promise.
| addEventListener("fetch", (event) => { | ||
| event.respondWith( | ||
| (async () => { | ||
| const preloadResponsePromise = Promise.resolve(event.preloadResponse); |
Member
There was a problem hiding this comment.
Suggested change
| const preloadResponsePromise = Promise.resolve(event.preloadResponse); | |
| const preloadResponsePromise = event.preloadResponse; |
| cacheFirst({ | ||
| request: event.request, | ||
| preloadResponsePromise: event.preloadResponse, | ||
| preloadResponsePromise: Promise.resolve(event.preloadResponse), |
Member
There was a problem hiding this comment.
Suggested change
| preloadResponsePromise: Promise.resolve(event.preloadResponse), | |
| preloadResponsePromise: event.preloadResponse, |
Contributor
Author
|
Thanks for the review. I removed the unnecessary Promise.resolve wrappers in 3dfabd2 and now pass/use event.preloadResponse directly while keeping the cache-hit waitUntil handling. Validation:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Updates the navigation preload examples so they keep the preloaded response promise alive when a cache match wins. The examples now capture a normalized
preloadResponsepromise before checking the cache, pass it through the cache-first helper, and useevent.waitUntil()on cache hits so the browser-started preload request can settle cleanly.Motivation
When navigation preload is enabled and the cache wins, the samples returned before the preload promise settled. In Chrome this can produce the warning that the navigation preload request was cancelled before
preloadResponsesettled, and thatwaitUntil()orrespondWith()should be used to wait for it.Keeping the promise alive makes the examples safer for readers who copy them into real service workers, while
Promise.resolve()keeps the samples tolerant ifpreloadResponseis unavailable or resolves toundefined.Additional details
Reference material:
FetchEvent.preloadResponse: https://developer.mozilla.org/docs/Web/API/FetchEvent/preloadResponseValidation run locally on
bd97d19:git diff --check origin/main...HEAD./node_modules/.bin/prettier --check files/en-us/web/api/fetchevent/preloadresponse/index.md files/en-us/web/api/service_worker_api/using_service_workers/index.md./node_modules/.bin/markdownlint-cli2 files/en-us/web/api/fetchevent/preloadresponse/index.md files/en-us/web/api/service_worker_api/using_service_workers/index.mdRelated issues and pull requests
Fixes #43055