Repository navigation
Conversation
The transpile-only CSS path parsed .module.css files with the invalid source index and printed them against an empty symbol table, so the first class or id selector hit Option::unwrap() on None in the CSS printer. Parse with the single-file source index, print with the symbols the parse produced, and name the locals with the same <name>_<hash of pretty path> scheme the linker uses, shared through LinkerContext::local_css_name, so the output matches what bun build emits for the same file.
Contributor
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
Collaborator
Author
|
Status: fix pushed, waiting on CI and review. Reproduced on released bun 1.4.0 and on main: calling |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it adds a new public StyleSheet::local_names method to the CSS crate and rewires the shared testing_impl (source index, symbol map, local-names table), a human look would still be worthwhile to confirm the placement of that helper in css_parser.rs rather than inline in the test binding.
What was reviewed:
Config::hash_sourceextraction incss_modules.rsis a byte-for-byte refactor of the inline call inCssModule::new; bundler behavior unchanged.- Changing
testing_implfromIndex::INVALIDto source 0 and building the symbol map fromextra.symbols— non-css-modules callers produce no refs, so the map is never consulted for them. - The removed
minifyTestWithOptionsre-export fromutil.tshad no other importers;css.test.tsnow uses the new wrapper andcssInternalsdirectly for the throw case. get_truthy+.to_boolean()filter followed byget_boolean_looseforpurematches existing option-reading patterns;{ pure: false }correctly falls through to enable css modules.
Extended reasoning...
Overview
This PR fixes 13 vacuous test calls in test/js/bun/css/css.test.ts that imported the raw minifyTestWithOptions binding directly (running at collection time, never asserting), wires up the previously commented-out css_modules parser option in the test-internal binding, and adds the plumbing needed for the printer to resolve css-modules symbol refs in a standalone sheet. Five files are touched: two test files (css.test.ts, util.ts), the test-only binding (css_jsc/css_internals.rs), and two CSS-crate files — a pure refactor in css_modules.rs extracting Config::hash_source, and a new StyleSheet::local_names method in css_parser.rs.
Security risks
None. This is test infrastructure plus a helper method that maps a sheet's local class/id names to their hashed output form. No untrusted input handling, no auth/crypto/permissions.
Level of scrutiny
Moderate. Most of the surface is test-only (bun:internal-for-testing bindings and test files), but ~40 lines land in the production CSS crate. The hash_source extraction is a pure refactor and the new local_names method is only invoked from the test binding today, so the effective blast radius on shipped behavior is near zero. Still, local_names is a new pub method on StyleSheet with its own hashing/pattern-application logic, and REVIEW.md cautions against adding production code solely to make a test writable — though the PR description notes the same mechanism is what transpiler.rs would need for the separately-reported --no-bundle panic, which argues for it living here.
Other factors
The PR description is unusually thorough: it verifies the tests fail on USE_SYSTEM_BUN=1 and pass on the debug build, and reruns the surrounding CSS/bundler suites. The testing_impl change from Index::INVALID to Index(0) and from an empty symbol::Map to one built via init_with_one_list(extra.symbols) affects all seven test bindings sharing that body, but non-css-modules parses produce no symbol refs so nothing consults the map — the description calls this out explicitly. I checked that init_with_one_list takes Vec<Symbol> matching StylesheetExtra.symbols, that get_boolean_loose exists and returns Option<bool>, and that no other file imported the removed minifyTestWithOptions re-export. Given the new public method on StyleSheet and the moderate diff size, a maintainer should confirm the layering choice rather than have it land on automated approval alone.
Collaborator
Author
|
On the placement of It also is not test-only in practice: |
…ls into real tests
css.test.ts imported the raw minifyTestWithOptions binding as
minify_test_with_options, so its 13 calls in describe("selectors") ran at
collection time, registered no test and compared nothing. The binding also
ignored the css_modules option (parser_options_from_js left it commented
out), so the 11 css-modules expectations, ported from lightningcss, never
held.
The binding now enables css modules for that option and prints the sheet
the way the --no-bundle path does: parsed as the runtime source, printed
with its own symbols and a local-name table. The naming rule the linker
and the --no-bundle path shared inside the bundler crate moves into bun_css
as css_modules::local_name, and StyleSheet::local_names builds the table
for a sheet printed on its own, so the linker, bun build --no-bundle and
the test binding all name locals through one function. Requesting the
unimplemented pure mode throws instead of being ignored. util.ts gets a
real minify_test_with_options wrapper and stops re-exporting the raw
binding; the expectations are updated to bun's <name>_<hash> output.
robobun
changed the base branch from
main
to
farm/33f77f8c/no-bundle-css-modules
August 14, 2026 20:40
robobun
force-pushed
the
farm/9877f509/css-test-binding-css-modules
branch
from
August 14, 2026 20:41
ac61379 to
b57417d
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+946
to
+948
| // `bun_css` wraps this as `css_modules::hash`; everything that derives a CSS | ||
| // module name (`css_modules::local_name`, the printer's pattern path) or a | ||
| // dependency placeholder from a path goes through that wrapper. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+2859
to
+2861
| // Computed the way the bundler computes `Source.path.pretty` (see | ||
| // `generic_path_with_pretty_initialized`: forward slashes on every | ||
| // platform), because CSS module names are hashed from it. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+3076
to
+3078
| // Every class/id selector of a CSS module becomes a `LocalCss` symbol | ||
| // `Ref` carrying this source index; the printer resolves those through | ||
| // the symbol map and local names built below, so the three must agree. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+456
to
+458
| /// The canonical implementation is `bun_base64::wyhash_url_safe`; this keeps the | ||
| /// `css_modules::hash` path for in-crate callers (`dependencies.rs`, | ||
| /// `rules/import.rs`, `local_name`). |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+464
to
+471
| /// The name a CSS module's local (a class or id selector) is printed as: | ||
| /// `<local>_<hash of path>`, where `path` is the stylesheet's path relative to | ||
| /// the project (`Source.path.pretty` in the bundler) so the names are stable | ||
| /// across machines. The bundler applies this to every local in the graph | ||
| /// (`LinkerContext::mangle_local_css`); [`StyleSheet::local_names`] applies it | ||
| /// to a sheet printed on its own. | ||
| /// | ||
| /// [`StyleSheet::local_names`]: crate::StyleSheet::local_names |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+2558
to
+2565
| /// The `local_names` to print this sheet with when it is not part of a | ||
| /// bundle (the bundler names the locals of the whole graph at once in | ||
| /// `LinkerContext::mangle_local_css`): every css-modules local, named | ||
| /// by [`css_modules::local_name`] as if the sheet were bundled from | ||
| /// `path`. Empty unless the sheet was parsed with css modules on. | ||
| /// | ||
| /// `source_index` must be the index the sheet was parsed with, since | ||
| /// the local refs embed it. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+26
to
+28
| /// Stands in for the path of the stylesheet under test when the `css_modules` | ||
| /// option is on: the names of its locals are hashed from it, exactly as | ||
| /// `bun build` would hash the path of a real `test.module.css`. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+175
to
+177
| // Same single-sheet setup as `Transpiler::build_css_output`: css-modules | ||
| // locals are refs into `extra.symbols` at this source index, which | ||
| // `init_with_one_list` below puts at index 0. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+271
to
+272
| // Bun's css modules have no lightningcss-style `pure` mode, so a test | ||
| // asking for it must not silently run without it. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+461
to
+462
| /// The printed name of a CSS module's local (class or id): `<local>_<hash of path>`, | ||
| /// `path` being the sheet's project-relative path (`Source.path.pretty` in the bundler). |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Comment on lines
+2558
to
+2560
| /// The `local_names` for printing this sheet outside a bundle (the bundler | ||
| /// builds the whole graph's in `LinkerContext::mangle_local_css`). | ||
| /// `source_index` must be the one the sheet was parsed with: the refs embed it. |
Contributor
There was a problem hiding this comment.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code
Contributor
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. It touches production bundler paths (LinkerContext::mangle_local_css refactored through a new shared css_modules::local_name helper, and transpiler.rs now scopes CSS-module locals for --no-bundle with a changed relative-path computation), adds public API to bun_css, and has grown past the description to include the --no-bundle crash fix — a human look is worthwhile.
What was reviewed
LinkerContext.rsrefactor is byte-for-byte behavior-preserving:local_name(name, path)computes<name>_<wyhash_url_safe(path, at_start=false)>exactly as the removed inline block did.css_internals.rschanges are test-only (bun:internal-for-testing); the non-css-modules path still produces no refs, so the emptylocal_namesmap and single-list symbol map are inert for existing tests.- New
--no-bundletests spawn hermetically withtempDir/bunEnv, drain both pipes, and pin parity against the bundled output.
Extended reasoning...
Overview
The PR spans nine files across four crates plus tests. What started as "make the css_modules test option work" now also contains the bun build --no-bundle *.module.css crash fix (commit 4ac28276) that the description says was "reported separately", plus a comment-shortening pass (ff3d3070). Concretely:
- Test-only:
src/css_jsc/css_internals.rswires up thecss_modulesoption, parses as source index 0, and hands the printer a real symbol map andlocal_namestable.test/js/bun/css/util.tsgains a realminify_test_with_optionswrapper;css.test.tsgets 17 real tests where 13 collection-time no-ops used to be. - New public
bun_csssurface:StyleSheet::local_names(source_index, path)andcss_modules::local_name(local, path). - Production bundler:
LinkerContext::mangle_local_cssis refactored to call the sharedlocal_namehelper instead of inlining the hash-and-format logic. - Production transpiler:
transpiler.rsnow parses CSS modules asIndex::RUNTIME, computesfile_path.prettyviarelative_platform::<Loose, false>(forward slashes on every platform, matchinggeneric_path_with_pretty_initialized), buildslocal_namesand a one-listsymbol::Mapfromextra.symbols, and passes them toto_css. This is the actual--no-bundlecrash fix. - New tests:
test/bundler/css/css-modules.test.tsgains adescribe.concurrentblock covering--no-bundleoutput (inline snapshot) and a parity check againstbun buildfor both a top-level and nested entry.
Security risks
None identified. No untrusted-input parsing changes; the hash is content-addressing for class names, not a security primitive. The --no-bundle path already parsed the file — only the printer wiring changed.
Level of scrutiny
Medium-high. The css_internals.rs and test changes are low-risk (test-only binding). But LinkerContext.rs runs for every bundled CSS module, and transpiler.rs runs for bun build --no-bundle and Bun.Transpiler on .module.css. The LinkerContext refactor is provably identical (I traced local_name → hash → bun_base64::wyhash_url_safe with the same at_start=false and the same {name}_{hash} concat), and existing css-modules.test.ts snapshots (baz_I7o34g) would catch any drift. The transpiler.rs change, however, is a behavioral change on Windows (relative path now uses forward slashes) whose only coverage is the new parity test — a maintainer should confirm that matching the bundler's pretty path here is the intended contract.
Other factors
- The PR description is out of sync with the diff: it says the
--no-bundlepanic was "reported separately", but the fix and its tests are in this PR. - The comment-cop bot flagged long comments in two rounds; the last round (20:56) still points at
css_modules.rs:462andcss_parser.rs:2560, and earlier flags attranspiler.rs:2861/3078— the current diff's comments there look reasonably tight, but a maintainer should confirm the bot is satisfied. - Design decision worth a human eye:
local_namehard-codes<local>_<hash>rather than going throughConfig.pattern, which is fine because both callers use the default pattern, but it does bake the format into a third place (the doc comment says as much).StyleSheet::local_namessimilarly ignores the sheet's ownoptions.css_modules.patternand always callslocal_name; that's consistent with the bundler today but is a choice.
Jarred-Sumner
added a commit
that referenced
this pull request
Oct 27, 2026
### What does this PR do? Consolidates the open pull requests on the CSS parser (`src/css`: parsing, minifying, lowering, printing, CSS modules) into one. 179 open pull requests were in scope. Each was judged by one rule: **the output must mean to a browser what the input meant.** Every claim was run through a build of `main`, a build with the change, lightningcss 1.33 and Chromium 156 (CSSOM, computed values, painted pixels for colors and gradients), and counted against 10,116 real stylesheets: the published CSS of about 190 popular npm packages, plus stylesheets from open source repositories. 107 land here, whole or in part, most of them cut down. The source diff is net negative. Of the 10,116, 9,509 are stylesheets that someone ships. The rest are test fixtures of other tools (521), Tailwind v4 source that needs its plugin (59) and SCSS, PostCSS or template source (27). | 9,509 shipped stylesheets | `main` | this branch | |---|---|---| | fail to build | 71 | 35 | | ... of those, valid CSS that Bun rejects | | 0 | | minified output reads differently in Chromium than the input | 552 | 2 | | CSS modules with a name that is referenced but not defined, or exported but not defined | 166 | 0 | | all 10,116 | `main` | this branch | |---|---|---| | fail to build | 192 | 141 | | minified output reads differently in Chromium than the input | 593 | 15 | | build on `main`, fail here | | 0 | | right on `main`, wrong here (minified, and at the default targets) | | 0 | | size of the minified output, no targets | 162.32 MB | 162.47 MB (+0.093%) | | size of the output at the default targets | 224.1 MB | 210.2 MB (-6.2%) | A syntax error is a build error, with one exception: the IE star hack `*zoom: 1` is skipped, because tachyons and handsontable ship it. The 35 shipped stylesheets that fail are invalid CSS: 28 have a selector that Chromium drops (semantic-ui-css, swiper, @ionic/core, Adobe Spectrum, xp.css), 2 an `@import` after other rules, and one each `@supports not X and Y`, a nested `@keyframes`, a stray comma, a selector list in `::slotted()`, `@media screen\0` (govuk-frontend). The 2 that read differently fold a `color-mix()` inside a gradient (see the known gaps). The growth of the minified output is rules and declarations that `main` deleted by mistake: 87 KB of rules, 45 KB of prefixed declarations that only browser targets can rule out. #### Popular CSS that `main` breaks today - normalize.css, modern-normalize, sanitize.css: `font-family: monospace, monospace` is collapsed to `monospace`, which changes the font size from 16px to 13px (#46662). - CKEditor 5, Tabler, Carbon, VitePress, Ionic, PatternFly, Excalidraw: a rule of custom properties is deleted as a "duplicate" of a later rule with the same selector and other custom properties. 37 stylesheets lose 1,681 custom properties (#33684). - Material Components, daisyUI, DocSearch, plyr: a `var()` size loses to its own fallback (#42101). - nextra, daisyUI, CKEditor: declarations written after a nested rule are moved above it (#45462). - antd 4, purecss, govuk-frontend: browser hacks in selector lists start to apply everywhere (#42100). - foundation-sites, flatpickr, 98.css, blueprintjs, scalar, react-datepicker: the build fails on valid CSS (#46756, #38552, #38534). tachyons, handsontable: it fails on the IE star hack (#46751). - nodejs.org, Mantine: a CSS module exports classes that no rule defines, because the `@apply` or `@mixin` in them is moved out of the rule (#46759). - hint.css and 42 more: `z-index: 1000001` and `1000000` become the same number (#46715). - Adobe Spectrum: `--version: 5.0.1` prints as `5.1`. - A quarter of all stylesheets: minified output has a stray space in `align-items: center` (#41993). #### Behavior changes to look at first 1. **`light-dark()` is kept as written at the default browser targets** (#42009). The lowered form `var(--buncss-light, a) var(--buncss-dark, b)` is invalid unless a compiled `color-scheme` covers the element: 184 of the 215 real stylesheets that get it never define the variable, so every browser drops the declaration. The docs section on the polyfill is removed. 2. **`border` is not built from longhands** (#42072): `border-width:0;border-style:solid;border-color:red` stays three declarations, because `border` also resets `border-image`. 305 real stylesheets grow by 30 KB in total; `bootstrap.min.css` by 113 bytes, 5 gzipped. No real stylesheet was found where the old merge shows, so this is the one to drop if size matters more. 14 existing expectations change. 3. **A CSS module no longer exports names that only occur inside `:global(...)`** (#45487). 73 real modules lose 202 export keys, all of them names that no rule has. `import { g }` or `composes: g` of such a name becomes a build error. 4. **Container names and `@counter-style` names stay global in a CSS module** (#45247 by @mikamikasuki). `main` hashes the `@container` name but not `container-name`, so a named query never matches. postcss-modules (Vite, css-loader, Next.js) keeps them global; lightningcss and esbuild hash both. Global keeps a container that is queried from another file or from JS working. 5. **Syntax errors are build errors, also two that `main` lets through**: a media query that stops short (`@media screen and {`, `@media not {`, `@import "a.css" screen and;`) and `@media (width <) foo {`. No stylesheet of the 10,116 has either. A condition that is valid by the grammar though no browser knows it, `( ... )` or `fn( ... )`, is kept as written (#46756): `(min-width:0\0)`, `(not(hover))`. 6. **Declarations after a nested rule stay where they were written**, as a nested declarations rule (#45462). Changes the output of 199 real stylesheets; in a fuzz of 2,850 nested programs compared by computed style, `main` was wrong in 1,056 and the pull request's code, before it was trimmed here, in none. 7. **`Bun.color`**: `color()` inputs are converted instead of returned as a string (`Bun.color("color(srgb 1 0 0)", "hex")` was `"color(srgb 1 0 0)"`); `currentColor`, system colors and `light-dark()` give `null` in the converting formats instead of the keyword as a string; a relative color or `color-mix()` outside sRGB is clipped, not gamut-mapped (`hsl(from oklch(100% .399 336.3) h s l)` is `#ff5bff`, was `#fff`). 8. The `:-moz-any()` and `:-webkit-any()` copies of rules with `:is()` are gone at the default targets (#45945, #42029). The `:-webkit-any()` copy outranked the original rule: `.title{color:blue} :is(h1,h2,h3){color:red}` painted red. 5.8% of all default-target output was the `:-moz-any()` copies. 9. `Property` grows from 216 to 224 bytes: `Url` stores its text as a slice so that two `url()` values can be compared (#46895). The pull request packed a pointer and a `u32` behind `unsafe` to avoid that. #### Not in this PR, needs a decision - **Raise the default browser targets (#40369) and add a `cssTarget` option (#40368).** Today: Chrome 80, Edge 80, Firefox 78, Safari 14, Opera 67, and no public way to change them. At Vite 8's floor the output of the 9,924 stylesheets is 15.3% smaller (3.9% gzipped). About a dozen fixes here only matter at today's floor, and code for older targets is unreachable without the option. - Vendor prefixes for `user-select`, `appearance`, `hyphens` and five more (#45814): 1,116 real stylesheets lack a prefix they need at the default targets. And for `backdrop-filter` (#44648): 173. - Clip instead of gamut-map in the sRGB fallback of a plain wide-gamut color: deletes about 90 lines, changes every pinned fallback hex. If the mapping stays, #44647 by @oddharsh makes it closer to what browsers paint (colors off by 2/255 or more: 168 to 110 of 3,170 real ones). #### Changes **Rules: duplicates, merging, nesting, fallbacks** - The `:-moz-any()` copy of every rule with `:is()` or `:not(a, b)` is gone: Firefox 78 has `:is()` (#45945). 1,164 real stylesheets - Keep declarations written after a nested rule in place with a nested declarations rule (#45462). 379 real stylesheets - With nesting compiled away, an unknown at-rule inside a style rule was moved out of it: `.wrapper{@apply flex}` printed a bare `@apply flex;` and the rule was gone, so a CSS module exported a class that no rule defines. It now stays in a block under the compiled selector. 72 real modules with Tailwind's `@apply`, 25 with Mantine's `@mixin` (#46759). 316 real stylesheets - Keep !important in the rules generated for logical properties and color-scheme (#38597). 311 real stylesheets - With browser targets, remove an earlier duplicate rule only when its declaration block is equal (#46861). 171 real stylesheets - Compare custom property names in `PropertyId` equality and importance in duplicate-rule detection (#33684). 40 real stylesheets - Keep same-condition @supports rules and empty named @layer blocks (#46733). 31 real stylesheets - Keep target-added vendor prefix passes on cloned and re-minified rules, and never print a bare `:full-screen` (#43339). 12 real stylesheets - Keep the selectors on rules added for color fallbacks and logical properties (#43240). 3 real stylesheets - A later declaration of a property drops the `@supports` color copies of the earlier one, which printed after the rule and won (#45560). 2 real stylesheets - Do not merge style rules in a stylesheet that holds `revert-rule` (#46953). 1 real stylesheets - `@import ... supports(-webkit-mask: none)` kept its prefix (#38589) **Properties and shorthands** - Print align-items:center without a stray space when no safe/unsafe keyword is given (#41993). 3,317 real stylesheets - Emit the physical longhands instead of the inset shorthand for targets without it (#38583). 1,452 real stylesheets - Keep the stops of an upward gradient as written, and drop a `50%` hint only at the midpoint (#46790). 104 real stylesheets - Emit unparsed border-block-* declarations when compiling logical borders (#38582). 89 real stylesheets - Keep repeated font families and dedupe only the inserted system-ui fallbacks (#46662). 88 real stylesheets - A `width` / `height` / `min-*` / `max-*` with `var()` or `env()` was printed before the plain fallback written above it, so the fallback won (#42101). 26 real stylesheets - Keep box-orient and box-direction when flex-direction or flex-flow follows (#45672). 16 real stylesheets - Write held prefixed background declarations at every flush (#46882). 6 real stylesheets - Derive equality for Size2D and BorderRadius, and keep every prefix in the border-radius handler (#45665). 2 real stylesheets - Print `none` instead of an empty border-image, mask-border or mask-box-image value (#45143). 2 real stylesheets - Keep font/transition sub-properties on the side of the shorthand they were written on (#41990). 2 real stylesheets - CSS-wide keywords are not names: `font:12px Arial;font-family:inherit` printed `font:12px inherit`, `color-scheme:inherit` and `color-scheme:auto` printed `normal`, `transition-property:unset` was folded into `transition` (#42022). 2 real stylesheets - Only fold font longhands into `font` when the rule declared a `font` (#45459) - Keep a font family name quoted when its identifiers do not read back as the same name (#46606) - Stop rewriting valid positions into ones browsers reject (#45197) - `border` is no longer built from longhands unless the rule also sets what `border` resets (`border-image`), and an author's `border-image` reset is no longer dropped. `transition` is likewise only written where the rule wrote one, because it resets `transition-behavior` (#42072) - `border; border-image: <gradient>; border-image-slice: 1` lost the gradient in every default build (#42018) - A prefixed `linear-gradient` keeps its direction, written or generated: the prefixed syntax names the starting side and measures angles differently (#46697) **At-rules** - Print @font-face format() keywords as strings (#37979). 439 real stylesheets - Print @supports `not` conditions without a leading space (#38559). 88 real stylesheets - Reset after the number token of a unicode-range (#45835). 33 real stylesheets - Keep `<general-enclosed>` media and container conditions as written (#46756). 24 real stylesheets - A strict range such as `(width < 768px)` lowered for old targets still matched at exactly 768px. The step is now `.02px`. The `not all and` form for other units is not taken (#46806). 18 real stylesheets - Keep scanning an @supports condition after a nested block (#38552). 9 real stylesheets - Every `@counter-style name` printed as `@counter-stylename`, which browsers drop (also #46692). In a CSS module the name now stays global, like the `list-style` that refers to it (#33679). 9 real stylesheets - Keep a keyframe whose block has an invalid item (#46288). 2 real stylesheets - Read an @Property initial-value on the stylesheet's parser, keep an unmatched value as written (#45806). 2 real stylesheets - `@media {}` is `all`. A media query that stops after `and`, `or` or `not` (`@media screen and {`) is a build error; `main` printed `not all` or dropped the condition, silently. The `@layer` and type selector parts are not taken (#46869). 2 real stylesheets - Accept a name-only `@container` and a `style()` feature with no value (#46689). 1 real stylesheets - Parse `<timeline-range-name> <percentage>` keyframe selectors (#45652) - Allow `or` inside parentheses after a media type (#45088) - Print the `=` comparison in media and container queries as `=` (#45071) - Parse the @font-palette-values font-family descriptor as a list of family names (#45484) **Selectors** - Never wrap a selector list in :is() (#46787). 414 real stylesheets - Stop emitting :-webkit-any() copies of :is() that outrank the original rule (#42029). 373 real stylesheets - With browser targets, a selector list with a vendor-prefixed member was split, which revives hacks that rely on other engines dropping the whole list: `.s *::-ms-backdrop,.s{flex:auto}` applied everywhere. Rewritten; the list is printed as written (#42100). 42 real stylesheets - Print the vendor prefix on downleveled :any-link, :read-only, :read-write, :placeholder-shown and :autofill (#42131). 12 real stylesheets - Fix nesting lowering of a `&` that does not lead its selector (#43047). 8 real stylesheets - `@scope`: `&` and implicit selectors compile to `:where(:scope)`, relative selectors and bare declarations parse (#43165). 7 real stylesheets - Only reject bare :global / :local pseudo-classes in CSS modules (#38534). 5 real stylesheets - Stop truncating `@scope to (...)` and stop replacing `&` in the scope-end selector (#43146). 2 real stylesheets - `[xl|href$=".pdf"]` lost its namespace prefix (#45159). 2 real stylesheets - Match An+B keywords exactly and take only `+` as the leading sign (#42953). 1 real stylesheets - Parse ::details-content and ::picker() like ::part() (#45504). 1 real stylesheets - Parse :window-inactive as an ordinary pseudo-class, and accept it after ::selection (#45815) - Print :nth-col(1) and :nth-last-col(1) in functional form (#41575) - `li::before::marker` failed the build (#45810) - `&` inside `:nth-child(... of S)` is resolved when nesting is compiled away. The second selector serializer (326 lines) is deleted (#43024) **Colors and `Bun.color`** - Keep light-dark() as written under the default browser targets (#42009). 227 real stylesheets - Number and percentage channels parse in `lab()`, `lch()`, `oklab()`, `oklch()`, `hsl()` and `hwb()`; the hue of `lch` / `oklch` is no longer scaled by alpha (#45084). 130 real stylesheets - `Bun.color` returns `null` for `currentColor`, system colors and `light-dark()` in the converting formats, instead of the CSS keyword as a string (#33047). 130 real stylesheets - Relative colors: channel keywords are on the scale of their function (`rgb(from #369 calc(r + 10) g b)`), the origin's alpha is kept, a relative color kept as written gets no `@supports` copies, and a result outside sRGB is not folded (#45362). 31 real stylesheets - Keep rotate: 0deg out of none, and keep the origin alpha in relative colors (#38486). 13 real stylesheets - Compute the D50 reference white in f32 so greys convert to lab(l 0 0) like lightningcss (#38535). 7 real stylesheets - Return a missing hue for a grey from conversions into lch, oklch, hsl and hwb (#45201). 5 real stylesheets - Fix color(a98-rgb), srgb-linear relative colors and color-mix(in xyz-d50) (#38487). 4 real stylesheets - Match named colors without regard to ASCII case (#45092). 3 real stylesheets - Fold min(), max() and clamp() over plain numbers (#39515) - `calc(l - .08)` in a percentage channel of a relative color became NaN and printed `#000` (also #45970) (#45971) - Bun.color: convert color() inputs instead of returning them as a CSS string (#38488) - A `color-mix(in srgb, ...)` result outside sRGB prints `color(srgb ...)` instead of a gamut-mapped hex (#39269) **`calc()` and numbers** - Rounding to 6 significant digits ate into the integer part: `z-index:2147483647` became `2147480000`, `max-width:9999999px` became `10000000px`, `--x:1234567.5` became `1234570`. It now only cuts a fraction short: a value kept as tokens prints its source text, a typed value prints every integer digit and at least one digit of the fraction (#46715). 43 real stylesheets - A number without a unit is no longer repaired into `px` (`height:1.5715` is dropped by a standards-mode browser, so it prints as written), `calc(-1 * 0)` is not a length, and on a property of the range [0,∞] (sizes, border radii, padding, border and outline widths, gap, `flex-basis`, `font-size`) a `calc()` that folds below zero prints `0` instead of an invalid negative literal, and a negative literal no longer replaces the valid declaration before it (`width:10px;width:-5px` printed `width:-5px`) (#42104). 24 real stylesheets - Print numbers from the f32's shortest digits, not its f64 expansion (#38565). 17 real stylesheets - Print negative numbers below 1 with a single minus sign (#38516) - Keep math functions over two different length units as written (#38489) - Fold rem() with a truncated quotient, so it keeps the sign of the dividend (#32286) - A one-argument `min()` / `max()` around a sum printed without any function (`width:100% - 20px`) (#46145) - `round()` breaks ties upward and handles a negative step as browsers do (#45249) **CSS modules** - A CSS module exports its `@keyframes` names (`styles.spin`). Hashing is unchanged (#46720). 145 real stylesheets - A name inside `:global(...)` prints plain but was exported as `name_<hash>`, a class no rule has (also #38573) (#45487). 90 real stylesheets - Keep container names global in CSS modules (#45247). 6 real stylesheets - Tag the keyframes name when an `animation` value falls back to raw tokens (#46034). 6 real stylesheets - In a plain stylesheet `composes: a from "./b.css"` was treated as an import: the file was inlined and an absolute path printed (#45961). 4 real stylesheets - Drop a rejected composes instead of failing the file at print time (#38538). 1 real stylesheets - Keep every rule's custom properties for the composes conflict check (#38523) - Keep composes working after a nested rule in the same block (#38520) - `IdentOrRef::hash` read 2 of 16 bytes, so in release builds every class of a module hashed alike (#30784) **Tokens, `url()`, diagnostics** - The block skip stack no longer allocates on every block exit (#39533) - Report the diagnostic's line without its line breaks in position.lineText (#38342). 192 real stylesheets - Compare `url()` values by their text (#46895). 26 real stylesheets - Values kept as tokens keep the blanks they were written with instead of having them re-invented at print time: `attr(x type(<number>))` printed `type(< number> )`, which browsers drop. Minified output still drops the blanks that cannot matter (#45577). 19 real stylesheets - The IE star hack (`*zoom:1`) failed the whole build. The declaration is skipped, as every browser does. It is the one syntax error that builds: tachyons, handsontable and jasmine ship it (#46751). 8 real stylesheets - The path of a copied asset spliced into `url()` is always quoted, so a file name with a space or a parenthesis cannot end the `url()` (#41557). 7 real stylesheets - Move the skip loop of parse_until_before out of the generic body (#39520) - Remove dead code from bun_css: the unused `impl Default for Property` (#45618) - Default the at-rule and qualified-rule parser traits, drop the url_to_css shim (#31999) **Speed** Four inputs that took quadratic or exponential time, or produced output out of proportion, are now linear, with no new limit and no extra pass (#31639, #42243, #38531, #43421). **Tests only** - `css-fuzz.test.ts` never ran in CI and timed out on debug builds; it is a deterministic table now (#38500) - `nth-anplusb-ident.test.ts` counts heaps in a child process (#46331) - `color.test.ts` checks the ansi-16 sweep with one `expect()` (#42824) - Not CSS, but it failed this PR's CI and fails on `main` too: `ws-proxy.test.ts` on Windows, "a message sent during a TLS renegotiation arrives after it". The helper server now renegotiates from the callback of the frame's write (#46170) **No pull request covered these** - `--v: 5.0.1` printed as `5.1`, `--w: 1.0.0` as `10`, `--x: 1+2` as `12`, `2n+1` as `2n1`: a number token that touches another keeps its source text. - `color-scheme: auto` printed as `normal`, and `color-scheme: dark light` as `light dark`. - A bare `:global` or `:local` in a CSS module sets the scope of the names after it, up to the end of the selector, as in css-loader. It was a build error. - The scrollbar states (`a:horizontal`, `::-webkit-scrollbar{&:horizontal{}}`) are valid in a plain compound, as in WebKit and Blink. - Without browser targets, `-webkit-box-align`, `-ms-flex-align` and the like were dropped next to their standard form (flatpickr, foundation-sites, materialize-css: 28 stylesheets), and `-webkit-fill-available` before `-moz-available` (beercss). - `(-o-min-device-pixel-ratio: 2/1)` printed as `2`. - `sin()`, `cos()` and `tan()` are exact at multiples of 90deg: `calc(100px * cos(90deg))` printed `-.00000437114px`. - A `display-p3` color at targets that have P3 but not the other `color()` spaces (Safari 10.1 to 14) got itself as a fallback. - `width: rem(-18px, 5px)` folds to a negative literal once `rem()` is right; it prints `0`. - Consecutive nested declarations rules, which arise when same-query `@media` blocks in one rule merge, are folded into one and count against the existing selector expansion limit as the `&{}` rules they replace did. #### Known and not fixed here - Four longhands are still folded into a `transition` the author did not write where the targets add a prefixed copy of a property (`mask-image` at Chrome 80): only the shorthand gives the copy its own times. - A later `background`, `box-shadow` or `border-*` declaration still loses to the `@supports` copy of an earlier `var()` value of the same property with a wide-gamut color. Same on `main`; #45560 is taken for the properties of the fallback handler and for unparsed values only. - A merged rule prints the unprefixed name in the passes of the rule it absorbed: `.d::-webkit-input-placeholder{}.d::-moz-placeholder{}` prints `::placeholder` for the second (#42106, not taken: it breaks the merge of prefix-equivalent rules). - The `border`, `outline` and `font` shorthands, `line-height`, `scroll-padding`, `background-size` and a few more still print a `calc()` that folds below zero as a negative literal. No real stylesheet has one. - `all` is not a property Bun knows, so `a{margin-top:1px;all:unset}` prints `all` first. Same on `main`. - A selector kept as raw tokens loses the blank before `*` when minified: `--sel: .dark *` prints `.dark*`. Same on `main`. - `width:calc(0% + 12px)` folds to `12px`, which lays out differently in a table cell. Same on `main`. - The `:dir()` / `:lang()` rules of a lowered logical property print before the nested rules of their rule, so a logical declaration written after a nested rule of equal specificity still loses to it. Same on `main`. - A fraction is still rounded to 6 significant digits (`12345.678` prints `12345.7`, `33.3333333333%` prints `33.3333%`). Keeping every digit would add up to 282 KB over the corpus. - `@keyframes "quoted"` is hashed in a module but not exported. - In a module, a keyframes name inside a `var()` fallback is not hashed: `animation: var(--enter, fade 1s)`. Same on `main`. - A `color-mix()` or relative color folded to a hex inside a gradient changes the interpolation from Oklab to sRGB (VS Code, clawhub). Printing `color(srgb ..)` instead needs a fallback for `mask-image`, `list-style-image` and custom properties at the default targets, which have none. - A comma list in `@container` still fails the build. - After a bare `:global`, names that are hashed at print time (`::view-transition-old(foo)`) are still hashed; inside `:global(...)` they are not. #### Pull requests in scope that do not land here - Duplicates of a pull request that lands here: #36961 (of #37979), #38478 (of #46662), #38480 (of #46733), #38482 (of #46715), #38491 (of #38500), #38508 (of #45201), #38513 (of #45362), #38573 (of #45487), #38640 (of #38500), #45525 (of #45247), #45589 (of #46756), #45970 (of #45971) - Already fixed on `main`: #38674 - Right, but for input nobody writes (0 of 10,116 stylesheets, and not new syntax): #31916, #38526, #38592, #39349, #41139, #41801, #42042, #42484, #43078, #45604, #45908, #46086, #46221, #46224, #46273, #46694, #46707, #46737, #46755, #46816, #46870, #46934 - Right or harmless, but not worth their code, their bigger output or the behavior they change: #30905, #31924, #33332, #34426, #36225, #38006, #38512, #38547, #38551, #38561, #38572, #38576, #38607, #38623, #38667, #38692, #42106, #42912, #42918, #43407, #43434, #44700, #45442, #45546, #45747, #45759, #45825, #45946, #46061, #46127, #46206 - Need a decision (see above), stay open: #40368, #40369, #44647, #44648, #45814 - Stays open: #45787 Out of scope: CSS linking and chunking in the bundler, the dev server, docs-only pull requests, and repository-wide sweeps that touch `src/css` in passing. Thanks to @mikamikasuki (#45247), @alii (#39520, #31999) and @oddharsh, whose measurements on #40369 and #41990 were used. Fixes #16727. Fixes #24770. Fixes #30772. Fixes #44704. Fixes #45089. Fixes #45237. Fixes #46872. ### How did you verify your code works? - The 35 CSS test files pass on a debug build, also with `FORCE_COLOR=1`. 600+ of the tests in this branch fail on a build of `main`. An earlier head of this branch passed all 179 CI jobs that ran. - The reproduction of each issue listed above was run on `main` (wrong) and on this branch (right). - Differential run over the 10,116 stylesheets, `main` against this branch, minified without targets and through `Bun.build` at the default targets: no stylesheet that builds on `main` fails; Chromium 156 compares the output with the input rule by rule (CSSOM, then computed values on a sized element in a standards-mode document, colors as painted pixels in sRGB and P3). The comparison has 195 control pairs that must read equal or different. For stylesheets that read differently on both builds, mostly because of lowering, the sets of differences were compared; the three new ones are two minifier fixtures that are wrong on both and one comparison artifact. - CSS modules are checked for consistency instead, since hashed names defeat the comparison: every reference points at a name the output defines, every export exists. - `Bun.color`: 59 inputs in 13 formats on both builds; every changed cell was read.
Collaborator
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.
Problem
test/js/bun/css/css.test.tsimported the rawminifyTestWithOptionsbinding under the nameminify_test_with_options, so the 13minify_test_with_options(...)calls indescribe("selectors")ran at collection time, registered no test, and never compared the output toexpected(the binding readsexpectedinto_expectedand drops it). They could only fail by throwing.css_modules, and the binding ignored that option:parser_options_from_jsinsrc/css_jsc/css_internals.rshad thecss_modulesbranch commented out (it was commented out in the Zig version too, since CSS fixes #16514). Their lightningcss-ported expectations (._8Z4fiW_foo{...}) never held; with released bun 1.4.0 the binding prints:local(.foo){width:20px},div.my-class{color:red}, etc.css_parser.rsadd_symbol_for_name) and the printer resolves it through the symbol map andlocal_names(printer.rslookup_symbol).testing_implparsed withIndex::INVALID, printed with an emptysymbol::Mapand an emptyLocalsResultsMap, and threw awayextra.symbols, so it had nothing to print those refs with.purecss-modules mode the block was ported for does not exist in bun (rules/style.rsstill carries it as a TODO; there is noImpureCSSModuleSelectorerror kind), which is why theminify_error_test_with_optionscases next to these aretest.skip.Fix
css_internals.rs: a truthycss_modulesoption enablescss_modules::Config::default()and sets the filename totest.module.css, the same shapeParseTask.rsuses for real module files.css_modules: { pure: true }throwscss_modules.pure is not supportedinstead of silently running without it.testing_implparses the sheet as source 0, builds the printer's symbol map fromextra.symbolswithsymbol::Map::init_with_one_list, and passes the table fromStyleSheet::local_namesaslocal_names. Nothing changes for calls without the option: they produce no refs, so the symbol map and name table are never consulted.css_parser.rs: newStyleSheet::local_names(arena, source_index)maps every entry of the sheet'slocal_scopeto its generated name by applying the configured css-modulesPatternwith the hash of the sheet's filename. It returns an empty map when css modules are off.css_modules.rs: the per-source hash computed inline inCssModule::newbecomesConfig::hash_source, solocal_namesand the printer's ownwrite_identpath (keyframes names, custom idents) hash the same input with the same settings. No behavior change for the bundler.bun buildemits for that file. The bundler names locals<name>_<wyhash_url_safe(path)>inLinkerContext::mangle_local_css; the default pattern is[local]_[hash]with the same hasher, andbun build test.module.cssprints.foo_SsLGXg, which is the suffix the updated expectations use. The keyframes test in the diff pins that selectors and@keyframesnames, which are scoped by two different code paths, get the same suffix.util.ts: realminify_test_with_optionswrapper (test()+expect(...).toEqual(expected)), the raw binding is no longer re-exported, andParserOptions.css_modulesdocuments what the option does and thatpureis rejected.css.test.ts: the 11 css-modules cases now run against bun's<name>_SsLGXgoutput, plus cases for:global()leaving its argument alone, a name reused across rules resolving to one local, class and id in one selector,css_modules: false,{ pure: false }, thepure: truerejection, keyframes scoping, and:local()passing through when the option is off. The skipped pure-mode error cases are unchanged.USE_SYSTEM_BUN=1 bun test test/js/bun/css/css.test.ts -t selectorsfails the 17 new/updated tests (242 pass, 17 fail);bun bd test test/js/bun/css/css.test.ts -t selectorspasses (259 pass, 0 fail). Also green with the debug build: the rest oftest/js/bun/css/(exceptcss-fuzz.test.ts, which times out here before and after because each of its 1000Bun.builditerations takes ~50ms under ASAN),test/regression/issue/27598.test.ts,test/bundler/css/css-modules.test.ts,test/bundler/esbuild/css.test.ts,test/bundler/bundler_loader.test.ts,test/internal/source-lints/, andcargo fmt --check.Background
ParserOptions.css_modulesis set, each class/id name in a selector is registered once in the sheet'slocal_scope(name toCssRef) and becomes abun_ast::Symbolof kindLocalCssinStylesheetExtra.symbols; the selector AST stores aRef(inner index + the source index passed toparse) instead of the text.:local(x)printsxscoped,:global(x)prints it unscoped.LocalsResultsMap(local_names): aRefto generated-name table the printer consults for each such ref; a ref missing from it prints under its original name. In a bundleLinkerContext::mangle_local_cssfills it for every file and hands it toto_css; the CSS crate itself never produced one, which is whatStyleSheet::local_namesadds for sheets printed on their own.symbol::Map: per-source list of symbol lists indexed by a ref's source index, which is why the test binding has to parse as source 0 and wrapextra.symbolswithinit_with_one_list.Printer::write_identapplyingConfig.patternwith a hash of the sheet's filename), so a standalone sheet has two naming paths that must agree;Config::hash_sourceis the shared piece.bun:internal-for-testing'scssInternalsfunctions are test-only bindings over the CSS parser/minifier/printer;testing_implis the shared body behindminifyTest,minifyTestWithOptions,testWithOptions,prefixTest, and friends, so all of them gain the option.Two unrelated bugs noticed while reading this code were reported separately:
bun build --no-bundle x.module.csspanics inPrinter::lookup_symbol(same missing symbols/local names, intranspiler.rs), and a bare:globalpseudo-class is rejected even without css modules (selectors/parser.rsprecedence).