Skip to content

Commit efb6353

Browse files
fix: prevent style={undefined} from destroying computed styles
1 parent b0b35b1 commit efb6353

File tree

2 files changed

+192
-2
lines changed

2 files changed

+192
-2
lines changed

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

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
1+
import { View as RNView } from "react-native";
2+
13
import { render } from "@testing-library/react-native";
4+
import { copyComponentProperties } from "react-native-css/components/copyComponentProperties";
5+
import { FlatList } from "react-native-css/components/FlatList";
6+
import { ScrollView } from "react-native-css/components/ScrollView";
27
import { Text } from "react-native-css/components/Text";
38
import { View } from "react-native-css/components/View";
49
import { registerCSS, testID } from "react-native-css/jest";
10+
import {
11+
useCssElement,
12+
type StyledConfiguration,
13+
type StyledProps,
14+
} from "react-native-css/native";
515

616
test("className with inline style props should coexist when different properties", () => {
717
registerCSS(`.text-red { color: red; }`);
@@ -88,3 +98,158 @@ test("View with multiple className properties where inline style takes precedenc
8898
},
8999
]);
90100
});
101+
102+
/**
103+
* Tests for style={undefined} not destroying computed className styles.
104+
*
105+
* Object.assign({}, left, right) copies all enumerable own properties from right,
106+
* including those with value undefined. When a component passes style={undefined}
107+
* (common when forwarding optional style props), the computed NativeWind styles
108+
* from className are overwritten.
109+
*
110+
* PR #224 fixed the default ["style"] target path. These tests cover the remaining
111+
* paths: non-"style" array targets (e.g. ["contentContainerStyle"]) and string targets.
112+
*/
113+
describe("style={undefined} should not destroy computed className styles", () => {
114+
// Path A: config.target = ["style"] (fixed in PR #224)
115+
test("View: className with style={undefined}", () => {
116+
registerCSS(`.text-red { color: red; }`);
117+
118+
const component = render(
119+
<Text testID={testID} className="text-red" style={undefined} />,
120+
).getByTestId(testID);
121+
122+
expect(component.props.style).toEqual({ color: "#f00" });
123+
});
124+
125+
test("View: className with style={null}", () => {
126+
registerCSS(`.text-red { color: red; }`);
127+
128+
const component = render(
129+
<Text testID={testID} className="text-red" style={null} />,
130+
).getByTestId(testID);
131+
132+
expect(component.props.style).toEqual({ color: "#f00" });
133+
});
134+
135+
// Path B: config.target = ["contentContainerStyle"] (non-"style" array target)
136+
test("ScrollView: contentContainerClassName with contentContainerStyle={undefined}", () => {
137+
registerCSS(`.bg-green { background-color: green; }`);
138+
139+
const component = render(
140+
<ScrollView
141+
testID={testID}
142+
contentContainerClassName="bg-green"
143+
contentContainerStyle={undefined}
144+
/>,
145+
).getByTestId(testID);
146+
147+
expect(component.props.contentContainerStyle).toEqual({
148+
backgroundColor: "#008000",
149+
});
150+
});
151+
152+
test("ScrollView: contentContainerClassName preserves styles with valid contentContainerStyle", () => {
153+
registerCSS(`.bg-green { background-color: green; }`);
154+
155+
const component = render(
156+
<ScrollView
157+
testID={testID}
158+
contentContainerClassName="bg-green"
159+
contentContainerStyle={{ padding: 10 }}
160+
/>,
161+
).getByTestId(testID);
162+
163+
// Non-"style" targets: inline contentContainerStyle overwrites className styles
164+
// (array coexistence is only implemented for the ["style"] target path)
165+
expect(component.props.contentContainerStyle).toEqual({ padding: 10 });
166+
});
167+
168+
test("ScrollView: contentContainerClassName without contentContainerStyle", () => {
169+
registerCSS(`.bg-green { background-color: green; }`);
170+
171+
const component = render(
172+
<ScrollView testID={testID} contentContainerClassName="bg-green" />,
173+
).getByTestId(testID);
174+
175+
expect(component.props.contentContainerStyle).toEqual({
176+
backgroundColor: "#008000",
177+
});
178+
});
179+
180+
// Path B: FlatList with columnWrapperClassName (another non-"style" array target)
181+
test("FlatList: contentContainerClassName with contentContainerStyle={undefined}", () => {
182+
registerCSS(`.p-4 { padding: 16px; }`);
183+
184+
const component = render(
185+
<FlatList
186+
testID={testID}
187+
data={[]}
188+
renderItem={() => null}
189+
contentContainerClassName="p-4"
190+
contentContainerStyle={undefined}
191+
/>,
192+
).getByTestId(testID);
193+
194+
expect(component.props.contentContainerStyle).toEqual({ padding: 16 });
195+
});
196+
197+
// Path B: custom styled() with string target (e.g. { className: { target: "style" } })
198+
test("custom styled() with string target: style={undefined} preserves styles", () => {
199+
registerCSS(`.bg-purple { background-color: purple; }`);
200+
201+
const mapping: StyledConfiguration<typeof RNView> = {
202+
className: {
203+
target: "style",
204+
},
205+
};
206+
207+
const StyledView = copyComponentProperties(
208+
RNView,
209+
(
210+
props: StyledProps<React.ComponentProps<typeof RNView>, typeof mapping>,
211+
) => {
212+
return useCssElement(RNView, props, mapping);
213+
},
214+
);
215+
216+
const component = render(
217+
<StyledView testID={testID} className="bg-purple" style={undefined} />,
218+
).getByTestId(testID);
219+
220+
expect(component.props.style).toEqual({ backgroundColor: "#800080" });
221+
});
222+
223+
// Real-world: optional style prop forwarding
224+
test("optional style prop forwarding preserves className styles", () => {
225+
registerCSS(`
226+
.p-4 { padding: 16px; }
227+
.bg-white { background-color: white; }
228+
`);
229+
230+
// Simulates a reusable component that forwards optional contentContainerStyle
231+
function MyScrollView({
232+
contentContainerStyle,
233+
}: {
234+
contentContainerStyle?: React.ComponentProps<
235+
typeof ScrollView
236+
>["contentContainerStyle"];
237+
}) {
238+
return (
239+
<ScrollView
240+
testID={testID}
241+
contentContainerClassName="p-4 bg-white"
242+
contentContainerStyle={contentContainerStyle}
243+
/>
244+
);
245+
}
246+
247+
// Called without contentContainerStyle — implicitly undefined
248+
const component = render(<MyScrollView />).getByTestId(testID);
249+
250+
expect(component.props.contentContainerStyle).toEqual({
251+
padding: 16,
252+
backgroundColor: "#fff",
253+
});
254+
});
255+
});

src/native/styles/index.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,31 @@ export function getStyledProps(
268268
return result;
269269
}
270270

271+
/**
272+
* Merges two prop objects, skipping keys from `right` whose value is `undefined`.
273+
*
274+
* `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.
278+
*
279+
* @param left - The computed NativeWind props (className-derived styles)
280+
* @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`
282+
*/
283+
function mergeDefinedProps(
284+
left: Record<string, any> | undefined,
285+
right: Record<string, any>,
286+
) {
287+
const result = left ? { ...left } : {};
288+
for (const key in right) {
289+
if (right[key] !== undefined) {
290+
result[key] = right[key];
291+
}
292+
}
293+
return result;
294+
}
295+
271296
function deepMergeConfig(
272297
config: Config,
273298
left: Record<string, any> | undefined,
@@ -359,10 +384,10 @@ function deepMergeConfig(
359384
}
360385
}
361386
} else {
362-
result = Object.assign({}, left, right);
387+
result = mergeDefinedProps(left, right);
363388
}
364389
} else {
365-
result = Object.assign({}, left, right);
390+
result = mergeDefinedProps(left, right);
366391
}
367392

368393
if (

0 commit comments

Comments
 (0)