Skip to content

fix(css): read a block's at-rule name with the shared helper when ordering declarations - #22590

Merged
alexander-akait merged 1 commit into
mainfrom
fix/css-at-rule-declaration-order
Oct 10, 2026
Merged

alexander-akait merged 1 commit into
mainfrom
fix/css-at-rule-declaration-order

Conversation

@alexander-akait

@alexander-akait alexander-akait commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Summary

#22569 removed path.atKeyword while #22513 was in review, so on main minifying any at-rule that holds declarations throws. The declaration order now reads the name with _atKeywordOf, as the rest of the printer does.

// before (main)
minify("@font-face{font-family:a;src:url(a.woff)}") // TypeError: path.atKeyword is not a function
minify("a{@media (min-width:1px){display:block;color:red}}") // TypeError
// after
"@font-face{font-family:a;src:url(a.woff)}"
"a{@media (width>=1px){color:red;display:block}}"

No changeset: the order change has not been released yet.

What kind of change does this PR introduce?

fix

Did you add tests for your changes?

Yes, two unit cases (@font-face and a nested @media); both throw on main.

Does this PR introduce a breaking change?

No

If relevant, what needs to be documented once your changes are merged or what have you already documented?

n/a

Use of AI

Written with Claude Code, which found the crash by running yarn benchmark:css-tools on main, wrote the fix and tests, and ran unit tests, CSS/HTML config cases and yarn lint; reviewed by the author.

🤖 Generated with Claude Code


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Corrected declaration ordering for styles nested within group rules inside style rules.
    • Preserved the original order of descriptors in @font-face rules.

…ering declarations

#22569 removed path.atKeyword while the declaration order change was in
review, so minifying any at-rule holding declarations, such as @font-face,
threw. Read the name with _atKeywordOf, as the rest of the printer does.
@changeset-bot

changeset-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: a28dbc7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions Bot added the area: css CSS support (lib/css) label Oct 9, 2026
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: webpack/webpack/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7e5c0e0a-db63-41e9-8548-6befbdc59d43

📥 Commits

Reviewing files that changed from the base of the PR and between 8953192 and a28dbc7.


📒 Files selected for processing (2)
  • lib/css/syntax-parser.js
  • test/unitCases/CssSyntax.unittest.js

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.



📝 Walkthrough

Walkthrough

The nested-group at-rule check now reads the owner’s name with path.name(owner). Tests cover declaration ordering in nested @media rules and order preservation in @font-face.

Changes

CSS declaration ordering

Layer / File(s) Summary
Nested group declaration ordering
lib/css/syntax-parser.js, test/unitCases/CssSyntax.unittest.js
The nested-group at-rule check uses path.name(owner). Tests verify that declarations in a nested style rule are reordered and that @font-face descriptors retain their order.

Suggested labels: area: css

Suggested reviewers: aryanraj45


Priority: ➖ Normal

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a28db

The declaration-order fix matches the parser’s at-rule classification, with tests covering nested media ordering and font-face descriptor preservation. No concrete merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title uses the required Conventional Commit format, fix(css): subject, and accurately describes the change to at-rule name lookup during declaration ordering.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

This PR is packaged and the instant preview is available (a7e5a84).

Install it locally:

  • npm
npm i -D webpack@https://pkg.pr.new/webpack@a7e5a84
  • yarn
yarn add -D webpack@https://pkg.pr.new/webpack@a7e5a84
  • pnpm
pnpm add -D webpack@https://pkg.pr.new/webpack@a7e5a84

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Types Coverage

Coverage after merging fix/css-at-rule-declaration-order into main will be
97.29%
Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
bin
   webpack.js98.82%100%100%98.82%103
declarations
   WebpackOptions.ts99.94%100%100%99.94%4188
examples
   build-common.js100%100%100%100%
   buildAll.js100%100%100%100%
   examples.js100%100%100%100%
   template-common.js98.21%100%100%98.21%72
examples/custom-javascript-parser
   test.filter.js100%100%100%100%
examples/custom-javascript-parser/internals
   acorn-parse.js100%100%100%100%
   meriyah-parse.js100%100%100%100%
   oxc-parse.js100%100%100%100%
