Release: develop -> main - #1231
Merged
Merged
Conversation
* Answer a missing build asset with 404, not the app shell An asset that a later deploy replaced had no file behind it, and Pages answers such a path with index.html and status 200. The /static/* rule in _headers then stamped that HTML with the cache lifetime meant for fingerprinted assets, so the client parsed an app shell as JavaScript and cached it under the chunk's own URL. A Pages Function now intercepts the asset paths and turns that fallback into a 404 with no-store, which is a failure a client can recover from. Nothing under those paths is ever HTML, so the content type separates a real asset from the fallback; both are status 200. _routes.json restricts the Function to /static/* and /widget/*. Every other request is served statically as before. The widget needed covering too: its chunks load on third-party pages and failed the same way. Measured with `wrangler pages dev` against a real build: existing js/css/chunk assets, the widget entry point, deep links, the root, favicon and manifest are unchanged; an absent asset under either path returns 404. A 404 cannot be expressed in _redirects, and an explicit /static/* rule there takes precedence over real files and shadows the assets themselves. * Bring the asset function under lint and test The function is deployed but was outside every check the CI runs: a syntax error in it would have reached production with a green build. ESLint now covers functions/**/*.js. The type-aware parser cannot resolve files outside tsconfig.json, so that path is linted without type information -- which still catches what matters here. Verified by introducing a syntax error and confirming the lint run fails on it. Four tests pin the single decision the function makes: an asset path answered with HTML becomes 404 with no-store, a real asset passes through with its headers intact, a response without a content type is passed through rather than guessed at, and a non-HTML error status is not rewritten. The function stays out of the app bundle; the built output contains no reference to it. Also corrects a comment that contradicted the one in public/_redirects: the fallback is Pages' own behaviour for a path with no file behind it, not the rule in that file. * Only treat a 200 HTML response as the missing-asset fallback An HTML error page -- a gateway or origin failure rendered as HTML -- was rewritten to 404 like the fallback. That reports a temporary outage as an asset gone for good, and sends the client off to reload when it should retry. The fallback is always status 200, so the condition now requires it. A test covers an HTML 503 passing through as 503. Also drops a claim from public/_redirects that the catch-all rule does not produce the fallback. It rested on local emulator behaviour, which does not establish what the deployed edge does. * Key the fallback check on success, and cut the comments back to what is verified The check required status 200 exactly, so a partial response carrying the fallback would have passed through as a valid asset. It now keys on a successful status, with a test for a 206 fallback. The comments claimed more than this repository shows. Removed: that a 404 makes the client reload while a 503 makes it retry (there is no retry path, and the reload in src/util/client-error.ts does not depend on status); that the catch-all rule behaves differently from an explicit one (the documented behaviour is that redirects apply regardless of a matching asset, and that is what the remaining sentence now says); and that no check reads the function (the new test does). The test's own summary said any HTML response is the fallback, which the HTML-503 case contradicts. * Do not let a conditional request confirm a cached app shell A client that stored the app shell under an asset URL before this existed would have had that entry confirmed on revalidation: the binding answers 304 against the fallback's own validator, 304 is not a successful status, and the response passed straight through. The poisoned entry then lived on. On 304 the lookup is now repeated without the conditional headers, so the path can be identified from what it actually holds. A real asset keeps its 304 and the client keeps its valid copy; the fallback becomes 404. The retry costs a lookup on revalidation only -- a client whose copy is still fresh sends no request, so it never reaches this code. That closes the one case the app-side recovery could not reach. Two tests cover it, including that the conditional headers are actually dropped. Measured against a real build: a conditional request for an existing asset still returns 304, and for an absent one returns 404. * Do not answer 304 when the unconditional retry failed The retry turned every non-HTML response into 304, a failure included. A transient error on the second lookup would then have renewed whatever the client holds -- including the app shell this path exists to remove, which is the opposite of the intent. Only a successful lookup now yields 304. A failure or redirect is passed on as itself, since it establishes nothing about what the path holds.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
August 1, 2026 07:38
TaprootFreak
approved these changes
Aug 1, 2026
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
1 participant
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist