Fix #3101: Normalise unitless 0 in clipPath WAAPI keyframes#3709
Fix #3101: Normalise unitless 0 in clipPath WAAPI keyframes#3709mattgperry wants to merge 1 commit intomainfrom
Conversation
Chromium 134 intermittently produces janky `clip-path: inset(...)` animations when keyframes mix unitless `0` and `0px` lengths (e.g. `inset(0 0px 0 0px)` → `inset(0 120px 0 120px)`). Convert bare zero length tokens to `0px` before handing keyframes to `element.animate()` so every length carries the same explicit unit. Fixes #3101 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Greptile SummaryThis PR fixes a Chromium 134 regression where
Confidence Score: 4/5Safe to merge; the fix is narrowly scoped to The Cypress mid-animation check uses packages/framer-motion/cypress/integration/animate-clip-path.ts — the mid-animation Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["startWaapiAnimation(valueName, keyframes)"] --> B{"propsToNormalise.has(valueName)?"}
B -- No --> D["Return keyframes as-is"]
B -- Yes --> C["normaliseAcceleratedKeyframes()"]
C --> E{"Array.isArray(keyframes)?"}
E -- Yes --> F["map: string keyframes → addPxToZeros()"]
E -- No --> G{"typeof keyframes === 'string'?"}
G -- Yes --> H["addPxToZeros(keyframes)"]
G -- No --> I["Return keyframe as-is (null/number)"]
F --> J["element.animate(keyframeOptions, options)"]
H --> J
D --> J
I --> J
Reviews (1): Last reviewed commit: "Normalise unitless 0 to 0px in clipPath ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| .should(($el) => { | ||
| const el = $el[0] as HTMLElement | ||
| const clipPath = getComputedStyle(el).clipPath | ||
| // Extract numeric values from inset() string | ||
| const matches = clipPath.match(/-?\d+(?:\.\d+)?/g) | ||
| expect(matches, `clipPath was: ${clipPath}`).to.not.be.null | ||
| const numbers = matches!.map(Number) | ||
| // inset() resolves to 4 values [top, right, bottom, left]. | ||
| // For our animation top/bottom stay 0, left/right go 0 → 120. | ||
| // At ~50% progress, left and right should be roughly 60 (give a wide tolerance). | ||
| expect(numbers.length).to.be.at.least(2) | ||
| // Find the largest value — it should be a meaningful intermediate | ||
| // value (not 0 and not the final 120). | ||
| const max = Math.max(...numbers) | ||
| expect(max, `expected mid-animation value, got ${clipPath}`) | ||
| .to.be.greaterThan(20) | ||
| .and.lessThan(100) | ||
| }) |
There was a problem hiding this comment.
.should() instead of .then()
The project's style guide (CLAUDE.md) explicitly warns: "Use .then(), not .should(), for mid-animation measurements. cy.should() retries assertions until they pass or timeout — so it will keep retrying until the animation completes, masking bugs." Here, .should() at the 500 ms checkpoint will keep retrying the assertion. If the Chromium regression reappears (animation never reaches an intermediate value and skips to the final), Cypress will simply time out with a confusing timeout error rather than a clear assertion failure — the mid-animation check can never definitively distinguish "stuck at 0 then jumped to 120" from "slowly reached 120."
| function addPxToZeros(value: string): string { | ||
| return value.replace(unitlessZeroLength, "$10px") | ||
| } |
There was a problem hiding this comment.
$10px replacement string relies on ECMAScript fallback behaviour
With only one capture group, the replacement "$10px" works because ECMA-262's GetSubstitution algorithm first tries to treat $10 as capture group 10, fails (only 1 group exists), then falls back to $1 (group 1) plus the literal character 0. The end result is correct — (preceding char) + "0px" — but this is not immediately legible to someone reading the code. Using a function replacement removes any ambiguity.
| function addPxToZeros(value: string): string { | |
| return value.replace(unitlessZeroLength, "$10px") | |
| } | |
| function addPxToZeros(value: string): string { | |
| return value.replace(unitlessZeroLength, (_, prefix: string) => `${prefix}0px`) | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
clip-path: inset(...)keyframes that mix unitless0and0pxlengths (the user's reproduction toggles betweeninset(0 0px 0 0px)andinset(0 120px 0 120px)), producing janky / glitching animations.element.animate(), replace any bare0length token (0followed by whitespace, comma, or close-paren) with0pxso every length in the keyframe carries an explicit unit.clipPathfor now since that is what's reported, but the helper takes a property-name allow list so additional shape-style properties can be added if more reports surface.Cause
getComputedStyleandgetAnimatableNoneboth preserve whichever unit notation the user supplies, so the keyframe pair Motion hands to WAAPI can keep the user's mixed0/0pxform. In Chromium 134 that mixed form interpolates incorrectly. Normalising to0pxproduces a CSS-equivalent string that interpolates cleanly.Notes for review
normaliseAcceleratedKeyframesare the real regression guard.waapi/animate-style/animate-filter-blurtests pass on both React 18 and React 19.Fixes #3101
Test plan
yarn test(793 passed, 7 skipped)animate-clip-path.tson React 18 and React 19waapi.ts,animate-filter-blur.ts,animate-style.ts,waapi-interrupt-transform.tsregress🤖 Generated with Claude Code