fix(build): pin cssTarget so esbuild preserves max-width media-query syntax#1310
Conversation
…y syntax Vite's default `build.cssTarget` follows `build.target` (modern evergreens), which lets esbuild's CSS minifier rewrite `@media (max-width: 768px)` to the Media Queries Level 4 range syntax `@media (width <= 768px)`. Both forms are semantically equivalent in modern browsers, but the rewrite is observable to user code that inspects `cssText` on `CSSMediaRule`s and breaks tools that pattern-match the raw query string. Next.js's CSS pipeline (webpack + cssnano, or lightningcss only when opted in via `experimental.useLightningcss`) preserves the original syntax by default, so user code carried over from Next.js can break when migrating to vinext. Pin `build.cssTarget` to ["chrome87", "edge88", "firefox78", "safari15"], which predate Safari 16.4 — the first browser to support MQ Level 4 range syntax. esbuild checks the configured target and skips lowering when any target is too old. Only literal media-query syntax is affected; matching behavior at runtime is unchanged. Mirrors the Next.js fixture test/e2e/app-dir/css-media-query/css-media-query.test.ts which asserts `cssText` preserves `max-width: 768px`. Tests: - New tests/css-media-query-target.test.ts builds a minimal fixture and asserts the produced CSS asset contains `max-width: 768px` and does not contain `width<=768px`. Fails without this change; passes with it. - All existing CSS / SCSS / build-optimization / nextjs-compat app-css tests continue to pass.
commit: |
|
|
/bigbonk review |
There was a problem hiding this comment.
Review: CSS target pinning for media-query syntax preservation
The fix correctly identifies a real parity gap — Next.js preserves max-width syntax and vinext was rewriting it to range syntax. The test is well-structured and the PR description is excellent. However, the chosen cssTarget is significantly more conservative than necessary, which causes collateral downleveling of unrelated CSS features.
Issue: Target is too conservative
The stated goal is to suppress the Media Queries Level 4 range syntax rewrite. The only browser version that matters for this is Safari 16.4 (the first to support range syntax). By setting safari15, esbuild sees the target as too old for range syntax and skips the rewrite — correct.
But the other targets (chrome87, edge88, firefox78) are far older than necessary and cause esbuild to unnecessarily downlevel several unrelated CSS features:
| Feature | Minimum required | Your target | Downleveled? |
|---|---|---|---|
:is() pseudo-class |
Chrome 88 | chrome87 | Yes — off by 1 version |
| CSS Nesting | Chrome 120, Safari 17.2 | chrome87, safari15 | Yes |
hwb() colors |
Chrome 101, Safari 15 | chrome87 | Yes |
lab(), oklch(), color() |
Chrome 111, Firefox 113 | chrome87, firefox78 | Yes |
| Gradient interpolation | Chrome 111, Firefox 137 | chrome87, firefox78 | Yes |
inset shorthand |
Chrome 87 | chrome87 | Borderline |
CSS Nesting and :is() are the most impactful. Users migrating from Next.js who write modern CSS nesting will get unnecessarily flattened output, and :is() selectors will be rewritten even though Chrome 88+ (and all other modern browsers) support them natively.
Suggested fix: Raise the non-Safari targets to match Vite 8's defaults. Only Safari needs to be pinned low:
cssTarget: ["chrome111", "edge111", "firefox114", "safari15"],This still prevents the range syntax rewrite (Safari 15 < 16.4), but allows all other modern CSS features through. The comment already correctly explains the Safari 16.4 threshold — the other targets just need to not drag the floor down further.
Alternatively, if esbuild/Vite gains a supported map for individual CSS features in the future, that would be even more surgical, but the raised-target approach works today.
Test: solid
The test is well-constructed:
- Creates a minimal fixture with the exact CSS pattern from the Next.js test
- Builds through vinext's production pipeline
- Asserts both the positive (preserves
max-width) and negative (no range syntax) - Properly cleans up temp dirs
- Links to the Next.js source test
One minor observation: the test uses build() directly (not createBuilder().buildApp()), which means it only builds the client bundle. This is fine for validating CSS target behavior since cssTarget is a top-level config that applies to all environments, but worth noting that the AGENTS.md says production builds require createBuilder.
Comment quality
The inline comment is thorough and well-referenced. The PR description is one of the better ones I've seen — it explains what, why, alternatives considered, and links to the specific Next.js test that surfaced the issue.
| // Mirrors the Next.js fixture | ||
| // test/e2e/app-dir/css-media-query/css-media-query.test.ts which | ||
| // asserts `cssText` preserves `max-width: 768px`. | ||
| cssTarget: ["chrome87", "edge88", "firefox78", "safari15"], |
There was a problem hiding this comment.
| cssTarget: ["chrome87", "edge88", "firefox78", "safari15"], | |
| cssTarget: ["chrome111", "edge111", "firefox114", "safari15"], |
Only safari15 needs to be pinned low (Safari 16.4 was the first to support MQ range syntax). The Chrome/Edge/Firefox targets can stay at Vite 8's defaults (chrome111, edge111, firefox114) to avoid unnecessarily downleveling :is() (needs Chrome 88), CSS nesting (Chrome 120), hwb()/lab()/oklch() color functions, and other modern CSS features.
The current chrome87 is off-by-one from :is() support (Chrome 88) and ~33 versions behind current Baseline. firefox78 is ESR-era and forces downleveling of gradient double positions (Firefox 83+) and color functions (Firefox 113+).
|
Review posted successfully. Here's a summary of what I found and commented: Review posted on PR #1310 with "Request Changes": The core fix is correct -- vinext was rewriting The one actionable issue: the
Suggested fix: |
Address review feedback on #1310. Only Safari needs to be pinned to a version that predates MQ Level 4 range syntax (Safari 16.4) so esbuild skips the @media (max-width: …) → @media (width <= …) rewrite. The other targets were pinned far older than necessary, which caused esbuild to downlevel unrelated modern CSS features (`:is()`, CSS Nesting, `lab()`, `oklch()`, gradient interpolation, etc.). Bumps chrome/edge/firefox targets to ~2023 baselines while keeping safari15 to retain the MQ rewrite fix.
|
/bigbonk review |
There was a problem hiding this comment.
Previous review feedback has been addressed. The second commit raises Chrome/Edge/Firefox targets to ~2023 baselines (chrome111, edge111, firefox114) while keeping safari15 pinned low to prevent the MQ range syntax rewrite. The comment is thorough and accurately documents which CSS features are preserved vs still lowered at these targets.
All CI checks pass (Check, Vitest, all 13 E2E projects, deploys, smoke tests). The test is well-structured — builds through the real vinext plugin pipeline, asserts both positive (max-width preserved) and negative (no range syntax) conditions, and cleans up temp dirs properly.
Ship it.
|
Approved PR #1310. The previous review's feedback (raising Chrome/Edge/Firefox targets to avoid collateral CSS downleveling) was addressed in the second commit. The fix is correct, CI is fully green, and the test properly validates the behavior. Ready to merge. |
|
/bigbonk how does this compare against nextjs for parity? |
There was a problem hiding this comment.
Next.js Parity Assessment
Verified this against how Next.js actually handles CSS media query syntax. The fix is correct and the approach is sound. Here's the full breakdown:
How Next.js preserves max-width syntax
Next.js never rewrites max-width to range syntax because none of its CSS tools perform old-to-new upgrades:
| Pipeline | Tool | Behavior |
|---|---|---|
| Webpack (legacy) | cssnano-simple via PostCSS | Minifier only — compresses whitespace/values, does NOT perform syntax upgrades |
| Webpack + postcss-preset-env | stage 3 preset | Transpiles new to old — would convert (width <= 768px) DOWN to (max-width: 768px), never the reverse |
| Turbopack (current default) | lightningcss | Explicitly includes Features::MediaRangeSyntax in the include flags, meaning range syntax is always compiled DOWN to traditional form |
| Webpack + lightningcss opt-in | lightningcss loader | Same — only lowers syntax, never upgrades |
This preservation is incidental, not an explicit design choice. The Next.js test (test/e2e/app-dir/css-media-query/css-media-query.test.ts) exists to guard against regressions as tooling evolves, and uses a completely empty next.config.js — no special config needed.
Why vinext needed an explicit fix
esbuild is unique among CSS tools in that it treats range syntax as a minification opportunity — it will rewrite @media (max-width: 768px) to @media (width<=768px) when targets support it, because the latter is shorter. cssnano, postcss-preset-env, and lightningcss all only lower syntax (new to old), never upgrade it.
Pinning safari15 in cssTarget is the right lever — it tells esbuild the target doesn't support range syntax (Safari 16.4 was the first), so it skips the rewrite.
Browser target comparison
Next.js's modern browserslist target (from packages/next/src/shared/lib/modern-browserslist-target.js):
chrome 111, edge 111, firefox 111, safari 16.4
vinext cssTarget in this PR:
chrome111, edge111, firefox114, safari15
The only meaningful difference is safari15 vs safari 16.4, which is the exact point of the fix. Chrome/Edge/Firefox are in the same ballpark. This is a clean approach — it pins the CSS target without affecting JS transpilation.
Remaining parity considerations (not blocking)
The broader CSS pipeline differs between Next.js and vinext in ways worth tracking but not relevant to this PR:
- No postcss-preset-env — vinext doesn't run
postcss-flexbugs-fixesor autoprefixing via PostCSS. esbuild handles some autoprefixing but not all the same cases. - No
colormin: false— Next.js's cssnano-simple explicitly disables color minification. esbuild may rewrite color values differently in minified output. - Reverse direction — in Turbopack, if a user writes
(width <= 768px), lightningcss compiles it DOWN to(max-width: 768px). In vinext, esbuild avoids range syntax because the target is old, which has the same practical effect but via a different mechanism.
None of these affect this PR, but they're worth tracking as broader CSS parity gaps.
Verdict
The fix correctly addresses the parity gap. The test mirrors the Next.js test assertions. The cssTarget values are well-chosen after the second commit raised Chrome/Edge/Firefox to avoid collateral downleveling. Ship it.
|
Review posted on PR #1310. Here's the summary: The fix achieves correct Next.js parity. The key insight is that Next.js's preservation of The I flagged three non-blocking broader CSS parity gaps worth tracking (no postcss-preset-env, no colormin disable, different mechanism for range syntax suppression) but none affect this PR. Approved. |
What
Pin
build.cssTargetto["chrome87", "edge88", "firefox78", "safari15"]in vinext's top-level build config.Why
Vite's default
build.cssTargetfollowsbuild.target(modern evergreens), which lets esbuild's CSS minifier rewrite@media (max-width: 768px)to the Media Queries Level 4 range syntax@media (width <= 768px).The two forms are semantically equivalent in modern browsers, but the rewrite is observable to:
cssTextonCSSMediaRuleobjectsNext.js's CSS pipeline (webpack + cssnano, or lightningcss only when opted in via
experimental.useLightningcss) preserves the original syntax by default. Pages migrated to vinext that relied on this default were silently rewritten.Pinning
cssTargetto a baseline that predates Safari 16.4 — the first browser to support MQ Level 4 range syntax — causes esbuild to skip the lowering. Only literal media-query syntax is affected; runtime matching behavior is unchanged.Failure this fixes
Found via the Next.js Deploy Suite. The Next.js fixture
test/e2e/app-dir/css-media-query/css-media-query.test.ts("should preserve max-width media query syntax instead of transpiling to range syntax") asserts:Today vinext outputs
@media screen and (width<=768px)and the assertion fails withReceived: undefined(the test's.find(rule => /@media/.test(rule.cssText) && /max-width/.test(rule.cssText))returns undefined because the substring no longer appears).Tests
tests/css-media-query-target.test.tsbuilds a minimal fixture with vinext's production build pipeline, then reads the produced CSS asset and assertsmax-width: 768pxis preserved andwidth<=768pxdoes NOT appear. Verified this test fails without the cssTarget change and passes with it.Considered alternatives
build.cssMinify: 'lightningcss'— would also preserve the syntax (lightningcss is more conservative by default), but vinext already stubs lightningcss out for Workers builds indeploy.ts:374. Adopting it as the CSS transformer would conflict with that decision and add a new dependency surface.cssMinify: false) — too aggressive; loses the size win on every other CSS optimisation esbuild does.build.cssTargetis a top-level config that fans out to all environments naturally, so a single top-level pin is the smallest change.