Ensure compiled range media queries are correctly parenthesised#1114
Merged
devongovett merged 1 commit intoparcel-bundler:masterfrom Jan 3, 2026
Merged
Ensure compiled range media queries are correctly parenthesised#1114devongovett merged 1 commit intoparcel-bundler:masterfrom
devongovett merged 1 commit intoparcel-bundler:masterfrom
Conversation
Fixes parcel-bundler#1105 In the test case added, the range query `(width < 256px)` gets compiled into `not (min-width: 256px)`. If we combine this with another query, for example `(width < 256px) or (hover: none)`, the compiled query should parenthesise the compiled range query. That is, the output should be `(not (min-width: 256px)) or (hover: none)`. Instead, we see an output of `not (min-width: 256px) or (hover: none)`, which incorrectly negates the entire media query, not just the `min-width`. `QueryFeature::needs_parens` determines if parentheses are need with the following logic: ```rust match self { QueryFeature::Interval { .. } => parent_operator != Some(Operator::And), QueryFeature::Range { operator, .. } => { matches!( operator, MediaFeatureComparison::GreaterThan | MediaFeatureComparison::LessThan ) } _ => false, } } ``` This is correct ***if*** both interval and range queries are being compiled, since parentheses are required when the range is replaced with a negation. However, above this return was the block: ```rust if !should_compile!(targets, MediaIntervalSyntax) { return false; } ``` This causes the check to be skipped when `Feature::MediaIntervalSyntax` isn't enabled. This leaves an edge case: if `Feature::MediaRangeSyntax` is enabled, without `Feature::MediaIntervalSyntax`, the correct parentheses check isn't carried out. One would think that this is an unreasonable edge case, since targeting a browser without range syntax support would also be a browser without interval syntax support. However, in `turbopack-css`, the `MediaRangeSyntax` feature is always included, without adding `MediaIntervalSyntax` (see vercel/next.js@a95f861#diff-389b0ea768c0dbbca95b4aff5d1237ecddb33677521e3e2eb1f717c43c0d4658). In Next 16, the default targets were updated (see vercel/next.js#84401), which caused `MediaIntervalSyntax` to no longer be included by default, hence opening up this edge case. I beleive that, unfortunately, the reason for Next adding `MediaIntervalSyntax` still holds, and applies to intervals too. I'll make a PR there to add `MediaIntervalSyntax` which will avoid this edge case, but it'd be nice to fix it here, too :) This moves the `MediaIntervalSyntax` check into the `Interval` case in the switch, and adds a separate `MediaRangeSyntax` check in the `Range` case.
JOT85
added a commit
to JOT85/next.js
that referenced
this pull request
Dec 20, 2025
MediaRangeSyntax was added in a95f861. From my testing, the reason for adding MediaRangeSyntax still holds, and also applies to MediaIntervalSyntax, which I think is enough merit alone to add this. I stumbled across it separately though, via parcel-bundler/lightningcss#1114 This bug in LightningCSS causes range syntax to be improperly compiled when MediaIntervalSyntax isn't enabled. Since Next 16 updated the default browserlist to browsers new enough to not need MediaIntervalSyntax, this was disabled by default, surfacing that bug.
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
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 #1105
In the test case added, the range query
(width < 256px)gets compiled intonot (min-width: 256px). If we combine this with another query, for example(width < 256px) or (hover: none), the compiled query should parenthesise the compiled range query. That is, the output should be(not (min-width: 256px)) or (hover: none). Instead, we see an output ofnot (min-width: 256px) or (hover: none), which incorrectly negates the entire media query, not just themin-width.QueryFeature::needs_parensdetermines if parentheses are need with the following logic:This is correct if both interval and range queries are being compiled, since parentheses are required when the range is replaced with a negation.
However, above this return was the block:
This causes the check to be skipped when
Feature::MediaIntervalSyntaxisn't enabled.This leaves an edge case: if
Feature::MediaRangeSyntaxis enabled, withoutFeature::MediaIntervalSyntax, the correct parentheses check isn't carried out.One would think that this is an unreasonable edge case, since targeting a browser without range syntax support would also be a browser without interval syntax support. However, in
turbopack-css, theMediaRangeSyntaxfeature is always included, without addingMediaIntervalSyntax(see vercel/next.js@a95f861). In Next 16, the default targets were updated (see vercel/next.js#84401), which causedMediaIntervalSyntaxto no longer be included by default, hence opening up this edge case.I beleive that, unfortunately, the reason for Next adding
MediaIntervalSyntaxstill holds, and applies to intervals too. I'll make a PR there to addMediaIntervalSyntaxwhich will avoid this edge case, but it'd be nice to fix it here, too :)This moves the
MediaIntervalSyntaxcheck into theIntervalcase in the switch, and adds a separateMediaRangeSyntaxcheck in theRangecase.