Skip to content

Commit d3ecc35

Browse files
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.
1 parent 99859a4 commit d3ecc35

File tree

2 files changed

+157
-8
lines changed

2 files changed

+157
-8
lines changed

src/__tests__/native/className-with-style.test.tsx

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ describe("style={undefined} should not destroy computed className styles", () =>
179179
});
180180
});
181181

182-
// Path B: FlatList with columnWrapperClassName (another non-"style" array target)
182+
// Path B: FlatList with contentContainerClassName (another non-"style" array target)
183183
test("FlatList: contentContainerClassName with contentContainerStyle={undefined}", () => {
184184
registerCSS(`.p-4 { padding: 16px; }`);
185185

@@ -257,3 +257,124 @@ describe("style={undefined} should not destroy computed className styles", () =>
257257
});
258258
});
259259
});
260+
261+
/**
262+
* Tests for style={{}} (empty object) not destroying computed className styles.
263+
*
264+
* An empty style object has no properties to apply, so it should not overwrite
265+
* computed className styles. The ["style"] target path (Path A) handles this via
266+
* filterCssVariables({}) returning undefined. These tests cover Path B (non-"style"
267+
* array targets) which previously used mergeDefinedProps that copied empty objects.
268+
*
269+
* Related: https://github.com/nativewind/react-native-css/issues/239
270+
*/
271+
describe("style={{}} should not destroy computed className styles", () => {
272+
// Path A: config.target = ["style"] — already handled by filterCssVariables
273+
test("View: className with style={{}}", () => {
274+
registerCSS(`.text-red { color: red; }`);
275+
276+
const component = render(
277+
<Text testID={testID} className="text-red" style={{}} />,
278+
).getByTestId(testID);
279+
280+
expect(component.props.style).toStrictEqual({ color: "#f00" });
281+
});
282+
283+
// Path B: config.target = ["contentContainerStyle"] — fixed by isEmptyPlainObject check
284+
test("ScrollView: contentContainerClassName with contentContainerStyle={{}}", () => {
285+
registerCSS(`.bg-green { background-color: green; }`);
286+
287+
const component = render(
288+
<ScrollView
289+
testID={testID}
290+
contentContainerClassName="bg-green"
291+
contentContainerStyle={{}}
292+
/>,
293+
).getByTestId(testID);
294+
295+
expect(component.props.contentContainerStyle).toStrictEqual({
296+
backgroundColor: "#008000",
297+
});
298+
});
299+
300+
// Path B: FlatList with contentContainerClassName (another non-"style" array target)
301+
test("FlatList: contentContainerClassName with contentContainerStyle={{}}", () => {
302+
registerCSS(`.p-4 { padding: 16px; }`);
303+
304+
const component = render(
305+
<FlatList
306+
testID={testID}
307+
data={[]}
308+
renderItem={() => null}
309+
contentContainerClassName="p-4"
310+
contentContainerStyle={{}}
311+
/>,
312+
).getByTestId(testID);
313+
314+
expect(component.props.contentContainerStyle).toStrictEqual({
315+
padding: 16,
316+
});
317+
});
318+
319+
// Path B: custom styled() with string target
320+
test("custom styled() with string target: style={{}} preserves styles", () => {
321+
registerCSS(`.bg-purple { background-color: purple; }`);
322+
323+
const mapping: StyledConfiguration<typeof RNView> = {
324+
className: {
325+
target: "style",
326+
},
327+
};
328+
329+
const StyledView = copyComponentProperties(
330+
RNView,
331+
(
332+
props: StyledProps<React.ComponentProps<typeof RNView>, typeof mapping>,
333+
) => {
334+
return useCssElement(RNView, props, mapping);
335+
},
336+
);
337+
338+
const component = render(
339+
<StyledView testID={testID} className="bg-purple" style={{}} />,
340+
).getByTestId(testID);
341+
342+
expect(component.props.style).toStrictEqual({ backgroundColor: "#800080" });
343+
});
344+
345+
// Non-empty contentContainerStyle override is covered by
346+
// "ScrollView: contentContainerClassName preserves styles with valid contentContainerStyle"
347+
// in the style={undefined} describe block above.
348+
349+
// Real-world: component with default empty object
350+
test("optional prop with empty object default preserves className styles", () => {
351+
registerCSS(`
352+
.p-4 { padding: 16px; }
353+
.bg-white { background-color: white; }
354+
`);
355+
356+
function MyScrollView({
357+
contentContainerStyle = {},
358+
}: {
359+
contentContainerStyle?: React.ComponentProps<
360+
typeof ScrollView
361+
>["contentContainerStyle"];
362+
}) {
363+
return (
364+
<ScrollView
365+
testID={testID}
366+
contentContainerClassName="p-4 bg-white"
367+
contentContainerStyle={contentContainerStyle}
368+
/>
369+
);
370+
}
371+
372+
// Called without prop — default {} used
373+
const component = render(<MyScrollView />).getByTestId(testID);
374+
375+
expect(component.props.contentContainerStyle).toStrictEqual({
376+
padding: 16,
377+
backgroundColor: "#fff",
378+
});
379+
});
380+
});