examples/markdown
   webpack.config.mjs100%100%100%100%
examples/module-federation
   test.filter.js100%100%100%100%
examples/reexport-components
   test.filter.js100%100%100%100%
examples/ssr
   test.filter.js100%100%100%100%
examples/typescript
   test.filter.js100%100%100%100%
examples/typescript-non-erasable
   test.filter.js50%100%100%50%5
examples/virtual-modules
   test.filter.js100%100%100%100%
examples/wasm-bindgen-esm
   test.filter.js100%100%100%100%
examples/wasm-complex
   test.filter.js100%100%100%100%
examples/wasm-emscripten
   test.filter.js100%100%100%100%
examples/wasm-simple
   test.filter.js100%100%100%100%
examples/wasm-simple-source-phase
   test.filter.js100%100%100%100%
lib
   Chunk.js100%100%100%100%
   Compilation.js98.60%100%100%98.60%1757, 2078, 2085, 2093, 2115, 2118, 23, 3075, 3578–3579, 3611, 4200, 4204, 4331, 4352, 4477, 4514, 4521, 4574–4575, 4579, 4584, 4600–4601, 4615–4616, 4621–4622, 5145, 5171, 6001, 6018, 6036, 6052, 6067, 6092, 6095, 6463, 6466–6467, 6472, 6487, 6570, 6594, 6708, 837–838
   Compiler.js98.81%100%100%98.81%1116, 1153, 1156, 1336–1337, 1345, 848, 934, 975
   Dependency.js100%100%100%100%
   DependencyTemplate.js100%100%100%100%
   EntryPlugin.js100%100%100%100%
   Entrypoint.js100%100%100%100%
   ExternalsPlugin.js100%100%100%100%
   FileSystemInfo.js100%100%100%100%
   InitFragment.js100%100%100%100%
   LibraryTemplatePlugin.js100%100%100%100%
   ModuleFilenameHelpers.js100%100%100%100%
   ModuleNotFoundError.js100%100%100%100%
   MultiCompiler.js98.88%100%100%98.88%460, 532, 719, 784
   NormalModule.js100%100%100%100%
   NullFactory.js100%100%100%100%
   ProgressPlugin.js100%100%100%100%
   RawModule.js100%100%100%100%
   RequestShortener.js100%100%100%100%
   SingleEntryPlugin.js100%100%100%100%
   WebpackError.js100%100%100%100%
   index.js99.78%100%100%99.78%206
   webpack.js97.84%100%100%97.84%278, 300, 302
lib/asset
   AssetBytesGenerator.js100%100%100%100%
   AssetBytesParser.js100%100%100%100%
   AssetGenerator.js100%100%100%100%
   AssetModule.js100%100%100%100%
   AssetModulesPlugin.js98.15%100%100%98.15%338, 362, 365, 495, 57, 62
   AssetParser.js100%100%100%100%
   AssetSourceGenerator.js100%100%100%100%
   AssetSourceParser.js100%100%100%100%
   RawDataUrlModule.js100%100%100%100%
   WebManifestGenerator.js100%100%100%100%
   WebManifestParser.js100%100%100%100%
lib/async-modules
   AsyncModuleHelpers.js100%100%100%100%
   AwaitDependenciesInitFragment.js100%100%100%100%
   InferAsyncModulesPlugin.js100%100%100%100%
   isGeneratorLowered.js100%100%100%100%
lib/bun
   BunTargetPlugin.js100%100%100%100%
lib/cache
   AddBuildDependenciesPlugin.js100%100%100%100%
   AddManagedPathsPlugin.js100%100%100%100%
   Cache.js96.49%100%100%96.49%10, 104
   CacheFacade.js100%100%100%100%
   IdleFileCachePlugin.js97.92%100%100%97.92%75, 87, 95
   MemoryCachePlugin.js92%100%100%92%33, 42
   MemoryWithGcCachePlugin.js93.42%100%100%93.42%106, 120–121, 130, 88
   PackFileCacheStrategy.js96.54%100%100%96.54%1304, 1404, 1408, 1470, 1716, 1800, 1823, 1853, 682, 701, 711–713, 715, 731–732, 737, 740, 742, 747, 752, 777, 783, 817, 823, 829, 834, 845, 854, 859–860, 862, 879, 885–886, 888
   ResolverCachePlugin.js100%100%100%100%
   getLazyHashedEtag.js100%100%100%100%
   mergeEtags.js100%100%100%100%
