fix(CSS): leave the second component at zero for single-argument translate and skew - #10385
Open
dennytosp wants to merge 7 commits into
Open
fix(CSS): leave the second component at zero for single-argument translate and skew#10385dennytosp wants to merge 7 commits into
dennytosp wants to merge 7 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughUpdated transform parsing so single-argument Merge Risk: ⚪ Minimal · up to Single-argument translate and skew transforms now preserve a zero value for the omitted Y component. No current merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
1 task
…slate and skew
`parseTranslate` and `parseSkew` fell back to `values[1] ?? values[0]`, copying
the single argument onto the Y axis:
processTransform('translate(25px)') -> [{ translateX: 25 }, { translateY: 25 }]
processTransform('skew(30deg)') -> [{ skewX: '30deg' }, { skewY: '30deg' }]
CSS Transforms Level 1 says the opposite for both: for `translate(tx)`, "if
<ty> is not provided, ty has zero value", and for `skew(ax)`, "if the second
parameter is not provided, it has a zero value". React Native's own parser
agrees — `processTransform` pads the list with a literal zero:
if (parsedArgs?.length === 1) {
parsedArgs.push(0);
}
so `translate(25px)` gives `[{ translate: [25, 0] }]` there.
The fallback looks like it was carried over from `parseScale`, where repeating
the argument is correct because `scale(s)` is uniform. It is not correct here:
`transform: 'translate(100px)'` moved the element 100px down as well as across.
Pass 0 instead. `parseTranslateY` accepts it as a number and `parseSkewY`
already maps a zero to `'0deg'`, so both keep emitting two entries and transform
lists still line up for interpolation. `parseScale` is left as it is.
dennytosp
force-pushed
the
fix/single-argument-translate-and-skew
branch
from
August 26, 2026 14:25
f3561df to
76e90fc
Compare
MatiPl01
added a commit
that referenced
this pull request
Aug 26, 2026
> [!NOTE] > This pull request was authored by AI on behalf of @dennytosp. ## Summary `processTransform` splits the string on `/\)\s*/`, so whitespace **after** each `)` is consumed, and `filter(Boolean)` drops the empty part a trailing one leaves behind. Nothing ever reaches the start of the string, so the first function keeps its padding: ```ts processTransform(' translate(25px) rotate(45deg)') // throws: [Reanimated] Invalid transform property: translate(25px) ``` `parseTransformProperty` splits that part on `/\(\s*/`, gets `' translate'` as the key, matches no `case`, returns `[]`, and `processTransform` turns the empty result into a thrown error. Only the first function in the string is affected — everything after a `)` is already fine. The same string without the two leading spaces parses without complaint, and React Native's parser accepts both: ``` RNCORE ' translate(25px) rotate(45deg)' -> [{"translate":[25,0]},{"rotate":"45deg"}] ``` CSS ignores leading whitespace in a property value, and a value assembled from a template literal or read out of a config file easily carries some — this is the kind of difference that produces a crash report with no obvious cause. ### Fix `value.trim()` before the split. Trimming the whole input rather than the parsed key also covers tabs and newlines, and leaves the error message intact for genuinely invalid input. ## Test plan Two cases added to the multi-transform table in `src/common/style/processors/__tests__/transform.test.ts` — one with leading and trailing spaces, one with newlines and indentation. Both **fail on `main`**: ```console $ git stash push src/common/style/processors/transform.ts $ yarn jest src/common/style/processors/__tests__/transform.test.ts ✕ parses translate(25, 25) scale(2) ✕ parses \n translate(25, 25)\n scale(2)\n Tests: 2 failed, 64 passed, 66 total ``` With the fix applied: ```console $ yarn jest Test Suites: 103 passed, 103 total Tests: 1558 passed, 1558 total ``` Every existing error case still throws — `translat(25, 25)`, `rotate(90)`, `scaleX()` and the rest are unaffected, since trimming only removes padding that was never part of a function name. `yarn eslint` and `oxfmt --check` are clean on both files. Touches the same file as #10385 but a different function; the two apply independently. ## Changelog - [x] I added an entry to the `Unpublished` section of each changed package's `CHANGELOG.md`, or this PR does not change `react-native-reanimated` or `react-native-worklets`. --------- Co-authored-by: Mateusz Łopaciński <lop.mateusz.2001@gmail.com>
…ranslate-and-skew # Conflicts: # packages/react-native-reanimated/CHANGELOG.md
…ranslate-and-skew
…ranslate-and-skew
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
2 participants
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.
Note
This pull request was authored by AI on behalf of @dennytosp.
Summary
parseTranslateandparseSkewfall back tovalues[1] ?? values[0], which copies the single argument onto the Y axis:CSS Transforms Level 1 says the opposite for both:
translate(tx)— "If<ty>is not provided, ty has zero value."skew(ax)— "If the second parameter is not provided, it has a zero value."React Native's own parser agrees.
Libraries/StyleSheet/processTransform.jspads the list with a literal zero:Confirmed by running it directly against this repo's installed React Native:
So
transform: 'translate(100px)'moves the element 100px down as well as across in Reanimated, and a CSS transition animating that value drags it diagonally.Why this reads as a copy-paste slip rather than a design choice
parseScaleuses the samevalues[1] ?? values[0]shape, and there it is correct —scale(s)is uniform per spec. ButparseScalealso has an explicitvalues.length === 1branch above it, so by the time that fallback runsvalues[1]is always defined and the?? values[0]is dead.parseTranslateandparseSkewhave no such branch, so they inherited a fallback that only made sense for scale.Fix
Pass
0as the missing second component. No guard changes were needed:parseTranslateYaccepts it as a number, andparseSkewYalready maps a zero to'0deg'(values[0] === 0 ? '0deg' : values[0]). Both still emit two entries, so transform lists keep the same length whether or not the second argument was written — which matters for interpolating between two transform strings.parseScaleis deliberately left alone.Test plan
This changes behavior that three existing assertions in
src/common/style/processors/__tests__/transform.test.tshad captured, so those three are updated rather than added to. They fail onmainagainst the spec-correct expectations:With the fix applied:
Two-argument forms,
scale(),translateX/Y(),skewX/Y(), the array input path, and every error case are untouched.yarn eslintandoxfmt --checkare clean on both files.If you would rather keep the current behavior for compatibility reasons, I am happy to close this — but then the two spec references above are worth a comment in the source, because the next reader will file the same report.
Changelog
Unpublishedsection of each changed package'sCHANGELOG.md, or this PR does not changereact-native-reanimatedorreact-native-worklets.