From a432ef17cfb5e22bc60a2184277c0a2ecc6fa773 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Fri, 27 Mar 2026 15:25:30 +0200 Subject: [PATCH] fix: prevent style={{}} from destroying computed styles in non-["style"] targets When a component passes an empty style object (e.g., contentContainerStyle={{}}) to a non-["style"] target, mergeDefinedProps copies the empty object over the computed className styles, destroying them. This is the same class of bug as style={undefined} (fixed in PR #291) but for empty objects. The ["style"] target path (Path A) already handles this correctly via filterCssVariables({}) returning undefined. This fix extends mergeDefinedProps with an isEmptyPlainObject check to skip empty objects on paths B and C, matching Path A's behavior. Common real-world trigger: components with default empty object parameters ({ contentContainerStyle = {} }) or explicit style={{}} props. --- .../native/className-with-style.test.tsx | 123 +++++++++++++++++- src/native/styles/index.ts | 33 ++++- 2 files changed, 148 insertions(+), 8 deletions(-) diff --git a/src/__tests__/native/className-with-style.test.tsx b/src/__tests__/native/className-with-style.test.tsx index e8e7bc4..c27875f 100644 --- a/src/__tests__/native/className-with-style.test.tsx +++ b/src/__tests__/native/className-with-style.test.tsx @@ -179,7 +179,7 @@ describe("style={undefined} should not destroy computed className styles", () => }); }); - // Path B: FlatList with columnWrapperClassName (another non-"style" array target) + // Path B: FlatList with contentContainerClassName (another non-"style" array target) test("FlatList: contentContainerClassName with contentContainerStyle={undefined}", () => { registerCSS(`.p-4 { padding: 16px; }`); @@ -257,3 +257,124 @@ describe("style={undefined} should not destroy computed className styles", () => }); }); }); + +/** + * Tests for style={{}} (empty object) not destroying computed className styles. + * + * An empty style object has no properties to apply, so it should not overwrite + * computed className styles. The ["style"] target path (Path A) handles this via + * filterCssVariables({}) returning undefined. These tests cover Path B (non-"style" + * array targets) which previously used mergeDefinedProps that copied empty objects. + * + * Related: https://github.com/nativewind/react-native-css/issues/239 + */ +describe("style={{}} should not destroy computed className styles", () => { + // Path A: config.target = ["style"] — already handled by filterCssVariables + test("View: className with style={{}}", () => { + registerCSS(`.text-red { color: red; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toStrictEqual({ color: "#f00" }); + }); + + // Path B: config.target = ["contentContainerStyle"] — fixed by isEmptyPlainObject check + test("ScrollView: contentContainerClassName with contentContainerStyle={{}}", () => { + registerCSS(`.bg-green { background-color: green; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.contentContainerStyle).toStrictEqual({ + backgroundColor: "#008000", + }); + }); + + // Path B: FlatList with contentContainerClassName (another non-"style" array target) + test("FlatList: contentContainerClassName with contentContainerStyle={{}}", () => { + registerCSS(`.p-4 { padding: 16px; }`); + + const component = render( + null} + contentContainerClassName="p-4" + contentContainerStyle={{}} + />, + ).getByTestId(testID); + + expect(component.props.contentContainerStyle).toStrictEqual({ + padding: 16, + }); + }); + + // Path B: custom styled() with string target + test("custom styled() with string target: style={{}} preserves styles", () => { + registerCSS(`.bg-purple { background-color: purple; }`); + + const mapping: StyledConfiguration = { + className: { + target: "style", + }, + }; + + const StyledView = copyComponentProperties( + RNView, + ( + props: StyledProps, typeof mapping>, + ) => { + return useCssElement(RNView, props, mapping); + }, + ); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toStrictEqual({ backgroundColor: "#800080" }); + }); + + // Non-empty contentContainerStyle override is covered by + // "ScrollView: contentContainerClassName preserves styles with valid contentContainerStyle" + // in the style={undefined} describe block above. + + // Real-world: component with default empty object + test("optional prop with empty object default preserves className styles", () => { + registerCSS(` + .p-4 { padding: 16px; } + .bg-white { background-color: white; } + `); + + function MyScrollView({ + contentContainerStyle = {}, + }: { + contentContainerStyle?: React.ComponentProps< + typeof ScrollView + >["contentContainerStyle"]; + }) { + return ( + + ); + } + + // Called without prop — default {} used + const component = render().getByTestId(testID); + + expect(component.props.contentContainerStyle).toStrictEqual({ + padding: 16, + backgroundColor: "#fff", + }); + }); +}); diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index 6379586..9ac5cc4 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -269,16 +269,23 @@ export function getStyledProps( } /** - * Merges two prop objects, skipping keys from `right` whose value is `undefined`. + * Merges two prop objects, skipping keys from `right` whose value is `undefined` + * or an empty plain object. * * `Object.assign({}, left, right)` copies all enumerable own properties from `right`, - * including those with value `undefined`. When a component passes `style={undefined}` - * or `contentContainerStyle={undefined}`, the computed NativeWind style from `left` - * gets overwritten. This function prevents that by only copying defined values. + * including those with value `undefined` or `{}`. When a component passes + * `style={undefined}` or `contentContainerStyle={{}}`, the computed NativeWind + * style from `left` gets overwritten. This function prevents that by only copying + * values that carry meaningful style information. + * + * Empty object detection uses inline `for...in` instead of a separate function or + * `Object.keys().length` — avoids both function call overhead and intermediate array + * allocation on a hot path (called per-prop per-render). Consistent with how the + * `["style"]` target path handles this via `filterCssVariables({})` returning `undefined`. * * @param left - The computed NativeWind props (className-derived styles) * @param right - The inline props from the component (guaranteed non-null by caller; may contain undefined values) - * @returns A new object with all properties from `left`, overridden only by defined values from `right` + * @returns A new object with all properties from `left`, overridden only by meaningful values from `right` */ function mergeDefinedProps( left: Record | undefined, @@ -286,8 +293,20 @@ function mergeDefinedProps( ) { const result = left ? { ...left } : {}; for (const key in right) { - if (right[key] !== undefined) { - result[key] = right[key]; + const value = right[key]; + if (value === undefined) continue; + // Non-object values (strings, numbers, booleans, null, arrays): always assign + if (typeof value !== "object" || value === null || Array.isArray(value)) { + result[key] = value; + continue; + } + // Plain object: assign only if non-empty. An empty style object (e.g., + // contentContainerStyle={{}}) has no properties to apply and should not + // overwrite computed className styles. The for...in loop only executes + // if the object has enumerable keys — empty objects are skipped. + for (const _ in value) { + result[key] = value; + break; } } return result;