[robobun]

Problem

• With default targets, bun build prints wrong transition lists for 18 properties (mask* and three more). .a{transition:-webkit-mask-box-image 1s,mask-border 2s,opacity 3s} prints -webkit-mask-box-image 1s, -webkit-mask-box-image 2s, mask-border 3s, opacity 1s. Merged rules repeat -webkit-mask-size .3s, and each rebuild adds one.
• expand_properties (src/css/properties/transition.rs) adds entries without a look at the list, and rule merging runs it on its own output. No user reported this.

Fix

• An entry gains only the prefixes that no other entry of its property has, and a -webkit- mask name only when the list lacks it. No entry is removed or reordered.
• ListedPrefixes (one VendorPrefix per PropertyIdTag) answers both. The loop fills it once, when the first entry needs it.
• The mask-composite name row now also returns PropertyId::WebKitMaskComposite for -webkit-.
• Verified: test/js/bun/css/css.test.ts, test/bundler/css/: 74 new cases, 54 fail on main. Self-reviewed: 31 concerns raised, 27 addressed (Notes).

Background

• A prefixable PropertyId carries a VendorPrefix bit set, which Targets::prefixes widens to what the targets need.
• mask-border, mask-composite and mask-mode have WebKit properties with other names, inserted as entries of their own.
• Considered: a scan per entry (the first version: 7 s for 64,000 entries), a table for every list, upstream's merge_properties (wrong durations).

Downsides

• A list that needs a prefix builds a 256-byte table: transition-property: -webkit-mask-size, mask-size costs 698 instructions per flush, main 499.
• As on main, an inserted entry shifts later durations: mask-border 1s, opacity 2s prints mask-border 2s, opacity 1s. Until css: compare custom property names in PropertyId equality and importance in duplicate-rule detection #33684, rules that differ only in an unknown name merge in one more case.
• A -webkit-mask-composite declaration is now parsed: XOR prints xor.

Notes

Reproduce. Plain bun build, default targets, no flags. Bun 1.4.3 and main print the same.

echo '.a{transition:-webkit-mask-box-image 1s,mask-border 2s,opacity 3s}' > c.css
bun build c.css
# before: transition: -webkit-mask-box-image 1s, -webkit-mask-box-image 2s, mask-border 3s, opacity 1s;
# after:  transition: -webkit-mask-box-image 1s, mask-border 2s, opacity 3s;

printf '.a{color:red}\n.a{transition:mask-size .3s}\n' > a.css
bun build a.css
# before: transition: -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s;
# after:  transition: -webkit-mask-size .3s, mask-size .3s;

echo '.a{transition:-webkit-mask-composite 1s,mask-composite 2s}' > b.css
bun build b.css
# before: transition: -webkit-mask-composite 1s, -webkit-mask-composite 2s, mask-composite 1s;
# after:  transition: -webkit-mask-composite 1s, mask-composite 2s;

echo '.a{transition:mask-composite 1s}' > cur.css
for i in 1 2 3; do bun build cur.css --outdir=o >/dev/null; cat o/cur.css; cp o/cur.css cur.css; done
# before: 1, 2, 3 entries of `-webkit-mask-composite 1s`
# after:  1, 1, 1

Cost of the handler change. Release builds (--profile=release --lto=off) of one tree: main, the head before this commit (2ef7de1, which copied the list and scanned it for each entry), and this head.

One rule with n x mask-size 1s, cssInternals.prefixTest, Safari 15, three rounds, in ms:

n	main	head before	this head
16,000	12, 11, 9	438, 454, 455	10, 11, 10
32,000	17, 18, 17	1,744, 1,783, 1,788	17, 18, 18
64,000	43, 35, 31	7,002, 7,100, 7,077	38, 32, 33
128,000	66, 67, 72	28,125, 28,573, 27,675	84, 74, 68

bun build of one rule with 64,000 x mask-size 1s: 62, 60, 57 ms (main 83, 54, 61 ms, head before 7,007, 7,099, 7,072 ms). 40,000 rules with a list of 1 / 4 / 12 entries that needs nothing: 57 / 105 / 221 ms (main 60 / 104 / 222 ms).

Instructions executed in one TransitionHandler::flush, from entry to return, without the allocator (gdb single step). The three numbers are main / head before / this head.

list	1 entry	4 entries	12 entries
opacity, color, width, top	688 / 781 / 690	1,864 / 2,226 / 1,791	4,454 / 5,614 / 4,197
transform, box-shadow, border-radius, flex (no prefix needed)	821 / 923 / 839	2,378 / 2,776 / 2,369	5,996 / 7,264 / 5,931
block-size, then plain names, Safari 8	731 / 754 / 762	1,900 / 2,192 / 1,858	4,562 / 5,512 / 4,398
mask-size	1,067 / 1,273 / 1,235	2,821 / 4,063 / 3,257	7,145 / 14,305 / 8,309
mask-border, then plain names	1,201 / 1,352 / 1,335	2,292 / 2,740 / 2,453	5,302 / 6,914 / 5,639

list	2 entries	4 entries	12 entries
transition: -webkit-mask-size 1s, mask-size 1s pairs	1,516 / 1,659 / 1,514	2,463 / 2,985 / 2,477	6,071 / 9,571 / 6,149
transition-property: -webkit-mask-size, mask-size pairs	499 / 851 / 698	794 / 1,629 / 1,113	1,974 / 6,203 / 2,773

• The table is built at most once in a flush. It is not built when no entry needs a prefix or a -webkit- name. A list that already has the -webkit- entry does build it, which is the last row.
• Heap allocations in a flush are the same as on main. The head before had one more malloc and free from the fifth entry.
• Stack frame of flush: 920 bytes (main 648, head before 792). Text of the handler: 6,822 bytes (main 5,945, head before 7,970).
• #[cold] on the table build: 12 plain entries cost 4,197 instructions with it and 4,230 without.

Output against the head before. 600,000 random stylesheets print the same bytes on 2ef7de1 and on this head. The lists use about 100 property names, 24 sets of targets, merged rules, runs of same-query @media rules, and a second pass over the output.

What the new transition cases pin. Each of these changes to the loop fails at least one case: a table that ignores the property, no write of the expansion to the rtl list, no insert into the rtl list, added prefixes computed against the unprefixed bit alone, no entry for a name that takes no prefix, a table that is built after the first store, all added prefixes dropped when one of them is listed, and a -webkit- name taken as listed when any entry of its property is.

The long list case. a long list is expanded in linear time builds transition-property lists of 200,000 x mask-size and of 500,000 x mask-border with -webkit-mask-box-image listed last, in a child process with a 20 s limit. The child takes 4.1 s on the debug build with ASAN. A release build of the head before needs about 70 s for the first list, so the limit ends it. duplicate-declaration-merge-hang.test.ts has the same shape.

Considered and not taken for the lookup. A pass for prefixes and -webkit- names in front of the loop for logical properties needs no write to the rtl list, but it walks each list twice. A table that is filled for each list costs a 256-byte fill where no entry needs it. A rewrite that appends to a new list also removes the quadratic insert, but it allocates for each list that changes, and it belongs with the fix of the durations.

Self-review of this head. 31 concerns raised, 27 addressed. Not taken:

• A split of the name table row into its own PR. It is its own commit (5ea1d73), and it closes the one default-target case of this class that still grew.
• A second-pass check inside prefix_test and minify_test (see "Not in this PR").
• Cases that pin a duration that prints wrong today.
• An issue on lightningcss about merge_properties.

The two changes need each other for mask-composite. masking::get_webkit_mask_property has 8 names that get a -webkit- entry with another name.

• With the handler change only, 7 of the 8 are stable when the output is minified again. mask-composite still grows, because the written -webkit-mask-composite did not resolve to PropertyId::WebKitMaskComposite, the id that the handler looks for.
• With the row change only, no transition output changes, because main inserts the entry without a look at the list.
• The test transition over %s is the same when minified again runs all 8 names.

Reach. This PR has no issue. No user reported either case. A check of the output of this PR found the mask-composite case.

• mask-composite is in 0 of 13,429 transition declarations in the 3,581 CSS files on the build machine (the packages of the repo and of its tests, and the install cache).
• The checklist of MVP of CSS parser #14167 has -webkit-mask-composite ticked under "Minify and downlevel properties". Before this PR the parser never reached that property.
• Under the default targets 18 property names gain an entry in a transition list: mask, mask-image, mask-position, mask-size, mask-repeat, mask-clip, mask-origin, mask-border and its five longhands, mask-composite, mask-mode, background-clip, backface-visibility, text-emphasis-color.
• The 367 stylesheets that parse in the repository and in the install cache of the build machine print the same bytes on main and on this head.

What still prints wrong. The handler inserts a -webkit- mask entry into the property list only. Each property takes its duration by position, so the durations after an inserted entry shift. transition: mask-border .5s, opacity .1s prints -webkit-mask-box-image .5s, mask-border .1s, opacity .5s. main and lightningcss do the same. This PR stops the shift only for a list that already has the -webkit- entry, because nothing is inserted then. The open review thread on transition.rs describes the defect.

More cases, all with plain bun build:

• A list that names one -webkit- mask entry while another entry still inserts one shifts the later durations by a different count than main. -webkit-mask-box-image 2s, mask-mode 1s, mask-border 1s ends in mask-border 2s here. On main it ends in mask-border 1s, after a second -webkit-mask-box-image 2s.
• transition-property: mask-size, opacity; transition-duration: .3s, .2s prints three names for two durations, as on main.
• transition: inset 1s, opacity 2s prints top 1s, bottom 2s, left 1s, right 2s, opacity 1s, as on main.
• Two rules whose lists differ only in a name that Bun does not parse are taken for equal rules (CSS minifier drops one of two adjacent :where() rules that hold only custom properties (1.4.2 regression) #44704, css: compare custom property names in PropertyId equality and importance in duplicate-rule detection #33684 is open for it). .a{transition:fill .3s} .b{transition:stroke .3s} merges on main too. .a{color:red} .a{transition:mask-size .3s,fill .3s} .b{color:red;transition:mask-size .3s,stroke .3s} merges here and not on main, where the doubled entry of .a keeps the two lists apart.
• The insert of a -webkit- mask entry, or of the names of a logical property, moves the rest of the list each time. Such a list costs quadratic time, as on main: 64,000 x mask-border 1s takes 459 ms (main 461 ms), 64,000 x inset 1s takes 472 ms (main 481 ms).

What changes for a -webkit-mask-composite declaration. The declaration parser uses the same name lookup in each block type (style rules, nested rules, @keyframes, @page, style attributes) and in @container style().

input	before	after
.a{-webkit-mask-composite:XOR}	XOR	xor
.a{-WEBKIT-MASK-COMPOSITE:xor}	-WEBKIT-MASK-COMPOSITE:xor	-webkit-mask-composite:xor
.a{-webkit-mask-composite: xor , source-over /* c */ }	xor,source-over (space at the end)	xor,source-over
.a{-webkit-mask-composite:xor}.b{-webkit-mask-composite:XOR}	two rules	.a,.b{-webkit-mask-composite:xor}
.a{-webkit-mask-composite:xor}.b{color:red}.a{-webkit-unknown:1}	.b{color:red}.a{-webkit-unknown:1} (first rule lost)	all three rules
@container style(-webkit-mask-composite: XOR)	XOR	xor
@import 'x.css' supports(-WEBKIT-MASK-COMPOSITE: XOR)	name as written	-webkit-mask-composite:XOR
values exclude, plus-lighter, var(--x), inherit, xor xor	as written	as written
names -moz-, -ms-, -o-mask-composite	unknown name	unknown name

• The keyword list is the one of lightningcss: clear, copy, source-over, source-in, source-out, source-atop, destination-over, destination-in, destination-out, destination-atop, xor. WebKit also has plus-darker and plus-lighter (CSSProperties.json, <-webkit-compositing-operator>). Firefox takes the name as an alias of mask-composite (longhands.toml, extra_prefixes = ["webkit"]), so exclude is valid there. Such values stay unparsed and print as written.
• The lost rule in row 5 comes from StyleRule::is_duplicate. It compares property ids, and all unknown names have the same id. The name now has its own id. This PR does not change the general case (css: compare custom property names in PropertyId equality and importance in duplicate-rule detection #33684 does).
• CSS modules: the composes conflict check now takes -WEBKIT-MASK-COMPOSITE and -webkit-mask-composite in two composed files as one property, as it does for COLOR and color. -webkit-mask-composite and mask-composite stay two properties.
• Bun has no mask handler, so a parsed -webkit-mask-composite passes through DeclarationBlock::minify in its place. mask-composite: exclude does not get a -webkit-mask-composite: xor line. That is as before.
• The two real stylesheets on the build machine that have the declaration and parse (@carbon/styles 1.118.0, styles.css and styles.min.css) build to the same bytes with and without the row change.

lightningcss. master has the same two rows (src/properties/mod.rs: the key "-webkit-mask-composite", and From<CowArcStr> strips the prefix before from_name_and_prefix). So lightningcss also reads the name as a custom property and prints the declaration as written. Its merge_properties() folds a later entry of the same property into the first one and leaves the duration list at full length.

Measurements of the row change. Base is main 449877f with the handler change. Release builds, same machine, one after the other.

• Size (nm -S, size -A): the name table values go from 4,016 to 4,000 B, the sort order from 502 to 500 B, the key text from 3,704 to 3,682 B. There are 183 maker functions where there were 184 (2,944 to 2,942 B). from_string is 841 B in both. .text is 82,166,153 B and .rodata is 22,598,604 B in both, because padding takes up the 42 B. The stripped bun is 107,730,656 B in both.
• Code (llvm-objdump -d): 3 of the 225 instructions of from_string differ. All 3 are immediates: the key count 251 to 250 at two sites, and the length of the key text. The calls are the same: 4 strncasecmp, 1 sorted_key_position, 1 indirect call of the maker. There is no allocation and no syscall. The mask-composite maker goes from 6 to 10 instructions (a bit test and a conditional move, no branch).
• Instructions executed (qemu-x86_64 -one-insn-per-tb -d exec,nochain -dfilter on from_string and sorted_key_position): one CSS module with a declaration for each of the 251 table keys and for 10 other names makes 264 lookups. They take 84,428 instructions in both builds. 4 lookups differ: border-block-end-color 470 to 502, border-block-end-style 264 to 296, border-block-end-width 330 to 264, -webkit-mask-composite 231 to 233. The three border-block-end-* names are in the same 22-byte group of the sorted table as the removed key.
• Table replay (a Python model of from_string and the table): the 46,574 names in the 15 sheets of test/js/bun/css/files need 43,594 searches and 133,817 compare steps with both tables. valgrind and perf are not installed on the build machine.
• Output: bun build prints the same bytes for those 15 sheets. None of them has the name.
• Newly reached code (gdb breakpoints, debug builds): the five WebKitMaskComposite arms in properties_generated.rs (property_id, value_to_css, parse, deep_clone, eql) run 0 times without the row change for the new test inputs. All five run with it. eql runs when two rules have equal values. deep_clone runs for .foo, .bar:focus-visible with Safari 13, which splits the rule.

Not in this PR.

• The duration shift after an inserted entry (see above).
• Names that the handler prints only for targets that bun build cannot select. transition:flex-grow 1s for Firefox 20 prints -moz-flex-grow, which the table does not read back (45, 63, 81 bytes over three passes). -ms-order (IE 10) and -o-transition (Opera 12) behave the same.
• all has no row: transition:ALL 1s keeps its upper case.
• Other output that grows when bun build builds it again, for example :root{color-scheme:light} (64, 103, 142, 181 bytes over four passes, css: emit the color-scheme fallback vars once when a block is minified again #38512 is open for it).
• A compile-time check that no key of the table starts with -. The table has no such key after this change. I can add the check if a maintainer wants it.
• A shared test helper that checks each expected output again for a fixed point. It is a change to test/js/bun/css/util.ts for all tests, so it belongs in its own PR.
• Links to reports in other projects about a doubled -webkit-mask-composite declaration. They are about declarations and not about transition lists, and I did not check them.

Other work on these lines. #45825 rewrites the table rows with an alias: column and changes from_string. It does not touch the two rows of this PR. Branch robobun/0e87a129/css-mask-handler (no PR) adds a mask handler and has the same row change in 61eb965.

Tests. bun bd test test/js/bun/css/css.test.ts: 1388 pass, 0 fail. 74 cases are new (73 in css.test.ts, 1 in test/bundler/css/css-modules.test.ts). On a build of main, 54 of them fail. With the handler change and without the row change, 23 fail. The other 20 pin behavior that does not change. test/bundler/css/, test/bundler/esbuild/css.test.ts and the other files of test/js/bun/css/ pass. css-fuzz.test.ts has 10 s limits that the debug build does not meet, with and without the change. It passes on a release build.

First description of the handler change (2026-08-14)

Problem

• With CSS targets that need vendor prefixes (this includes bun build's default browser target, whose safari 14 entry still prefixes the mask-* properties), transition lists come out with duplicated prefixed entries:
  • .a{color:red} .a{transition: mask-size .3s} -> transition: -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s (plain bun build a.css)
  • a run of same-query @media rules merged into one gains one more copy per merged rule: three rules give -webkit-mask-size .3s, -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s
  • transition: margin-inline-start .2s, transform .2s (safari 8) duplicates -webkit-transform inside every generated :lang() rule, without any rule merging
  • already autoprefixed input does it in a single pass: transition: -webkit-transform .3s, transform .3s, opacity .2s (safari 6) -> -webkit-transform .3s, -webkit-transform .3s, transform .3s, opacity .2s; same for transition-property: -webkit-transform, transform
  • the same for the -webkit-mask-box-image entry inserted for mask-border: a merged rule or transition: -webkit-mask-box-image .2s, mask-border .2s ends up with it twice
• Cause: expand_properties in src/css/properties/transition.rs expands every unprefixed entry to the full prefix set for the targets (set_prefixes_for_targets) and inserts the mask-box-image counterpart of a mask-border* entry unconditionally, without looking at what the rest of the list already says. The handler's own output lists every prefix as a separate entry, so each pass over a block adds the expansion again, and rule merging (flush_pending_style_merge and the CssRule::Media arm in src/css/rules/mod.rs) as well as add_logical_rule all run a block through the handlers more than once.
• lightningcss 1.30.2 prints each of the inputs above with a single prefixed entry.

Fix

• expand_properties keeps a copy of the list as written. When an entry is expanded for the targets, the prefixes the expansion added that another entry of the same property already carries are dropped again (expand_prefixes), and the -webkit-mask-box-image* entry is only inserted when the list does not contain it yet (webkit_mask_property_to_insert). Both helpers replace the previously duplicated ltr/rtl blocks.
• Why this is the right shape:
  • Only bits the expansion added are ever removed, and no entry is removed or reordered. Each entry therefore keeps its own duration, delay and timing function (in flush those lists are paired with the property list by position), and author-written repeats such as transform 1s, transform 2s still expand both entries, since the unprefixed bit is never something the expansion added.
  • The handler's output is now a fixed point of the handler: every prefix it generated is present as an explicit entry of the same property, so a second pass adds nothing, and the output no longer depends on how many rules were merged into a block. The merged and :lang() outputs now match lightningcss.
  • Upstream handles the explicit-prefix case with merge_properties(), which folds later entries of the same property into the first one and drops them while leaving the duration/delay/timing lists alone, so every later entry is paired with the wrong values: lightningcss 1.30.2 turns transition: -webkit-transform .3s, transform .3s, opacity .2s into ..., opacity .3s, also without targets. Bun gets that input right today, so that function is deliberately not ported. Subtracting from the expansion instead gives upstream's output whenever the merged entries have the same values (every re-minify case) and keeps the author's values otherwise.
  • Upstream does not dedupe the mask-box-image insertion either (it duplicates it on a merged rule too); it is covered here because it is the same fixed-point requirement in the same function.
• Verified:
  • test/js/bun/css/css.test.ts, transition block: 13 new prefix_tests (explicit prefixed entry before and after the unprefixed one, different durations per entry, transition-property, repeated unprefixed entries, merged same-selector rules, a run of three merged @media rules, the :lang() rules for a logical property, and mask-border: insertion for safari 15, no insertion for safari 16, explicit entry, merged rule, logical rules). 10 of them fail on the unfixed build with the duplicated entries, the other 3 pin the unchanged behavior they build on; all pass with the fix, along with the rest of the file.
  • bun bd test test/bundler/css/ test/bundler/esbuild/css.test.ts: pass.
  • bun build on the stylesheet below produces the single-entry output with the fix.

Background

• Minifying a stylesheet runs every declaration block through the property handlers (DeclarationHandler); handlers such as TransitionHandler buffer the related declarations and write a minimal form in finalize/flush. When CssRuleList::minify merges a rule into an earlier one with the same selector, or an @media rule into the previous one with the same query, the merged block is minified again, and the :lang() rules a handler emits through add_logical_rule are minified as new rules. So a handler sees its own output as input, and its output has to be stable under that.
• A PropertyId of a prefixable property carries a VendorPrefix bit set. Targets::prefixes replaces a set that contains the unprefixed bit with the full set the browser targets need and returns an explicitly prefixed set unchanged; that is what set_prefixes_for_targets applies to each list entry. flush then turns a multi-bit entry of a transition shorthand into one transition per bit (get_transitions), which is why the handler's own output comes back as separate single-prefix entries; a transition-property list prints a multi-bit entry as one name per bit.
• mask-border and its longhands have no prefixed spelling; WebKit's equivalent is -webkit-mask-box-image, a different property, so masking::get_webkit_mask_property returns a separate PropertyId that the handler inserts in front of the entry instead of setting a prefix bit.

bun build repro (default targets)

.a { color: red }
.a { transition: mask-size .3s }
.b { transition: -webkit-mask-size .3s, mask-size .3s, opacity .2s }
@media (min-width: 1px) { .e { transition: mask-size .3s } }
@media (min-width: 1px) { .f { color: blue } }
@media (min-width: 1px) { .g { color: green } }

bun 1.4.0 / main:

.a { color: red; transition: -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s; }
.b { transition: -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s, opacity .2s; }
@media (min-width: 1px) {
  .e { transition: -webkit-mask-size .3s, -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s; }
  ...
}

with this change:

.a { color: red; transition: -webkit-mask-size .3s, mask-size .3s; }
.b { transition: -webkit-mask-size .3s, mask-size .3s, opacity .2s; }
@media (min-width: 1px) {
  .e { transition: -webkit-mask-size .3s, mask-size .3s; }
  ...
}

Comparison with lightningcss 1.30.2 (targets chrome 20, firefox 20, safari 5, ie 9, ios_saf 6, android 4 unless noted)

input	bun before	bun after	lightningcss
.a{font-weight:bold}.a{transition: opacity 1s, transform 1s}	opacity 1s,-webkit-transform 1s,-ms-transform 1s,-webkit-transform 1s,-ms-transform 1s,transform 1s	opacity 1s,-webkit-transform 1s,-ms-transform 1s,transform 1s	same as after
three merged @media (x:1) rules, first with transition: opacity 1s, transform 1s	three copies of -webkit-transform 1s,-ms-transform 1s	one	same as after
.a{transition: opacity 1s, -webkit-transform 1s, transform 1s}	-webkit-transform twice	opacity 1s,-webkit-transform 1s,-ms-transform 1s,transform 1s	same as after
.a{transition-property: opacity, -webkit-transform, transform}	-webkit-transform twice	opacity,-webkit-transform,-ms-transform,transform	same as after
.a{transition: -webkit-transform .3s, transform .3s, opacity .2s} (safari 6)	-webkit-transform twice, opacity .2s	-webkit-transform .3s,transform .3s,opacity .2s	-webkit-transform .3s,transform .3s,opacity .3s
.a{transition: transform 1s, -webkit-transform 1s} (safari 6)	-webkit-transform 1s,transform 1s,-webkit-transform 1s	transform 1s,-webkit-transform 1s	-webkit-transform 1s,transform 1s
.a{font-weight:bold}.a{transition: mask-border 1s}	-webkit-mask-box-image twice	-webkit-mask-box-image 1s,mask-border 1s	-webkit-mask-box-image twice

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: farm/e66ba5e1/css-transition-prefix-dedupe
• File: .coderabbit.yaml
• Commit: 34d3eee
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 274ad202-26b7-49bb-a982-0154ec35ce64

Commits

Reviewing files that changed from the base of the PR and between f345c0f and 34d3eee.

Files selected for processing: 1

• test/js/bun/css/css.test.ts

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: farm/e66ba5e1/css-transition-prefix-dedupe
• File: .coderabbit.yaml
• Commit: f345c0f
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: fe972981-13ea-46fc-bd3d-03cf78578313

Commits

Reviewing files that changed from the base of the PR and between 2ef7de1 and f345c0f.

Files selected for processing: 3

• src/css/properties/properties_generated.rs
• src/css/properties/transition.rs
• test/js/bun/css/css.test.ts

Files with no reviewable changes: 1

• src/css/properties/properties_generated.rs

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

The CSS property lookup now resolves WebKit-prefixed mask-composite through the prefixed mask-composite entry. Transition expansion preserves explicitly written prefixes and adds required WebKit mask aliases only when absent. Tests cover prefix expansion, mask-property parsing and minification, composition warnings, and rebuilt output.

Changes

CSS prefix handling

Layer / File(s)	Summary
Property lookup and transition expansion src/css/properties/properties_generated.rs, src/css/properties/transition.rs	The lookup maps WebKit-prefixed mask-composite to its distinct ID. Transition expansion skips prefixes already listed for the same property and adds the WebKit mask alias only when the target requires it and it is absent.
Transition prefix tests test/js/bun/css/css.test.ts	Tests cover prefix expansion for existing transform entries and WebKit mask properties, including duplicate handling, target thresholds, merged rules, logical-property rules, long lists, and rebuilt output.
Mask property integration tests test/js/bun/css/css.test.ts, test/bundler/css/css-modules.test.ts	Tests cover WebKit mask-composite parsing and serialization, container style queries, and composition warnings.

Suggested reviewers: jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to f345c

No actionable issue remains from the supplied changes; the PR is mergeable after normal checks.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the main change: preventing duplicate transition entries when the declaration already lists the corresponding prefixed property.
Description check		The description is detailed and covers the problem, fix, scope, limitations, and verification results. It does not use the template headings exactly, but it provides the required information, includin…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description is detailed and covers the problem, fix, scope, limitations, and verification results. It does not use the template headings exactly, but it provides the required information, including how the code was tested.

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[robobun]

Status

Reproduced on Bun 1.4.3 and on main with plain bun build (default targets):

• .a{color:red} .a{transition:mask-size .3s} prints transition: -webkit-mask-size .3s, -webkit-mask-size .3s, mask-size .3s;
• .a{transition:-webkit-mask-composite 1s,mask-composite 2s} prints transition: -webkit-mask-composite 1s, -webkit-mask-composite 2s, mask-composite 1s;

Not taken. The consolidated CSS parser work in #47025 leaves this change out, so this PR is closed. The branch stays available.

[claude]

LGTM — focused fixed-point fix in expand_properties so re-minifying a block (rule merge, :lang() emission, or already-prefixed input) no longer duplicates prefixed transition entries. The inline note below is pre-existing and strictly mitigated here, not introduced.

Extended reasoning...

Overview

Touches src/css/properties/transition.rs (expand_properties + two new local helpers, ~40 net lines) and adds 13 prefix_test cases to test/js/bun/css/css.test.ts. The behavioral change: before expanding prefixes, snapshot the list as written; after set_prefixes_for_targets, drop only the newly added prefix bits that another entry of the same .tag() already carries; and skip the -webkit-mask-box-image* insert when the snapshot already contains it. The two helpers also replace the previously duplicated ltr/rtl blocks.

Security risks

None. Pure CSS-minifier output shaping — no I/O, no unsafe, no external input beyond parsed CSS that was already handled identically before this change. Worst-case failure mode is suboptimal (but still valid) CSS output.

Level of scrutiny

Moderate. The prefix-bit arithmetic is subtle, so I traced it: written bits are never in added, so an entry can never lose a bit it started with (the result is never empty and author order/positions are preserved — transform 1s, transform 2s still expands both, pinned by a test). as_written is used only for containment/tag-match, not indexed access, so index shifts from earlier logical-property replace() calls don't affect it. The rtl branch reuses the ltr as_written, which is sound because rtl_properties is cloned from properties and the two lists stay index-aligned (all Inline ltr/rtl replacement slices are equal-length). PropertyId::tag() returns the prefix-less discriminant, so Transform(WEBKIT) and Transform(NONE) compare equal for the fold — verified in properties_generated.rs.

Other factors

• The one finding is explicitly pre-existing (the properties.insert without growing durations/delays/timing_functions); this PR reduces how often that path fires and does not make it worse.
• The deliberate divergence from upstream merge_properties() is well-argued: upstream's approach shifts durations onto the wrong entries (opacity .2s → .3s in the comparison table), which Bun avoids today and this change preserves.
• Test coverage spans the variant matrix the PR description enumerates: explicit prefix before/after unprefixed, distinct durations, transition-property, repeated unprefixed, merged same-selector, three merged @media, :lang() rules, and the four mask-border cases.

[claude]

🟣 Pre-existing (not introduced by this PR, and strictly mitigated by it): when the -webkit-mask-box-image* entry is inserted here, durations/delays/timing_functions in flush are not grown to match, so get_transitions cycles and shifts every subsequent entry — safari 15 + transition: mask-border .5s, opacity .1s comes out as -webkit-mask-box-image .5s, mask-border .1s, opacity .5s. The LogicalPropertyId::Block/Inline replace() arms (e.g. inset-block → two entries) have the same shape. Noting only because the refactored block is where a fix would land and the new tests all use single-entry or equal-duration lists that hide the cycling; not blocking.

Extended reasoning...

What the bug is

expand_properties receives only the properties list (flush() calls it as expand_properties(&mut p.0, arena, context)) and grows it in place via properties.insert(index, property_id) when a mask-border* entry needs a -webkit-mask-box-image* counterpart. The durations, delays, and timing_functions lists that flush holds alongside it are never touched, so after the insert the property list is one entry longer than the value lists it is paired with.

get_transitions then walks properties.slice() and pairs each entry with durations.at(durations_idx), bumping the index once per property via cycle_bump(&mut durations_idx, durations.len()). When properties.len() > durations.len(), that cycling shifts every entry after the insertion point onto its neighbor's value and wraps the last entry back to the first slot.

Step-by-step repro

Input (targets: safari 15, which needs the -webkit-mask-box-image insert):

.foo { transition: mask-border .5s, opacity .1s }

1. handle_property splits the shorthand into properties = [MaskBorder, Opacity], durations = [.5s, .1s] (delays/timing similarly length-2).
2. flush calls expand_properties. webkit_mask_property_to_insert returns WebKitMaskBoxImage (as_written does not contain it), so properties.insert(0, WebKitMaskBoxImage) runs. Now properties = [WebKitMaskBoxImage, MaskBorder, Opacity] — length 3.
3. durations is still [.5s, .1s] — length 2; flush never grew it.
4. get_transitions iterates:
   • WebKitMaskBoxImage → durations[0] = .5s, bump → idx 1
   • MaskBorder → durations[1] = .1s, bump → idx 0 (cycle)
   • Opacity → durations[0] = .5s
5. Output: transition: -webkit-mask-box-image .5s, mask-border .1s, opacity .5s — mask-border lost its .5s and opacity was rewritten from .1s to .5s.

The prefix-expansion path (expand_prefixes) is not affected: a multi-bit VendorPrefix stays a single properties[] entry and get_transitions fans it out with one durations_idx increment. Only the mask insert (and the LogicalPropertyId::Block/Inline arms whose replace() calls insert_slice for multi-entry expansions like InsetBlock → [Top, Bottom]) add real list entries.

Why this is pre-existing and why the new tests don't catch it

The removed code did the identical properties.insert(index, property_id) without growing the value lists; this PR only wraps the condition in webkit_mask_property_to_insert and adds the as_written dedup. So this is not a regression — and the PR strictly reduces how often it fires, since the second-pass insert (merged rules, :lang() rules) is now skipped when the entry is already listed. The upstream lightningcss code the file was ported from has the same behavior.

The 13 new prefix_tests all use either a single mask-border entry (durations.len() == 1, so cycling is a no-op) or equal durations across every entry (.2s, .2s), both of which are invariant under the shift. A two-entry list with distinct durations, e.g. transition: mask-border .5s, opacity .1s at safari 15, would exercise it.

What a fix would look like

The value lists need the entry duplicated at the same index the property was inserted at (so the inserted -webkit-mask-box-image reuses mask-border's duration and everything after it stays paired). That means either passing the value lists into expand_properties and inserting alongside the property, or having expand_properties return the insertion indices for flush to apply to all four lists. The same treatment would cover the multi-entry Block/Inline replace() expansions. Not asking for it here — it's a separate change from the fixed-point fix this PR makes, and it exists upstream too — but flagging it since the PR description's "no entry is removed or reordered, so each entry keeps its own duration" holds for expand_prefixes but not for this insert.

[robobun]

The description now states this limit under Downsides. The insert still changes the property list only, so transition: mask-border .5s, opacity .1s prints -webkit-mask-box-image .5s, mask-border .1s, opacity .5s, as on main.

This PR does not change the insert. It stops the shift only for a list that already has the -webkit- entry, because nothing is inserted then. 5ea1d73 adds cases with different durations for that path, for example -webkit-mask-composite 1s, mask-composite 2s, opacity 3s.

[robobun]

One more case for this thread, found in the review of f345c0f. When a list names one -webkit- mask entry and another entry still inserts one, the later durations shift by a different count than on main. -webkit-mask-box-image 2s, mask-mode 1s, mask-border 1s ends in mask-border 2s with this PR and in mask-border 1s on main.

The description lists it under "What still prints wrong". This PR does not change the pairing. That fix needs the expansion to keep the source entry of each name that it inserts.

[robobun]

^{Updated 12:25 PM PT - Oct 26th, 2026}

✅ @robobun, your commit 34d3eee884d23f103652a21376e914f81122f598 passed in Build #133189! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38692

That installs a local version of the PR into your bun-38692 executable, so you can run:

bun-38692 --bun

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the parts new since the last review: the removed b"-webkit-mask-composite" row in properties_generated.rs was unreachable (from_string strips the vendor prefix via VendorPrefix::strip_from before the table lookup, and no other full-name prefixed rows remain), and -webkit-mask-composite values outside the WebKit keyword set still fall to Unparsed and print as written, which the new mask tests pin. In expand_properties, deduping the rtl list against the ltr as_written snapshot is sound because the _ arm only runs for entries that are identical in both lists (logical entries are replaced in the other arms), so the ltr and rtl outputs can only differ in the replaced physical properties.

Extended reasoning...

The change touches the CSS transition handler's prefix expansion in src/css/properties/transition.rs and one row of the hand-maintained property name table in src/css/properties/properties_generated.rs, plus tests; no security-sensitive surface. The findings-present note records what else was examined and ruled out since the prior review; the inline findings already signal a human should look.

[claude]

🟣 pre-existing, not blocking: pre-existing: users minifying two same-selector rules that each hold only custom or unknown properties with different names lose the earlier rule. PropertyId equality at properties_generated.rs:896 compares tag and prefix only, so every Custom id is equal; is_duplicate (style.rs:480) then lets rules/mod.rs:858 drop the earlier rule. This PR moves only -webkit-mask-composite out of that population, which is what these two new cases pin. Fix: make duplicate detection compare the custom/unknown property name as well, so --a vs --b and -webkit-x vs -webkit-y rules are never duplicates, while identical names still dedupe.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Input such as :root{--a:1} .x{color:red} :root{--b:2} goes through CssRuleList::minify. src/css/rules/mod.rs:850-859 computes a StyleRuleKey from the selector hash plus property-id tags (src/css/rules/style.rs:44-47; both declarations have tag Custom), then calls is_duplicate. src/css/rules/style.rs:480 compares a.property_id() != b.property_id(), and src/css/properties/properties_generated.rs:893-897 defines PropertyId equality as self.tag() == other.tag() && self.prefix() == other.prefix(), so PropertyId::Custom("--a") == PropertyId::Custom("--b"). The earlier rule is set to CssRule::Ignored at rules/mod.rs:858 and --a disappears from the output. The base branch behaves the same; this diff does not touch is_duplicate or PartialEq, it only makes -webkit-mask-composite a non-Custom id, and the new tests at test/js/bun/css/css.test.ts:8098-8106 pin that one escape while every other unknown or --custom name pair still collapses.

Verification: pre-existing — triggered whenever two same-selector rules whose declarations are all custom/unknown properties with different names are not adjacent; the earlier rule is dropped. PartialEq for PropertyId (src/css/properties/properties_generated.rs:893-897) is self.tag() == other.tag() && self.prefix() == other.prefix(). The base branch fails identically by the same route.

[robobun]

Agreed that PropertyId equality ignores the custom name, and that this PR does not change is_duplicate. The two cases here pin only that -webkit-mask-composite is no longer a Custom id. A name-aware duplicate check is a separate change.

[robobun]

#33684 is open for the general case. It compares the names of custom and unknown properties in PropertyId equality, so is_duplicate no longer takes two such rules for the same rule.

[robobun]

Related, and now in the description: until #33684 lands, that equality merges two rules in one more case with this PR.

.a{color:red} .a{transition:mask-size .3s,fill .3s} .b{color:red;transition:mask-size .3s,stroke .3s} merges .a and .b here, so .b loses stroke. On main the doubled -webkit-mask-size entry of .a keeps the two lists apart. .a{transition:fill .3s} .b{transition:stroke .3s} merges on main too.

[claude]

I re-reviewed after the latest push and found no bugs; the fixed-point matrix and the Bun.build test now pin exact first-pass output, which closes the earlier nit. A human look is still worthwhile for one deliberate product choice rather than any defect: the mask-composite row change makes Bun parse -webkit-mask-composite as its own property (keywords lowercased, same-selector rules merged), which lightningcss does not do, so it changes output for existing stylesheets beyond the transition bug.

What was reviewed:

• expand_prefixes: added is computed as new-minus-written, so an entry's own prefix is never stripped; Targets::prefixes returns an already-vendor-prefixed entry unchanged, so -webkit-transform passes through untouched. RTL list gets the same two helpers at the same index.
• Whole bug class: set_prefixes_for_targets / get_webkit_mask_property have no other call sites in src/css, so there is no sibling handler left with the old shape.
• Name table: from_string strips the prefix before from_name_and_prefix, confirming the removed b"-webkit-mask-composite" key was unreachable and the new make(pre) closure returns WebKitMaskComposite only for -webkit-; -moz-/-ms-/-o- still fall to Custom.
• Tests: cssInternals.minifyTest(source, "", targets) matches the bindgen signature; assertions are toBe/toEqual on exact output.

Extended reasoning...

The change touches the CSS transition property handler (src/css/properties/transition.rs, two new helpers replacing inline prefix-expansion and -webkit- mask insertion) and one row of the hand-maintained property name table (src/css/properties/properties_generated.rs), plus ~550 lines of tests in test/js/bun/css/css.test.ts and one css-modules test. It touches no security-sensitive surface. The handler logic was traced by hand and is behaviour-preserving except for the intended dedupe, and the prior-run nit about non-exact assertions was addressed in commit 2ef7de1; no third-party CHANGES_REQUESTED or CODEOWNERS coverage applies. Defer rather than approve because the name-table row intentionally diverges from lightningcss by parsing -webkit-mask-composite declarations, a user-visible output change a maintainer should consciously accept.

Still open from earlier reviews (2):

• Unresolved: 2 minor or pre-existing.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Still open from earlier reviews (2):

• Unresolved: 2 minor or pre-existing.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline note, I also checked two things in this push: the deleted literal -webkit-mask-composite row in from_name_and_prefix was unreachable (from_string strips the vendor prefix before the lookup, so that key could never match), so the new mask-composite row is the only way that name resolves and nothing is lost; and the ltr/rtl lists in expand_properties stay parallel, since the rtl clone is taken after earlier entries were already expanded and every later prefix write and -webkit- insert goes to both lists at the same index. Also confirmed Targets::prefixes returns an already-prefixed entry unchanged, so a listed -webkit-x never re-expands.

Extended reasoning...

The change rewrites the prefix/webkit-alias expansion loop in src/css/properties/transition.rs around a lazily built per-tag ListedPrefixes table, deletes the now-unused PropertyId::set_prefixes_for_targets, and re-routes -webkit-mask-composite through the mask-composite name row; it touches no security-sensitive surface. The one confirmed finding is a test nit (child stderr inherited rather than captured), and the remaining open question from the prior push is the long-list test's 60 s/90 s wall-clock bound, which the latest commit widened rather than shrank.

Still open from earlier reviews (3):

• Unresolved: 3 minor or pre-existing.

[claude]

I re-reviewed the head after the last push and found no bugs; the stderr capture in the linear-time test resolves what I raised last time. Because this rewrites the transition prefix-expansion loop and makes -webkit-mask-composite a parsed property (user-visible output change), a maintainer look is still worthwhile.

What was reviewed:

• The lazily built ListedPrefixes table versus replace() running before or after it: replaced logical ids contribute only the NONE bit, which can never intersect added (always NONE-free) or satisfy contains() for a -webkit- name, so build order is harmless.
• targets.prefixes() returns a prefixed written unchanged, so only unprefixed entries expand and the expanded != written gate is equivalent to the old set_prefixes_for_targets writes on both lists; no remaining callers of the deleted helper.
• The name-table change: from_string strips the vendor prefix before lookup, so the old b"-webkit-mask-composite" row was unreachable and the new row adds parsing rather than altering an existing path; tag() as usize is bounded by PROPERTY_BITSET_BITS.

Extended reasoning...

The change touches src/css/properties/transition.rs (rewritten vendor-prefix expansion loop with a per-property listed-prefix table), src/css/properties/properties_generated.rs (removed helper, mask-composite row now maps the -webkit- prefix to PropertyId::WebKitMaskComposite), and two test files adding ~75 cases. It touches no security-sensitive surface. Since my previous review only one test-only commit landed, which addressed my last nit; the two remaining open inline threads of mine are pre-existing, non-blocking notes the author replied to. Not approving because the loop rewrite relies on non-obvious invariants (lazy table, rtl-list parity) and the parsing change alters bundler output for existing stylesheets, which a CSS maintainer should weigh; no CODEOWNERS entry covers these paths and the hunt ran dry.

Still open from earlier reviews (2):

• Unresolved: 2 minor or pre-existing.

[robobun]

#47022 also changes expand_properties. It records, for each name, the index of the entry as written, so that get_transitions gives a transition entry that becomes several names its own duration, delay and timing function (transition: inset 1s, opacity 2s printed top 1s, bottom 2s, left 1s, right 2s, opacity 1s).

It has no duplicate-name rule, so the two PRs do not overlap in behavior. They touch the same function. The one that merges second has a small conflict there: the sources parameter, and the record_names call after the match.

[Jarred-Sumner]

Not taken in #47025 (7942008), which consolidated the open CSS parser pull requests. It is right or harmless, but not worth its code, the bigger output or the behavior it changes.