From 5d8680e054ca2b391cbfa7372f700f0ebd217591 Mon Sep 17 00:00:00 2001 From: inv1x Date: Sat, 4 Apr 2026 15:42:12 +0300 Subject: [PATCH 1/2] fix: preserve className styles on multi-config and non-style array targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related bugs caused className-computed styles to be silently destroyed: 1. deepMergeConfig did not merge className + inline styles for length-1 array targets like ["contentContainerStyle"]. Inline values simply overwrote the className-computed value instead of producing a style array (the behavior already implemented for the ["style"] path). 2. getStyledProps iterates over multiple configs (e.g. ScrollView has className→style and contentContainerClassName→contentContainerStyle). Each iteration produced a full props object via Object.assign, so later iterations overwrote earlier iterations' correctly-merged target props. Fix (1) by adding a finalKey merge block for length-1 array targets in deepMergeConfig, mirroring the existing string-target merge logic. Fix (2) by saving each config iteration's computed target value and restoring them after the loop completes. Adds tests for both paths including ScrollView, FlatList, and custom styled() components with className + inline style combinations. --- .../native/className-with-style.test.tsx | 147 +++++++++++++++++- src/native/styles/index.ts | 69 ++++++++ 2 files changed, 211 insertions(+), 5 deletions(-) diff --git a/src/__tests__/native/className-with-style.test.tsx b/src/__tests__/native/className-with-style.test.tsx index e8e7bc4..a25a19e 100644 --- a/src/__tests__/native/className-with-style.test.tsx +++ b/src/__tests__/native/className-with-style.test.tsx @@ -160,11 +160,11 @@ describe("style={undefined} should not destroy computed className styles", () => />, ).getByTestId(testID); - // Non-"style" targets: inline contentContainerStyle overwrites className styles - // (array coexistence is only implemented for the ["style"] target path) - expect(component.props.contentContainerStyle).toStrictEqual({ - padding: 10, - }); + // Both className and inline contentContainerStyle should coexist as array + expect(component.props.contentContainerStyle).toStrictEqual([ + { backgroundColor: "#008000" }, + { padding: 10 }, + ]); }); test("ScrollView: contentContainerClassName without contentContainerStyle", () => { @@ -257,3 +257,140 @@ describe("style={undefined} should not destroy computed className styles", () => }); }); }); + +/** + * Tests for multi-config components (e.g. ScrollView with both className and + * contentContainerClassName) where inline style on one target should not + * destroy computed className styles on a different target. + * + * Bug: getStyledProps loops over configs, and each iteration calls deepMergeConfig + * which produces a full props object via Object.assign({}, left, right). When a + * later config iteration runs, it overwrites the correctly-merged target props + * from earlier iterations. + * + * Example: ScrollView with className="bg-red" and style={{ paddingTop: 10 }}. + * The first config (className→style) correctly merges both. The second config + * (contentContainerClassName→contentContainerStyle) does Object.assign which + * copies the inline style={{ paddingTop: 10 }} over the merged style, destroying + * the backgroundColor from className. + */ +describe("multi-config: inline style should not destroy className styles on other targets", () => { + test("ScrollView: className with style should preserve className styles", () => { + registerCSS(`.bg-red { background-color: red; }`); + + const component = render( + , + ).getByTestId(testID); + + // className backgroundColor should coexist with inline paddingTop + expect(component.props.style).toStrictEqual([ + { backgroundColor: "#f00" }, + { paddingTop: 10 }, + ]); + }); + + test("ScrollView: className + contentContainerClassName + style preserves all", () => { + registerCSS(` + .bg-red { background-color: red; } + .p-4 { padding: 16px; } + `); + + const component = render( + , + ).getByTestId(testID); + + // className-derived style merged with inline style + expect(component.props.style).toStrictEqual([ + { backgroundColor: "#f00" }, + { paddingTop: 10 }, + ]); + + // contentContainerClassName should be independently preserved + expect(component.props.contentContainerStyle).toStrictEqual({ + padding: 16, + }); + }); + + test("ScrollView: className + contentContainerClassName + both inline styles", () => { + registerCSS(` + .bg-red { background-color: red; } + .p-4 { padding: 16px; } + `); + + const component = render( + , + ).getByTestId(testID); + + // Both targets should have merged className + inline styles + expect(component.props.style).toStrictEqual([ + { backgroundColor: "#f00" }, + { paddingTop: 10 }, + ]); + + expect(component.props.contentContainerStyle).toStrictEqual([ + { padding: 16 }, + { marginTop: 5 }, + ]); + }); + + test("ScrollView: className without style still works (single-config path)", () => { + registerCSS(`.bg-red { background-color: red; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toStrictEqual({ backgroundColor: "#f00" }); + }); + + test("ScrollView: consumed className sources should be removed from props", () => { + registerCSS(`.bg-red { background-color: red; }`); + + const component = render( + , + ).getByTestId(testID); + + // className and contentContainerClassName should be consumed, not passed through + expect(component.props.className).toBeUndefined(); + expect(component.props.contentContainerClassName).toBeUndefined(); + }); + + test("FlatList: contentContainerClassName with contentContainerStyle preserves both", () => { + registerCSS(`.bg-blue { background-color: blue; }`); + + const component = render( + null} + contentContainerClassName="bg-blue" + contentContainerStyle={{ height: 200 }} + />, + ).getByTestId(testID); + + expect(component.props.contentContainerStyle).toStrictEqual([ + { backgroundColor: "#00f" }, + { height: 200 }, + ]); + }); +}); diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index 6379586..bece114 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -191,6 +191,14 @@ export function getStyledProps( const styledProps = state.stylesObs?.get(state.styleEffect); + // When multiple configs exist (e.g. ScrollView with className→style and + // contentContainerClassName→contentContainerStyle), each iteration of + // deepMergeConfig produces a full props object via Object.assign({}, left, right). + // Later iterations overwrite earlier ones' correctly-merged target props. + // We save each iteration's target value and restore them after the loop. + const computedTargets: Record = {}; + const consumedSources: string[] = []; + for (const config of state.configs) { result = deepMergeConfig( config, @@ -207,6 +215,21 @@ export function getStyledProps( ); } + // Save the correctly-merged target prop from this iteration + if (result && config.target) { + const targetKey = Array.isArray(config.target) + ? config.target[config.target.length - 1] + : config.target; + if (targetKey && targetKey in result) { + computedTargets[targetKey] = result[targetKey]; + } + } + + // Track consumed className sources for cleanup + if (config.source !== config.target) { + consumedSources.push(config.source); + } + // Apply the handlers if (hoverFamily.has(state.ruleEffectGetter)) { result ??= {}; @@ -265,6 +288,17 @@ export function getStyledProps( } } + // Restore correctly-merged target props that may have been overwritten + // by later config iterations' Object.assign({}, left, right) + if (result) { + for (const key in computedTargets) { + result[key] = computedTargets[key]; + } + for (const source of consumedSources) { + delete result[source]; + } + } + return result; } @@ -418,6 +452,41 @@ function deepMergeConfig( ); } + // For length-1 array targets (e.g. ["contentContainerStyle"]), the loop + // above runs 0 iterations. Merge the target prop so inline styles don't + // silently overwrite className-computed styles (same as string target path). + const finalKey = config.target[config.target.length - 1]; + if (finalKey && rightIsInline) { + let rightValue = right?.[finalKey]; + if (rightValue !== undefined) { + rightValue = filterCssVariables(rightValue); + } + if (rightValue === undefined || rightValue === null) { + // Inline is empty — preserve className-computed value + if (left && finalKey in left) { + result[finalKey] = left[finalKey]; + } + } else if (left && finalKey in left) { + const leftValue = left[finalKey]; + const leftIsObj = + typeof leftValue === "object" && + leftValue !== null && + !Array.isArray(leftValue); + const rightIsObj = + typeof rightValue === "object" && + rightValue !== null && + !Array.isArray(rightValue); + if (leftIsObj && rightIsObj) { + if (hasNonOverlappingProperties(leftValue, rightValue)) { + result[finalKey] = [leftValue, rightValue]; + } + // else: all left keys are in right, right overrides — already set by mergeDefinedProps + } else { + result[finalKey] = [leftValue, rightValue]; + } + } + } + return result; } From 9b4d9142836061de51c36ae722717ce0e6c76727 Mon Sep 17 00:00:00 2001 From: inv1x Date: Sat, 4 Apr 2026 16:21:23 +0300 Subject: [PATCH 2/2] fix: address review feedback on finalKey merge block - Guard finalKey block with config.target.length === 1 so it only runs for length-1 array targets, not nested paths handled by recursion - Explicitly write filtered rightValue in all code paths to prevent unfiltered VAR_SYMBOL objects from leaking into RN props - Add defensive comment about leaf-key storage limitation in getStyledProps --- src/native/styles/index.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index bece114..1112557 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -196,6 +196,12 @@ export function getStyledProps( // deepMergeConfig produces a full props object via Object.assign({}, left, right). // Later iterations overwrite earlier ones' correctly-merged target props. // We save each iteration's target value and restore them after the loop. + // + // Note: This uses the leaf key of config.target for storage/restoration. + // For nested array targets (length > 1), the leaf key is stored at the + // top level, which is correct because deepMergeConfig already builds the + // nested structure. If two configs ever share the same leaf key, the last + // one wins — but no built-in component mapping produces this scenario. const computedTargets: Record = {}; const consumedSources: string[] = []; @@ -456,15 +462,18 @@ function deepMergeConfig( // above runs 0 iterations. Merge the target prop so inline styles don't // silently overwrite className-computed styles (same as string target path). const finalKey = config.target[config.target.length - 1]; - if (finalKey && rightIsInline) { + if (config.target.length === 1 && finalKey && rightIsInline) { let rightValue = right?.[finalKey]; if (rightValue !== undefined) { rightValue = filterCssVariables(rightValue); } if (rightValue === undefined || rightValue === null) { - // Inline is empty — preserve className-computed value + // Inline is empty or fully filtered — preserve className-computed value if (left && finalKey in left) { result[finalKey] = left[finalKey]; + } else { + // No left value either — remove unfiltered inline value from result + delete result[finalKey]; } } else if (left && finalKey in left) { const leftValue = left[finalKey]; @@ -479,11 +488,16 @@ function deepMergeConfig( if (leftIsObj && rightIsObj) { if (hasNonOverlappingProperties(leftValue, rightValue)) { result[finalKey] = [leftValue, rightValue]; + } else { + // All left keys are in right — use filtered right value + result[finalKey] = rightValue; } - // else: all left keys are in right, right overrides — already set by mergeDefinedProps } else { result[finalKey] = [leftValue, rightValue]; } + } else { + // No left value — use filtered right value (not the unfiltered one from mergeDefinedProps) + result[finalKey] = rightValue; } }