lib/config
   OptionsApply.js100%100%100%100%
   PlatformPlugin.js100%100%100%100%
   WebpackOptionsApply.js100%100%100%100%
   WebpackOptionsDefaulter.js100%100%100%100%
   browserslistTargetHandler.js100%100%100%100%
   cli.js98.61%100%100%98.61%116, 546, 578, 628, 9
   defaults.js99.44%100%100%99.44%1624–1626, 1634, 283, 286, 291, 295
   defineConfig.js100%100%100%100%
   getClaimedAssetTypes.js100%100%100%100%
   normalization.js99.12%100%100%99.12%279–280, 346, 361
   target.js100%100%100%100%
   validateSchema.js94.67%100%100%94.67%100, 87, 89, 98
lib/container
   ContainerEntryDependency.js100%100%100%100%
   ContainerEntryModule.js100%100%100%100%
   ContainerEntryModuleFactory.js100%100%100%100%
   ContainerExposedDependency.js100%100%100%100%
   ContainerPlugin.js100%100%100%100%
   ContainerReferencePlugin.js100%100%100%100%
   FallbackDependency.js100%100%100%100%
   FallbackItemDependency.js100%100%100%100%
   FallbackModule.js100%100%100%100%
   FallbackModuleFactory.js100%100%100%100%
   HoistContainerReferencesPlugin.js100%100%100%100%
   ModuleFederationPlugin.js100%100%100%100%
   RemoteModule.js100%100%100%100%
   RemoteRuntimeModule.js100%100%100%100%
   RemoteToExternalDependency.js100%100%100%100%
   declaredRemotes.js100%100%100%100%
   moduleFederationHooks.js100%100%100%100%
   options.js100%100%100%100%
lib/context
   ContextExclusionPlugin.js100%100%100%100%
   ContextModule.js99.88%100%100%99.88%1494
   ContextModuleFactory.js95.66%100%100%95.66%230, 257, 276, 280–281, 289, 465, 486, 491, 532, 541, 543, 547, 550, 556–557
   ContextReplacementPlugin.js100%100%100%100%
lib/css
   CssGenerator.js99.47%100%100%99.47%1085, 136, 842
   CssInjectStyleRuntimeModule.js100%100%100%100%
   CssLoadingRuntimeModule.js100%100%100%100%
   CssModule.js83.16%100%100%83.16%197, 202–210, 212–217
   CssModulesPlugin.js99.86%100%100%99.86%886
   CssParser.js99.48%100%100%99.48%1048, 1079, 24, 2410, 3057, 3060, 3066, 3107, 3110, 3149, 3152, 3489, 5048, 5051, 932
   CssServerStylesRuntimeModule.js100%100%100%100%
   conventions.js98.65%100%100%98.65%93
   cssMinify.js97.50%100%100%97.50%162, 171
   data.js99.84%100%100%99.84%229, 5601
   syntax-parser.js99.65%100%100%99.65%10098–10099, 10633–10634, 10671, 10866, 10912, 13132–13135, 13235–13236, 13333–13334, 13979, 14276, 14278, 14768, 15134, 17241, 2467, 3166, 3168, 3173, 3192, 3937, 3943, 7384, 7422, 7429
   syntax-printer.js99.31%100%100%99.31%2532–2533, 2535–2536, 2538–2539, 2544, 2811–2814, 2907, 3580, 367, 4000, 4109, 4797, 5098, 5884, 735
   syntax.js100%100%100%100%
   topologicalSort.js100%100%100%100%
lib/debug
   ProfilingPlugin.js96.22%100%100%96.22%169, 425, 554, 561, 570, 574, 583, 588, 594, 599, 92