src/native/styles/index.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -269,25 +269,53 @@ export function getStyledProps(
269269
}
270270

271271
/**
272-
* Merges two prop objects, skipping keys from `right` whose value is `undefined`.
272+
* Returns true if the value is an empty plain object with no own enumerable
273+
* string properties. Used to treat `style={{}}` as a no-op — an empty object
274+
* has no style properties to apply and should not overwrite computed className
275+
* styles. Consistent with how the `["style"]` target path handles this via
276+
* `filterCssVariables({})` returning `undefined`.
277+
*
278+
* @param value - The value to check
279+
* @returns true if value is a non-null, non-array object with zero own string keys
280+
*
281+
* @example
282+
* isEmptyPlainObject({}) // true
283+
* isEmptyPlainObject({ padding: 10 }) // false
284+
* isEmptyPlainObject(null) // false
285+
* isEmptyPlainObject([]) // false (arrays are not plain objects)
286+
*/
287+
function isEmptyPlainObject(value: unknown): boolean {
288+
return (
289+
typeof value === "object" &&
290+
value !== null &&
291+
!Array.isArray(value) &&
292+
Object.keys(value).length === 0
293+
);
294+
}
295+
296+
/**
297+
* Merges two prop objects, skipping keys from `right` whose value is `undefined`
298+
* or an empty plain object.
273299
*
274300
* `Object.assign({}, left, right)` copies all enumerable own properties from `right`,
275-
* including those with value `undefined`. When a component passes `style={undefined}`
276-
* or `contentContainerStyle={undefined}`, the computed NativeWind style from `left`
277-
* gets overwritten. This function prevents that by only copying defined values.
301+
* including those with value `undefined` or `{}`. When a component passes
302+
* `style={undefined}` or `contentContainerStyle={{}}`, the computed NativeWind
303+
* style from `left` gets overwritten. This function prevents that by only copying
304+
* values that carry meaningful style information.
278305
*
279306
* @param left - The computed NativeWind props (className-derived styles)
280307
* @param right - The inline props from the component (guaranteed non-null by caller; may contain undefined values)
281-
* @returns A new object with all properties from `left`, overridden only by defined values from `right`
308+
* @returns A new object with all properties from `left`, overridden only by meaningful values from `right`
282309
*/
283310
function mergeDefinedProps(
284311
left: Record<string, any> | undefined,
285312
right: Record<string, any>,
286313
) {
287314
const result = left ? { ...left } : {};
288315
for (const key in right) {
289-
if (right[key] !== undefined) {
290-
result[key] = right[key];
316+
const value = right[key];
317+
if (value !== undefined && !isEmptyPlainObject(value)) {
318+
result[key] = value;
291319
}
292320
}
293321
return result;

0 commit comments

Comments
 (0)