Repository navigation
refactor(native): combine two values of a style prop in one place - #487
Draft
YevheniiKotyrlo wants to merge 7 commits into
Draft
YevheniiKotyrlo wants to merge 7 commits into
YevheniiKotyrlo wants to merge 7 commits into
Conversation
…tton family Gesture Handler's Pressable, its button family and PureNativeButton render GestureHandlerButton, a codegen'd native view the react-native rewrite never reaches, so className fell onto a view that declares no such prop. DrawerLayoutAndroid wraps react-native's component, which has no styled twin, and the Android GestureHandlerRootView renders a native view too. react-native-css/components/react-native-gesture-handler re-declares these with className mapped to the style each forwards, and the Metro resolver routes react-native-gesture-handler to it. GestureHandlerRootView keeps its flex: 1 default, which a resolved class would otherwise displace. The resolver anchors its own-package check on the layout segment, since it runs from src/metro under the source condition and from dist/<format>/metro once built; a fixed level count sent this package's own components back through the wrapper. Pressable calls a function style with its pressed state, so the className merge keeps a callback a callback on every merge path rather than merging it into an array, where it never runs. The inline branch of the style-target merge is removed: the length-one target merge after it always decides the inline style.
…er suite Jest's teardown reads every global, and expo's lazy fetch then loads the renderer through the suite's react-native mock. With nativeFabricUIManager still cleared, that throws, and on Linux Jest reports the suite as failed to run.
…s flex:1 default Gesture Handler's root falls back to `flex: 1` while no style is given, and the re-declaration kept that default under every class. Yoga reads a positive flex over an explicit or absent basis, so a root a class sizes to its content — `grow-0`, `shrink-0`, `basis-*`, `flex-none` — collapsed or stretched instead. The default now yields to a class that states its own flex sizing, and an inline style still displaces it, as Gesture Handler's own does.
… modules The resolver computed the package root and the own-module test the babel import plugin already computes, and its copy dropped the trailing path separator, so a directory beside the package whose name merely begins with dist or src counted as the package's own and its imports were left unrewritten. Both now import one isFromThisModule.
deepMergeConfig combined a computed style value with another one three ways: flattened for
important styles, an array collapsed to the inline value when it overrides every computed key
for a one-segment array target, and a plain array for a one-segment string target. A new kind
of style value had to be taught to all three, as the callback styles were.
combineStyleValues is the one combination: a callback composes, a value that overrides every key
of the other replaces it, and otherwise the two are flattened when this library computed both or
layered when one came from the element. A one-segment string target now collapses as its array
spelling does, so `{ target: "style" }` and `"style"` combine an inline style the same way.
YevheniiKotyrlo
marked this pull request as ready for review
October 16, 2026 08:36
YevheniiKotyrlo
marked this pull request as draft
October 20, 2026 20:37
…as it came filterCssVariables rebuilt every inline object, so an inline style lost the identity the layering keeps, and an Animated value inside one reached the component as a plain copy no animation drives. It now copies only what holds a variable, an undefined entry or a Symbol key, and leaves a class instance whole.
Contributor
Author
An inline
|
style |
its second entry | that entry's opacity |
|
|---|---|---|---|
main (eda2b286) |
[{ padding: 4 }, { opacity }] |
a copy of the inline style | { _value: 0.5, _offset: 0, … }, a plain object no animation drives |
| this PR | [{ padding: 4 }, { opacity }] |
the inline style itself | the same Animated.Value |
The same holds for contentContainerStyle, and for both spellings of each target, as className-with-style.test.tsx asserts.
Re-render cost — main against this PR
Each case re-renders 3000 times after 200 of warm-up, in its own jest process after a warm-up case, and the sides alternate over ten rounds after one of warm-up. µs per re-render, median and interquartile range:
| case | main (eda2b286) |
this PR | per-round difference |
|---|---|---|---|
| a class style alone | 49.0 (40.9–69.8) | 44.9 (40.8–49.1) | −5.4 (−14.5 to 0.5) |
| a class style beside an inline style | 46.0 (42.8–53.3) | 45.1 (42.3–74.5) | +0.8 (−5.4 to 29.2) |
a class style beside an inline Animated.Value |
58.4 (53.1–67.8) | 54.1 (51.0–59.2) | −1.5 (−6.1 to 10.5) |
| a Pressable's class style beside a style function | 61.0 (54.1–79.5) | 61.9 (54.8–79.1) | +1.8 (−1.7 to 23.3) |
Other work on the machine widened every spread; a class style alone, which neither the combination nor the filter reaches, puts the noise floor near 10 µs, and no difference passes it.
Windows 11 Pro 10.0.26200, AMD Ryzen 9 9950X3D, Node 26.8.1, jest 29.7.0.
This branch has not been deployed
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
deepMergeConfigcombines a computed style value with another one in three places, three ways:"style","contentContainerStyle"): the inline value alone when it overrides every computed key, otherwise both in an array;{ target: "style" }): always both in an array.So
{ className: "style" }and{ className: { target: "style" } }, two spellings of one mapping, combinestyle={{ backgroundColor: "blue" }}overbg-reddifferently: the first gives{ backgroundColor: "blue" }, the second[{ backgroundColor: "#f00" }, { backgroundColor: "blue" }]. And a new kind of style value has to be handled in all three places, as #416 does for a Pressable's callback style.Before any of them,
filterCssVariablesrebuilds every inline object key by key, so an inline style never reaches the component as the object it was, and anAnimated.Valueinside one arrives as{ _value: 0.5, _offset: 0, … }, which no animation drives.Solution
combineStyleValues(left, right, layering)is the one combination: a callback composes with the other value, a value that overrides every key of the other replaces it, and otherwise the two are flattened when this library computed both (important styles) or layered when one came from the element, since an inline style object can carry an identity a copy would lose. The three sites call it, so the string target collapses as its array spelling does; the important and array-target paths combine as before.filterCssVariablescopies only what holds a CSS variable, anundefinedentry or a Symbol key, and leaves a class instance whole, so an inline style that holds none reaches the component as it came.Tests
src/__tests__/native/className-with-style.test.tsx: both spellings of a style target, with an inline style that sets every property the class sets (it replaces the class's value) and one that sets other properties (the two are layered). Onmainthe object form returns both values in an array where the shorthand returns the inline value alone. ForstyleandcontentContainerStylein both spellings, an inline style is layered as the same object and anAnimated.Valuein it stays itself; onmainall eight fail. The existing suites hold the other two paths, the CSS variable filtering, and #416's callback styles.Verification
Every
ci.ymljob passes on this head merged intomainwith #477's harness fix: lint, typecheck, the tests (1519 passed), both builds, the web export, nothing unstaged, commitlint, and the iOS dev build on macOS 27.0.1 with Xcode 27.0, where CI pins 26.6.A re-render costs the same within the noise. The runs, and the
Animated.Valuesnippet onmainand here, are in this comment.Merge order