Conversation
Greptile SummaryThis PR improves Lighthouse scores through five coordinated changes: (1) build-time pre-compression of static assets (gzip/brotli/zstd) via a new Vite plugin and
Confidence Score: 4/5Safe to merge for most users; Tailwind v4 users will see a regression where Tailwind CSS is not injected after the granular Radix import path change. One P1 finding: the _RADIX_IMPORT_RE regex in shared_tailwind.py uses url(...) syntax but Tailwind v4's new template emits bare @import "…" layer(components), so add_tailwind_to_css_file never injects Tailwind for v4 users. The remaining findings are P2. All other new functionality is well-implemented and well-tested. packages/reflex-base/src/reflex_base/plugins/shared_tailwind.py — _RADIX_IMPORT_RE regex needs to cover the v4 bare @import format Important Files Changed
Sequence DiagramsequenceDiagram
participant App as reflex build
participant Vite as Vite + Plugins
participant FS as Static FS
participant ASGI as PrecompressedStaticFiles
participant Browser
App->>Vite: vite build
Vite->>FS: write JS/CSS/HTML chunks
Vite->>FS: compressPlugin → write .gz/.br sidecars
Vite->>FS: imageOptimizePlugin → write .webp/.avif sidecars
App->>FS: duplicate route index.html + copy sidecars
App->>FS: copy SPA fallback → 404.html + sidecars
App->>FS: _compress_static_output (no-op if Vite already ran)
Browser->>ASGI: GET /app.js (Accept-Encoding: br, gzip)
ASGI->>FS: lookup app.js.br
FS-->>ASGI: found
ASGI-->>Browser: 200 app.js.br (Content-Encoding: br, Vary: Accept-Encoding)
Browser->>ASGI: GET /hero.png (Accept: image/avif, image/webp)
ASGI->>FS: lookup hero.avif
FS-->>ASGI: found
ASGI-->>Browser: 200 hero.avif (Content-Type: image/avif, Vary: Accept)
Reviews (2): Last reviewed commit: "fix: expose dynamic component tags on wi..." | Re-trigger Greptile |
| try: | ||
| subprocess.run( | ||
| command, | ||
| check=True, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=180, | ||
| ) | ||
| except subprocess.CalledProcessError as err: | ||
| pytest.fail( | ||
| "Lighthouse execution failed.\n" | ||
| f"Command: {' '.join(command)}\n" | ||
| f"stdout:\n{err.stdout}\n" | ||
| f"stderr:\n{err.stderr}" | ||
| ) |
There was a problem hiding this comment.
subprocess.TimeoutExpired not handled
subprocess.run() is called with timeout=180, but only CalledProcessError is caught. If Lighthouse takes longer than 180 seconds, subprocess.TimeoutExpired will propagate uncaught, producing a raw Python traceback in CI output rather than a friendly pytest.fail() message.
| try: | |
| subprocess.run( | |
| command, | |
| check=True, | |
| capture_output=True, | |
| text=True, | |
| timeout=180, | |
| ) | |
| except subprocess.CalledProcessError as err: | |
| pytest.fail( | |
| "Lighthouse execution failed.\n" | |
| f"Command: {' '.join(command)}\n" | |
| f"stdout:\n{err.stdout}\n" | |
| f"stderr:\n{err.stderr}" | |
| ) | |
| try: | |
| subprocess.run( | |
| command, | |
| check=True, | |
| capture_output=True, | |
| text=True, | |
| timeout=180, | |
| ) | |
| except subprocess.TimeoutExpired as err: | |
| pytest.fail( | |
| f"Lighthouse timed out after {err.timeout}s.\n" | |
| f"Command: {' '.join(command)}" | |
| ) | |
| except subprocess.CalledProcessError as err: | |
| pytest.fail( | |
| "Lighthouse execution failed.\n" | |
| f"Command: {' '.join(command)}\n" | |
| f"stdout:\n{err.stdout}\n" | |
| f"stderr:\n{err.stderr}" | |
| ) |
| - name: Install playwright | ||
| run: uv run playwright install chromium --only-shell |
There was a problem hiding this comment.
The lighthouse job falls back to npx --yes lighthouse@13.1.0 (via get_lighthouse_command()) when no lighthouse binary is found, relying on whatever Node.js version is pre-installed on ubuntu-22.04. The exact Node.js version shipped with GitHub's runner images can change without notice and is not formally guaranteed.
Adding an explicit actions/setup-node step would make the workflow reproducible across runner image updates:
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: "20"
- name: Install playwright
run: uv run playwright install chromium --only-shell- Wrap blank template in rx.el.main landmark with page title/description - Add aria_label/title to ColorModeIconButton and StickyBadge - Add landing page prod benchmark alongside blank template test - Document page structure and metadata best practices
Add a Vite build plugin that generates .gz copies of assets for sirv to serve pre-compressed. Split socket.io and radix-ui into separate chunks for better caching. Refactor lighthouse benchmark to use `reflex run --env prod` directly instead of AppHarnessProd.
|
Shouldn't the compression be left to the reverse-proxy/ingress/deployment? If not, consider adding brotli alongside gzip? |
Yes, generally. However, better performance can be achieved by configuring the reverse proxy to serve the pre-compressed files so they don't need to be compressed on the fly when requested. @FarhanAliRaza a good point is raised here though, I think the vite compress plugin should be able to handle output to gzip, brotli, or zstd for completeness. We should add some documentation to the self-hosting page about how to enable and use pre-compressed responses in caddy and nginx. The other thing is that I found this old, possibly unmaintained starlette plugin: https://github.com/magnuswatn/starlette-precompressed-static It pretty much does what we want with the exception that it doesn't support Because compression does touch on the intersection between application and deployment, we need to make sure these options are configurable by the end user. I don't think anyone is directly reverse proxying to a |
…ic assets Support gzip, brotli, and zstd build-time pre-compression via the new `frontend_compression_formats` config option. The Vite compress plugin now handles multiple formats, and a new PrecompressedStaticFiles middleware negotiates Accept-Encoding to serve matching sidecar files directly. Replace the custom http.server-based prod test harness with Starlette/Uvicorn to match the production serving stack. Remove redundant post-build compression pass that was re-compressing assets already handled by the Vite plugin. Move sidecar stat calls off the async event loop into a worker thread.
…lities - Extract shared walkFiles/outputDirectoryExists/validateFormats into vite-plugin-utils.js - Add vite-plugin-image-optimize for WebP/AVIF sidecar generation - Add vite-plugin-purgecss to strip unused CSS from radix-ui - Add bounded concurrency to compression plugin - Simplify format normalization: config validates, downstream just looks up - Fix sidecar copy only running when target HTML was actually created - Remove blank page lighthouse test, keep only landing page - Add integration test for full prod build pipeline (gz, purge, image opt)
# Conflicts: # pyi_hashes.json # reflex/app.py # tests/units/test_prerequisites.py
3b6eef0 to
11aae47
Compare
- Run compress-static.js on final static output to pre-compress assets - Skip already-compressed sidecars and always compress HTML entrypoints - Switch socket.io decoder to JSON5.parse for more lenient message parsing - Add json5 as a frontend dependency
Replace StaticFiles with PrecompressedStaticFiles in get_frontend_mount to support content negotiation for compressed and optimized image formats. Add unit tests for the new behavior.
# Conflicts: # packages/reflex-base/src/reflex_base/compiler/templates.py # reflex/testing.py # reflex/utils/exec.py
Add node-version input to setup_build_env action instead of hardcoding v22. Include actual timeout duration in Lighthouse CLI failure messages and add unit tests for both preparation and execution timeout paths.
Only ship the Radix color scales actually referenced by Theme components (falling back to the monolithic stylesheet when a color is state-driven), and expose external libraries on window.__reflex via named imports so Rolldown can drop unused exports.
Capture named imports during Component serialization so tags only reachable via evalReactComponent (Component-typed vars, state field defaults) are included in window.__reflex. Without this, dynamic components fail to resolve their imports after the tree-shaking changes. Also splits @radix-ui/themes into its own chunk.
e7f9c2a to
31820ec
Compare
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
All Submissions:
Type of change
Please delete options that are not relevant.
New Feature Submission:
Changes To Core Features:
closes #6295