lib/define
   DefinePlugin.js98.85%100%100%98.85%1495, 268–269, 285, 304, 378, 936, 938, 943, 967
   DotenvPlugin.js98.07%100%100%98.07%391, 439, 452–453
   EnvironmentPlugin.js97.14%100%100%97.14%49
lib/deno
   DenoTargetPlugin.js100%100%100%100%
lib/dependencies
   ConstDependency.js100%100%100%100%
   ContextElementDependency.js100%100%100%100%
   NullDependency.js100%100%100%100%
lib/dependencies/amd
   AMDDefineDependency.js100%100%100%100%
   AMDDefineDependencyParserPlugin.js100%100%100%100%
   AMDPlugin.js99.31%100%100%99.31%54
   AMDRequireArrayDependency.js100%100%100%100%
   AMDRequireContextDependency.js100%100%100%100%
   AMDRequireDependenciesBlock.js100%100%100%100%
   AMDRequir

…truncated

@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.19%. Comparing base (a3232d2) to head (a28dbc7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #22590      +/-   ##
==========================================
+ Coverage   95.54%   96.19%   +0.64%     
==========================================
  Files         774      774              
  Lines      128230   128374     +144     
  Branches    42484    42525      +41     
==========================================
+ Hits       122514   123485     +971     
+ Misses       5716     4889     -827     
Flag Coverage Δ
css-parsing 25.12% <0.00%> (-0.01%) ⬇️
html5lib 27.01% <ø> (-0.01%) ⬇️
integration 82.43% <100.00%> (+0.05%) ⬆️
minify-corpora 79.44% <ø> (?)
syntax-equivalence 79.38% <100.00%> (+0.06%) ⬆️
test262 56.73% <ø> (?)
test262-parser 84.55% <ø> (?)
unit 75.23% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Generated code size

Comparing base (a3232d2) to head (a28dbc7). Merging this pull request changes the size of 34 asset(s) whose tests it leaves untouched and moves 1 asset(s) whose tests it edits too.

Warning

The baseline is a3232d2, not 8953192 — the main commit this run was merged with. Everything that landed on main in between is counted below and is not this pull request's.

What moved Tests Assets Gzip Raw Test edit
Changed, test untouched 17 34 🔴 ↑ +70 B — —
Changed, test edited 1 1 🔴 ↑ +36 B 🔴 ↑ +109 B 🔴 ↑ +118 B
New 0 0 — — —
Deleted 0 0 — — —
Unchanged — 8415 — — —

Read Changed, test untouched first: the test is byte-identical on both sides, so webpack generated the difference. Changed, test edited moved partly because the test did — Test edit is how many bytes the test gained, and a bundle that grew by less is not a regression. New and Deleted are whole files, not deltas — a new asset was created, not grown, so it reads gray (⚪ added) — and adding tests cannot bury a real change. Gzip decides, because that is what users download; raw is the tiebreak, and brotli and zstd are per asset below.

34 asset(s) changed size, test untouched, biggest 20 by raw or gzip change
Asset Before After Change Gzip (9) Brotli (11) Zstd (19)
🔴 ↑ css/css-modules web-production-global/244.bundle5.css 27.36 KiB 27.36 KiB 0 B (—) +28 B (+0.38%) +14 B (+0.21%) +19 B (+0.27%)
🔴 ↑ css/css-modules web-production/652.bundle1.css 24.94 KiB 24.94 KiB 0 B (—) +15 B (+0.18%) -1 B (-0.01%) +3 B (+0.04%)
🔴 ↑ css/large 1/382.bundle1.css 1.86 MiB 1.86 MiB 0 B (—) +10 B (+0.01%) -187 B (-0.41%) +14 B (+0.02%)
🔴 ↑ css/large 1/961.bundle1.css 1.27 MiB 1.27 MiB 0 B (—) +9 B (+0.00%) +126 B (+0.10%) +247 B (+0.18%)
🔴 ↑ css/minimize-corpus minimize-shorthands.css 11.23 KiB 11.23 KiB 0 B (—) +5 B (+0.13%) +4 B (+0.12%) +9 B (+0.23%)
🔴 ↑ css/minimize-corpus minimize-values.css 22.93 KiB 22.93 KiB 0 B (—) +5 B (+0.06%) +6 B (+0.09%) -2 B (-0.03%)
🟢 ↓ css/minimize-embedded-data-url bundle0.css 479 B 479 B 0 B (—) -4 B (-1.37%) -32 B (-12.50%) +1 B (+0.36%)
🟢 ↓ css/case-insensitive 1/bundle1.css 390 B 390 B 0 B (—) -3 B (-1.01%) -3 B (-1.26%) -5 B (-1.71%)
🔴 ↑ css/minify-chrome-120 minify-legacy-box-prefixes.css 359 B 359 B 0 B (—) +3 B (+1.67%) +1 B (+0.80%) —
🟢 ↓ css/mini-css-extract-plugin shared-import_js.chunk.css 62 B 62 B 0 B (—) -2 B (-2.67%) -3 B (-6.38%) -2 B (-3.03%)
🔴 ↑ css/minimize-corpus minimize-declined.css 6.94 KiB 6.94 KiB 0 B (—) +2 B (+0.07%) — +1 B (+0.04%)
🔴 ↑ css/minimize-corpus minimize-shorthand-folds.css 1.10 KiB 1.10 KiB 0 B (—) +2 B (+0.37%) +1 B (+0.22%) +2 B (+0.37%)
🔴 ↑ css/minimize-corpus minimize-nesting.css 3.24 KiB 3.24 KiB 0 B (—) +1 B (+0.07%) -2 B (-0.17%) -1 B (-0.07%)
🔴 ↑ css/minimize-corpus minimize-value-validity.css 2.43 KiB 2.43 KiB 0 B (—) +1 B (+0.09%) +1 B (+0.11%) +2 B (+0.18%)
🟢 ↓ css/minimize-vendor-prefixes bundle0.css 3.04 KiB 3.04 KiB 0 B (—) -1 B (-0.09%) -1 B (-0.11%) -4 B (-0.34%)
🟢 ↓ css/minimize-vendor-prefixes-engine-switch bundle0.css 452 B 452 B 0 B (—) -1 B (-0.37%) +2 B (+1.01%) -1 B (-0.37%)
🔴 ↑ css/minimize-vendor-prefixes-flexbox-2009 bundle0.css 1.08 KiB 1.08 KiB 0 B (—) +1 B (+0.22%) -13 B (-3.67%) +2 B (+0.44%)
🔴 ↑ html/link bundle0.css 98 B 98 B 0 B (—) +1 B (+1.18%) +3 B (+5.56%) +3 B (+4.11%)
🟢 ↓ html/minimize-transforms main.js 6.62 KiB 6.62 KiB 0 B (—) -1 B (-0.03%) +3 B (+0.12%) -2 B (-0.07%)
🟢 ↓ html/minimize-transforms page.html 5.98 KiB 5.98 KiB 0 B (—) -1 B (-0.04%) +3 B (+0.14%) -3 B (-0.11%)
… 14 more, see the uploaded report
1 asset(s) changed size, test edited
Asset Before After Change Gzip (9) Brotli (11) Zstd (19) Test edit (per test)
🔴 ↑ css/minimize-transforms-off bundle0.css 1.34 KiB 1.45 KiB +109 B (+7.92%) +36 B (+4.95%) +22 B (+3.85%) +36 B (+4.87%) +118 B

No runtime gained or lost a runtime module.

Built test/configCases with the defaults a user gets: 2207 test(s), 8450 asset(s), 99 emitted nothing.

@codspeed

codspeed Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 108 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 34 benchmarks have no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

✅ 406 untouched benchmarks
⏩ 6 skipped benchmarks1


Comparing fix/css-at-rule-declaration-order (a28dbc7) with main (8953192)

Open in CodSpeed

Footnotes

  1. 6 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@alexander-akait
alexander-akait merged commit a7e5a84 into main Oct 10, 2026
69 of 70 checks passed
@alexander-akait
alexander-akait deleted the fix/css-at-rule-declaration-order branch October 10, 2026 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: css CSS support (lib/css)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant