Repository navigation
Conversation
…and timing function `expand_properties` can turn one `transition-property` entry into several names. A logical shorthand becomes its physical longhands, and `mask-border` gets `-webkit-mask-box-image`. `get_transitions` matched the four lists by position, so every name after such an entry took the values of a different entry. `transition: inset 1s, opacity 2s` became `top 1s, bottom 2s, left 1s, right 2s, opacity 1s`. `expand_properties` now records, for each name, the index of the entry as written that the name came from. It starts the record when an entry first becomes several names. `get_transitions` reads the duration, delay and timing function at that index. The `transition-property` longhand output does not change.
|
Updated 7:29 AM PT - Oct 27th, 2026
✅ @robobun, your commit 7e116a00cfe4a721ef469b363bd9df828c4b7f79 passed in 🧪 To try this PR locally: bunx bun-pr 47022That installs a local version of the PR into your bun-47022 --bun |
|
Status: reproduced and fixed for the
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Track source entries during expansion src/css/properties/transition.rs |
Property expansion records source positions for expanded names and advances positions by the number of names produced. |
Reconstruct transitions with source indices src/css/properties/transition.rs, test/js/bun/css/css.test.ts, test/bundler/css/mask-geometry-box.test.ts |
LTR and RTL reconstruction selects timing values using each property’s source position. Tests cover expanded properties, list cycling, ordering, overrides, and prefixed mask-border output. |
Possibly related PRs
- oven-sh/bun#38692: Changes transition property expansion and addresses duplicate prefixed entries, while later transition values can still shift.
- oven-sh/bun#47025: Changes when the transition handler flushes buffered data for
transition-behavior. - oven-sh/bun#42042: Resets declaration-handler state for a resetting
alldeclaration, affecting transition data passed to flush.
Suggested reviewers: jarred-sumner
Priority:
Merge Risk: 
7e116
-
Merged rules can give a transition the wrong duration. This is a bounded visual regression to track if merged.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: pairing expanded transition entries with their own duration, delay, and timing function. Description check The description explains the problem, fix, scope, limitations, performance impact, and verification. It does not use the exact template headings, but it provides the required content in equivalent sec… 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 explains the problem, fix, scope, limitations, performance impact, and verification. It does not use the exact template headings, but it provides the required content in equivalent sections.
-
- 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.
Comment @coderabbitai help to get the list of available commands.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I also checked two things in src/css/properties/transition.rs and ruled them out: the unchecked sources[index as usize] read in get_transitions cannot go out of bounds, because once record_names materializes the record it is extended for every subsequent entry and the RTL list is expanded in lockstep with the same lengths; and the Block arm now stepping by block.len() skips only physical longhands that neither take a vendor prefix nor map to a -webkit- mask name, so the second visit it removes was a no-op.
Extended reasoning...
The change rewires how the CSS transition shorthand handler pairs expanded property names with durations, delays and timing functions, touching only src/css/properties/transition.rs plus tests; it touches no security-sensitive surface. Confirmed findings about right-to-wrong output for merged and override rules are posted inline, so a human should weigh those before merging.
Findings marked 🟡 are optional suggestions and need no follow-up push.
…ed entry A `transition-property` list that is written as a longhand had the same fault as the shorthand. The expanded names were written beside the value lists as the author wrote them, so `transition-property: inset, opacity` with `transition-duration: 1s, 2s` gave five names for two durations. `flush` now rewrites each value list that it writes as a longhand, with the record that `expand_properties` fills: every name gets the value of its entry. A list that already does that stays as written. A property that is printed once per vendor prefix is still one entry of the list, so its value is not repeated. The bundler case moves into the existing mask test file.
There was a problem hiding this comment.
Beyond the inline findings, I also checked two things in the new push: the Block arm now stepping i by block.len() loses nothing, since every Block list in get_logical_properties holds only unprefixed physical ids that set_prefixes_for_targets and the -webkit-mask lookup leave untouched; and sharing one sources record between the LTR and RTL lists is sound, because every Inline mapping is one name to one name, so both lists keep identical lengths and indices.
Extended reasoning...
The change is confined to src/css/properties/transition.rs (expand_properties, get_transitions, a new repeat_for_names) plus tests, with no security-sensitive surface. Two confirmed findings are posted inline; this note records only the extra index and skip-step checks ruled out from the logical-property table.
Findings marked 🟡 are optional suggestions and need no follow-up push.
`flush` writes the `transition-duration`, `transition-delay` and `transition-timing-function` longhands as the rule wrote them again, as on main. Only the `transition` shorthand gives each expanded name the values of its entry. `get_transitions` asserts that the record has one index per property.
|
This PR is back to the pairing of the @Jarred-Sumner, one question before more work goes into this. #47025 does not land changes that are right but for input nobody writes. This PR is in that group. It changes 0 of 4,313 real stylesheets (3,490 of them are published files of 112 npm packages). daisyUI has the shape,
|
There was a problem hiding this comment.
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 the two things the new sources record depends on in release builds, where the debug_assert! compiles out: the unchecked sources[index] in get_transitions is covered for both direction lists because every Inline pair in get_logical_properties has equal LTR/RTL lengths and every arm of expand_properties applies the same-length edit to both lists, and the lazy extend(0..source) on first materialization is sound because sources only stays None while every earlier entry produced exactly one name. The i += block.len() change in the Block arm is behavior-preserving: none of the block target names take vendor prefixes or a -webkit- mask twin, so the skipped revisit was a no-op.
Extended reasoning...
The latest push narrows the change to the transition shorthand path in src/css/properties/transition.rs (the longhand repeat_for_names rewrite and the Set-based merged-rule assertion from the previous version are gone) and adds prefix/minify cases plus a CLI/API bundler pair; no security-sensitive surface is involved. The remaining item is the test-coverage nit posted inline, and the author-documented right-to-wrong override case still rests on a judgment call a maintainer should weigh.
Problem
bun buildturnstransition: inset 1s, opacity 2sintotop 1s, bottom 2s, left 1s, right 2s, opacity 1s.mask-borderfails likewise.expand_properties(src/css/properties/transition.rs:526) makes one entry into several names.get_transitions(:468) matches the lists by position.Fix
expand_propertiesrecords, for each name, the index of its entry as written.get_transitionsreads the values at that index.flushwrites stay as on main.test/js/bun/css/css.test.ts(20 new tests, 18 fail on main),test/bundler/css/mask-geometry-box.test.ts. Alsotest/js/bun/css/,test/bundler/css/,test/bundler/esbuild/css.test.ts.Background
insetbecomestop, bottom, left, right, andmask-bordergets-webkit-mask-box-image.linear()easing is not parsed (Notes).Downsides
transition-propertymeets one value per name..a{transition:inset 1s,opacity 2s} .a:hover{transition-property:color,opacity}gives hoveropacity1s (main: 2s). Notes has a second shape, which css: consolidate the open CSS parser fixes #47025 removes.inset, opacitywith1s, 2s), and the quadratic inserts.textgrows by 768 B.Notes
Origin. There is no user report. The fault was found while #38692 was reviewed. lightningcss 1.33.0 prints the same wrong pairs for every input in the Problem section, so there is no upstream fix to port.
Spec. css-transitions-1,
transition-property: "The lists are matched up from the first value: excess values at the end are not used. If one of the other properties doesn't have enough comma-separated values to match the number of values oftransition-property, the user agent must calculate its used value by repeating the list of values until there are enough." And: "if one of the identifiers listed is a shorthand property, implementations must start transitions for all its longhand sub-properties ..., using the duration, delay, and timing function at the index corresponding to the shorthand."What changes.
transitionshorthand that the handler writes: from atransitiondeclaration, or from a block with all four longhands, which main folds into the shorthand. At the default targets,mask-border 1s, opacity 2swas-webkit-mask-box-image 1s, mask-border 2s, opacity 1sand is now-webkit-mask-box-image 1s, mask-border 1s, opacity 2s.transition-property,transition-duration,transition-delayandtransition-timing-functionlonghands print as on main.The one change to the loop. The
Blockarm now steps over all the names it wrote (i += block.len()), as lightningcss does. Before, it stepped by one and visited the rest again. None of those names is a logical property, takes a vendor prefix or has a-webkit-mask name, so the second visit did nothing.Reach. 4,313 stylesheets, each built with
Bun.buildat the default targets, plain and minified: 8,626 outputs, 0 differ between main and this PR (198 fail to build on both). 3,490 of the stylesheets are published files of 112 npm packages. The other 823 are from other software on the build machine (the WebKit Web Inspector and websites, the Rust documentation, bun's own tests). daisyUI has the one declaration in that set where the pairing matters, 12 times:transition:inset .3s linear(0, .5 10%, .9 30%, 1.05 50%, 1.1 75%, 1), background-color .2s ease-out.linear()is not parsed, so the declaration is written back whole. With an easing that is parsed, main givesbottomandrightthe values ofbackground-color. A test has that shape withcubic-bezier().Relation to #47025. #47025 changes the same
ifinflush: the handler writestransitiononly where the rule wrote one, or where a property gets a prefixed copy. On that base this PR is one argument,sources.as_deref(), in the twoget_transitionscalls, and themask-bordercase below is not folded any more. This PR does not build on it yet. It needs a rebase when #47025 lands.Cases that stay wrong.
flushwrites as longhands beside an expanded name.transition-property: inset, opacity; transition-duration: 1s, 2sstill gives five names beside1s, 2s, as on main. An earlier push of this PR repeated the values per name. That needs the lists of a second pass to line up as well (a merged rule, a prefixed property that is one entry and two names), and each repair made another merged output wrong.transitionshorthand: another rule, the!importantdeclarations of the same block.:lang()rules) still gets-webkit-mask-box-imageor a prefixed name again, as on main. In the shorthand the duplicate has the values of its entry.expand_propertiesstill shift the rest of the list. 128000 xinset 1stakes about 2.3 s on main and with this PR.transitionwithvar()orlinear()in it is not parsed, so no entry of it is expanded.Output that goes from right to wrong.
transition-propertyafter atransitionwith an expanded entry. Main's wrong pairs are the value lists as written, repeated. So a later property list meets the durations in the order the author wrote. A correct pairing changes that order. With one selector:.a{transition:inset 1s,opacity 2s}.a{transition-property:color,opacity}givescolor 1s, opacity 1s, main givescolor 1s, opacity 2s. The same happens in one block withtransition-property: color, opacity !important. A todo test holds the case. The order that keeps both right puts the names round-robin, and that breaksall .3s, transform .5s, where the later entry must stay later.mask-border,mask-compositeormask-modethat a later rule with the same selector completes:.a{transition-property:mask-border,opacity;transition-duration:1s,2s}.a{transition-delay:0s;transition-timing-function:ease}givesmask-border 2s, opacity 1s. Main givesmask-border 1s, opacity 2s, because its second pass writes-webkit-mask-box-imagetwice and the list has exactly two entries. With three entries main is wrong too. The first rule is printed with two names for one entry, and the second pass folds that list into a shorthand by entry. css: consolidate the open CSS parser fixes #47025 does not fold such a block.Shapes scored. A differential run of main against this PR: 1500 random lists of 1 to 5 entries from 13 property names (3 plain, 4 that expand, 6 that take a vendor prefix for some of the 4 target sets), each in 14 forms. Each printed name is scored against the values of its entry.
@media(3 forms)Every input of rows 3 and 4 with a name that goes from right to wrong has
mask-borderormask-composite(463 inputs). The names that stay wrong in rows 1 and 2 are in the longhand forms.Measurements. Release builds of this branch and of the same tree with the
src/diff reverse-applied, at the default targets unless a target is named.TransitionHandler::flush, main -> PR (gdb breakpoints on the mimalloc entry points, flushing thread only, two runs with equal counts): no transition 0 -> 0; 1 entry 0 -> 0; 2/4/8/9 entries 2/3/4/5 -> 2/3/4/5; longhands that expand nothing 0 -> 0;transition-property:mask-sizealone 0 -> 0;transform 1s,opacity 2s(Safari 6) 3 -> 3;mask-size 1s,opacity 2s3 -> 3; a shorthand plus a leftover-prefix longhand 2 -> 2;inset 1s,opacity 2s5 -> 6;mask-border 1s,opacity 2s4 -> 5; longhands withinsetormask-border1 -> 2.flushwith callees, main -> PR (gdbstepi, two runs with equal counts): no transition 18 -> 18; 1 entry 682 -> 718; longhands that expand nothing 368 -> 406;transition-property:mask-sizealone 373 -> 398; longhandsmask-size,opacitywith1s,2s473 -> 511. A row that frees memory differs by about 100 between runs, which is more than the change. One run: 2 entries 1336 -> 1379, 4 entries 2268 -> 2331, 8 entries 3983 -> 4086. Another run: 1336 -> 1471, 2268 -> 2228, 3983 -> 3983.text+768 B (size). Transition handler symbols 13084 B -> 13801 B (nm -S):flush5583 -> 5693,get_transitions985 -> 1012, the newrecord_names580.bloatyis not installed on the build machine.Reviews. The first version of this change was a rewrite of the expansion (branch
robobun/5df6e060/css-transition-lower-with-values, +381 source lines). A self-review found it too large for the evidence. The second version is this one. The first review on this PR led to a push that also repeated the longhand value lists per name (+32 lines). The second review showed that a prefixed property then got one value for two names. A version with one value per printed name (+47 lines) was built and measured, and not pushed: a second self-review, and the rule that #47025 states for the open CSS pull requests, led back to the shorthand. No published stylesheet has the input of the longhand half, and it added merged outputs that go from right to wrong. Not done: an issue that tracks the cases that stay wrong (they are listed above), and a report to lightningcss.Another shape, if no pairing code is wanted. Stop the expansion of transition entries: remove the multi-name rows of
get_logical_propertiesand the-webkit-mask inserts inexpand_properties, about 100 lines.transition: inset 1sthen stays as written, and an old browser that does not knowinsetruns no transition for it.