Repository navigation
[No QA] Serve the web build's Brotli 11 twins from the Cloudflare Worker - #104493
Conversation
The build already writes a Brotli 11 twin of every js, css, svg, wasm and ttf file, about 20% smaller than Cloudflare's edge compression. The Worker now serves the twin with encodeBody: manual, for current assets and previous builds in R2, and Cloudflare decodes it for clients without Brotli. The asset store gives a twin its original's ETag and the edge weakens ETags it re-encodes, so the Worker drops conditional headers from asset requests, gives twins their own ETag and answers If-None-Match itself; otherwise fixed-name files like service-worker.js stopped revalidating with 304.
The Worker serves a previous build's twin from R2 when the current build no longer has the file.
…-brotli # Conflicts: # web/cloudflare-worker/README.md # web/cloudflare-worker/src/index.ts
The pre-commit hook formatted them with a newer oxfmt than the repository pins.
The twin check ran on the raw pathname, so /foo.js.%62r reached the asset router, which decodes it and served the raw twin. It also sent any navigation ending in .br, such as an email address on a .com.br domain, to a 404 instead of the app shell. It now checks the decoded path against the twinned file types only.
Falling back to an empty tag gave every such twin the same ETag, so a 304 for one file could answer a revalidation of another.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e57f1a60c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ttf: 'font/ttf', | ||
| }; | ||
| const PRECOMPRESSED_PATH = /\.(?:js|css|svg|wasm|ttf)$/; | ||
| const TWIN_PATH = /\.(?:js|css|svg|wasm|ttf)\.br$/i; |
There was a problem hiding this comment.
Production builds also emit HTML twins because config/rsbuild/rsbuild.common.ts:619 includes html, and .assetsignore now uploads every .br file. Since this guard only recognizes JS/CSS/SVG/WASM/TTF twins, a direct request such as /index.html.br bypasses the early 404 and is served by Static Assets, contrary to the documented invariant that twins are never available at their own URLs. Include HTML in the direct-twin guard while continuing to exclude it from PRECOMPRESSED_PATH so nonce-bearing HTML is not selected as an encoded response.
Useful? React with 👍 / 👎.
|
Going to merge this and CP to staging to test. |
|
🚧 roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Cherry-picked to staging by https://github.com/roryabraham in version: 9.5.20-0 🚀
|
|
@MelvinBot now that this is live on staging, check if it's working as expected |
|
The Worker works on staging (9.5.20-0), but Chrome, Edge and Firefox users don't get the Brotli copy yet. Those browsers send
Other checks (all pass)
I tested with |
|
Tried to the find relevant compression rules in Cloudflare, I'm not sure I have access to them. |
Explanation of Change
foo.js.br) of each JS, CSS, SVG, WASM and font file, but nothing served them. The Worker now sends that copy to browsers instead of letting Cloudflare compress the file on each request, somaindownloads 1.12 MB instead of 1.44 MB, and its first byte arrives in 83 ms instead of 210 ms."<etag>-br") and answers revalidation (304) itself./foo.js.bris a404), browsers without Brotli get the file decoded by Cloudflare, and files from previous builds are served from the R2 bucket, compressed copy first.Fixed Issues
$ #101250
For https://github.com/Expensify/Expensify/issues/688126
PROPOSAL:
Tests
web/cloudflare-worker, runnpm testandnpm run typecheckand verify all 33 tests and the typecheck pass.dist/and deploy to test-staging:npx wrangler deploy --env test-staging.curl -s -D - -o /tmp/main.br -H 'Accept-Encoding: gzip, deflate, br, zstd' https://test-staging.new.expensify.com/<main bundle>and verifycontent-encoding: br, an ETag ending in-br", and that/tmp/main.bris byte-for-byte the build's<main bundle>.br.Accept-Encoding: gzipandidentity, and verify gzip and the exact original respectively.service-worker.js,canvaskit-0.41.0.wasm,version.jsonandfavicon.png, send back the received ETag inIf-None-Matchwith each of thoseAccept-Encodingvalues, and verify every request gets304with no body.https://test-staging.new.expensify.com/<main bundle>.br,.%62rand.BRreturn404, that a navigation to/settings/profile/contact-methods/user@example.com.brgets the app shell, and that the app loads with no console errors.This is live on test-staging.new.expensify.com right now, though it might be overwritten if we do another deploy.
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
No QA. Only changes the encoding of the same files on staging and production, verified on test-staging.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari