Repository navigation
Conversation
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
PR Summary by QodoKeep font URLs external in minified CSS
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can route each severity your way: inline, summary, both, or drop |
compressCSS() bundled every stylesheet with esbuild using `dataurl` loaders for .ttf/.otf/.woff/.woff2/.eot, so every `url()` pointing at a font was embedded as base64. For the pad page that made `static/css/pad.css` ~1.7 MB, because it pulls in the editor fonts (Montserrat, OpenDyslexic, Roboto, Roboto Mono, Quicksand, Alegreya) and the icon font in three formats. Every visitor had to download all of it — including the formats their browser never uses — before the pad could render, and again after each upgrade because the `?v=` cache key changes. Mark the font URLs external instead, via an esbuild onResolve plugin, so the browser fetches only the faces it needs, lazily, and caches them separately from the CSS. Images are still inlined as before. esbuild offers no declarative way to do this: the `external` option applies only to imports, the `file` loader requires an output path, and external globs accept a single wildcard so they cannot match the query-string icon-font URLs such as `fontawesome-etherpad.woff?2`. The plugin matches those too. The fonts are referenced from the CSS by relative path, so the emitted URLs resolve to the same files as before and no server-side changes are needed. The regression test asserts that no font data URLs remain in the bundled stylesheet, that the output stays small, that each @font-face source is still present as an external URL, and that every such URL resolves to a real file. It fails if the plugin is removed. Fixes ether#8268
sqmyou
force-pushed
the
fix/8268-font-urls
branch
from
October 7, 2026 17:58
a0f0014 to
cb6affc
Compare
This branch has not been deployed
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.
Fixes #8268
What and why
compressCSS()bundles each stylesheet with esbuild usingdataurlloaders for.ttf,.otf,.woff,.woff2and.eot. Everyurl()pointing at a font was therefore embedded in the CSS as base64.For the pad page that meant
static/css/pad.csswas about 1.7 MB, because it pulls in the editor fonts (Montserrat, OpenDyslexic, Roboto, Roboto Mono, Quicksand, Alegreya) and the icon font in three formats. Every visitor had to download all of it — including the formats their browser never uses — before the pad could render, and again after each upgrade because the?v=cache key changes.This marks the font URLs external instead, so the browser fetches only the faces it needs, lazily, and caches them separately from the CSS. Images are still inlined exactly as before.
static/css/pad.cssWhy a plugin
esbuild has no declarative way to keep CSS
url()s external. I tried the obvious options first and each fails:external: ['.ttf', ...]— theexternaloption only applies to imports, not CSSurl().loader: {'.ttf': 'file'}— requires an output path and returns separate files.external: ['*/font/*']— works for the plain paths but misses the icon font, whose URLs carry query strings (fontawesome-etherpad.woff?2).external: ['*.ttf', '*.woff?2', ...]— esbuild allows only a single*wildcard, so these cannot match.An
onResolveplugin matching the font extensions (including the query-string variants) handles all of them.The fonts are referenced from the CSS by relative path, so the emitted URLs resolve to the same files as before and no server-side changes are needed.
Tests
src/tests/backend/specs/cssFontInlining.tsasserts that no font data URLs remain, that the output stays small, that each@font-facesource is still present as an external URL, and that every such URL resolves to a real file.I confirmed the test fails if the plugin is removed, so it is a genuine regression test rather than a tautology.
Verification I ran locally:
tsc --noEmit: cleanGET /static/css/pad.cssreturns 61,002 bytes with 0 inlined fonts; the resolved font URLs return 200 withcontent-type: font/otf, including the query-string icon fontNote on lint
pnpm run lintfails on a cleandevelopcheckout in this environment (ESLint 10.12.0cannot findeslint.config.*; the repo still uses.eslintrc.cjs). This is unrelated to the change — it reproduces with my edits stashed.