fix: keep the media type first when flattening nested @media queries

[Lfan-ke]

When nested @media blocks are flattened, a media type coming from an inner block currently lands after the outer conditions, e.g.

@media (max-width: 500px) { @media screen { … } }

produced @media (max-width: 500px) and screen { … }, which is invalid CSS - the spec requires the media type to lead the query.

This moves any media type (screen, only screen, print and (color), …) to the front of the flattened query, so the example now yields @media screen and (max-width: 500px). Queries without a media type, and those already in the correct order, are unchanged.

Closes #3694
Closes #3764

Summary by CodeRabbit

• Bug Fixes

  • Fixed nested @media flattening to preserve and correctly reorder media types when they appear inside conditional media queries, including not/only prefixed cases.
  • Improved handling for complex and nested media-query permutations to avoid incorrect or invalid flattened output.

• Tests

  • Added new unit test fixtures and expected CSS coverage for nested typed media under conditional @media blocks, including unknown media-type and variable/feature-condition guard scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

Nested media-query evaluation detects media types, including not and only forms, and places them before other flattened conditions. New LESS and CSS fixtures cover ordering across nested media-query combinations.

Changes

Nested media-query ordering

Layer / File(s)	Summary
Detect and reorder media types packages/less/lib/less/tree/nested-at-rule.js	Adds media-type detection and moves matching fragments ahead of other conditions during nested media-query flattening.
Add nested media regression fixtures packages/test-data/tests-unit/media-nested-type/*	Adds nested media-query inputs and expected CSS covering typed conditions, preserved ordering, queries without media types, unknown media types, and variable-based conditions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• less/less.js#4409: Both changes modify nested media-query handling in nested-at-rule.js.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main behavior change: keeping media types first when flattening nested @media queries.
Linked Issues check	✅ Passed	The change addresses #3694 and #3764 by reordering flattened media queries so media types precede conditions, including typed nested cases.
Out of Scope Changes check	✅ Passed	All code and fixtures support media-query reordering and validation; no unrelated or extraneous changes are evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

Fixes nested media-query flattening by:

• Detecting media-type fragments, including prefixed and unknown media types.
• Moving detected media types before feature conditions.
• Adding fixtures for ordering permutations, feature-only queries, and escaped conditions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

_{Reviews (4): Last reviewed commit: "fix: keep the media type first when flat..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/test-data/tests-unit/media-nested-type/media-nested-type.less (1)

8-12: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a not regression case.

The implementation handles both not and only, but this fixture only exercises only. Add nested @media not screen input and the matching expected CSS; otherwise modifier-specific regressions can pass unnoticed. not negates the entire query, unlike only. (w3.org)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.less`
around lines 8 - 12, Add a nested `@media` not screen regression fixture alongside
the existing `@media` only screen case, and add its matching expected CSS output.
Keep the current only case unchanged so the fixture covers both media-query
modifiers and verifies that not negates the entire query.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/less/lib/less/tree/nested-at-rule.js`:
- Around line 42-45: Update startsWithMediaType to recognize any string
identifier as a media type after optional “not” or “only” modifiers, rather than
restricting matches to MEDIA_TYPES. Exclude the operators “and”, “or”, and “,”
so nested unknown media types are reordered correctly without treating query
operators as media types.

---

Nitpick comments:
In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.less`:
- Around line 8-12: Add a nested `@media` not screen regression fixture alongside
the existing `@media` only screen case, and add its matching expected CSS output.
Keep the current only case unchanged so the fixture covers both media-query
modifiers and verifies that not negates the entire query.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 71e7a9f8-d106-427b-826a-f82ce3e65417

📥 Commits

Reviewing files that changed from the base of the PR and between ebbce6d and 4de62db.

📒 Files selected for processing (3)

• packages/less/lib/less/tree/nested-at-rule.js
• packages/test-data/tests-unit/media-nested-type/media-nested-type.css
• packages/test-data/tests-unit/media-nested-type/media-nested-type.less

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

packages/test-data/tests-unit/media-nested-type/media-nested-type.css (1)

1-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the not modifier in the regression fixture.

The implementation explicitly handles leading not, but this fixture only exercises only and bare/unknown media types. Add a nested not screen input and expected output to protect that path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.css` around
lines 1 - 35, Extend the media nested-type regression fixture with a case using
a leading “not screen” modifier, including its nested rule and expected output.
Keep the existing only, bare, and unknown media-type cases unchanged, and ensure
the new fixture exercises the implementation path handling leading not.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/less/lib/less/tree/nested-at-rule.js`:
- Around line 54-64: Update the media-type detection logic around the fragment
head calculation to inspect the first non-whitespace token of Anonymous values
rather than treating the entire value as a media type. After optional not/only,
require the derived head to be a bare identifier; return false for parenthesized
or otherwise non-identifier conditions, while preserving Keyword and valid
Expression handling.

In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.less`:
- Line 23: Insert a blank line immediately before each of the three SCSS
comments in media-nested-type.less, including the locations around the existing
comments at lines 23, 29, and 35, without changing their ordering or other
styling.

---

Nitpick comments:
In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.css`:
- Around line 1-35: Extend the media nested-type regression fixture with a case
using a leading “not screen” modifier, including its nested rule and expected
output. Keep the existing only, bare, and unknown media-type cases unchanged,
and ensure the new fixture exercises the implementation path handling leading
not.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8db31cd5-d61e-4ea5-be5f-f9ba8fe7f5a1

📥 Commits

Reviewing files that changed from the base of the PR and between 4de62db and 449944a.

📒 Files selected for processing (3)

• packages/less/lib/less/tree/nested-at-rule.js
• packages/test-data/tests-unit/media-nested-type/media-nested-type.css
• packages/test-data/tests-unit/media-nested-type/media-nested-type.less

[Lfan-ke]

The Anonymous case can't occur: a feature condition like (max-width: 500px) parses as a Paren, not an Anonymous, so startsWithMediaType returns false and never reorders it before screen (the existing @media screen { @media (max-width: 500px) } fixture covers this). The classifier is correct as-is. Stylelint blank-line nit fixed.

[matthew-dean]

@Lfan-ke

The Anonymous case can't occur: a feature condition like (max-width: 500px) parses as a Paren, not an Anonymous, so startsWithMediaType

I think it can occur, if someone does ~"(max-width: 500px)". Not that someone should, but I think CodeRabbit is saying that this then would be sorted incorrectly.

[Lfan-ke]

You're right - it read the whole Anonymous value as the head, so ~"(max-width: 500px)" matched. Fixed: startsWithMediaType now inspects only the first token, and a fragment leading with ( is a feature condition, never a media type. Added an escaped-string fixture; suite passes.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/less/lib/less/tree/nested-at-rule.js`:
- Around line 52-71: Update startsWithMediaType so Keyword and Anonymous
fragments also skip leading not/only modifiers before evaluating the media type.
Inspect the first non-modifier token and preserve the existing condition
handling for following parenthesized expressions, so inputs like “not (color)”
are not classified as media types.

In `@packages/test-data/tests-unit/media-nested-type/media-nested-type.less`:
- Line 46: Update the LESS variable declaration `@fc` in the media-nested-type
fixture so the active scss/at-rule-no-unknown lint rule no longer rejects it,
using a narrowly scoped LESS-specific override or suppression for this line
only.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ea5efec-824b-4708-a3ff-0b8fc25aa791

📥 Commits

Reviewing files that changed from the base of the PR and between 6615258 and 412f135.

📒 Files selected for processing (3)

• packages/less/lib/less/tree/nested-at-rule.js
• packages/test-data/tests-unit/media-nested-type/media-nested-type.css
• packages/test-data/tests-unit/media-nested-type/media-nested-type.less

[Lfan-ke]

Both are non-issues after checking against actual behavior:

1. not/only fragments aren't misordered. @media (min-width: 1px) { @media not (color) {} } compiles to @media (min-width: 1px) and not (color) on this branch, byte-identical to 4.8.0 - the not (color) fragment stays in place, it isn't hoisted ahead of anything. Normal-parsed not (color) never even reaches the string branch (it's an Expression whose non-modifier head is (color)), and the escaped ~"not (color)" case reorders the same as before. Only genuine media types (screen) get moved to the front, which is the whole point of the fix. There's no input that reproduces the described reordering.

2. There's no stylelint in this repo - CI is test:node plus eslint ... .js,.ts, and .less fixtures aren't linted at all, so scss/at-rule-no-unknown doesn't apply. @fc: ~"..." is fine.