Skip to content

Commit 96c4da7

Browse files
fix: prevent style={undefined} from destroying computed styles
1 parent 786f305 commit 96c4da7

File tree

2 files changed

+204
-10
lines changed

2 files changed

+204
-10
lines changed

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

Lines changed: 177 additions & 8 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; }`);
@@ -11,8 +21,8 @@ test("className with inline style props should coexist when different properties
1121
).getByTestId(testID);
1222

1323
// Both className and style props should be applied as array
14-
expect(component.props.style).toEqual([
15-
{ color: "#f00" }, // Changed from "red" to "#f00"
24+
expect(component.props.style).toStrictEqual([
25+
{ color: "#f00" },
1626
{ fontSize: 16 },
1727
]);
1828
});
@@ -24,8 +34,8 @@ test("className with inline style props should favor inline when same property",
2434
<Text testID={testID} className="text-red" style={{ color: "blue" }} />,
2535
).getByTestId(testID);
2636

27-
// When same property exists, inline style should win (not array)
28-
expect(component.props.style).toEqual({ color: "blue" });
37+
// When same property exists, inline style should win
38+
expect(component.props.style).toStrictEqual({ color: "blue" });
2939
});
3040

3141
test("only className should not create array", () => {
@@ -36,7 +46,7 @@ test("only className should not create array", () => {
3646
).getByTestId(testID);
3747

3848
// Only className should be a flat object
39-
expect(component.props.style).toEqual({ color: "#f00" }); // Changed from "red" to "#f00"
49+
expect(component.props.style).toStrictEqual({ color: "#f00" });
4050
});
4151

4252
test("only inline style should not create array", () => {
@@ -45,7 +55,7 @@ test("only inline style should not create array", () => {
4555
).getByTestId(testID);
4656

4757
// Only inline style should be a flat object
48-
expect(component.props.style).toEqual({ color: "blue" });
58+
expect(component.props.style).toStrictEqual({ color: "blue" });
4959
});
5060

5161
test("important should overwrite the inline style", () => {
@@ -55,7 +65,7 @@ test("important should overwrite the inline style", () => {
5565
<Text testID={testID} className="text-red!" style={{ color: "blue" }} />,
5666
).getByTestId(testID);
5767

58-
expect(component.props.style).toEqual({ color: "#f00" });
68+
expect(component.props.style).toStrictEqual({ color: "#f00" });
5969
});
6070

6171
test("View with multiple className properties where inline style takes precedence", () => {
@@ -75,7 +85,7 @@ test("View with multiple className properties where inline style takes precedenc
7585

7686
// Inline style should override paddingRight from px-4 class
7787
// Other className styles should be preserved in array
78-
expect(component.props.style).toEqual([
88+
expect(component.props.style).toStrictEqual([
7989
{
8090
paddingLeft: 16,
8191
paddingRight: 16,
@@ -88,3 +98,162 @@ 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).toStrictEqual({ 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).toStrictEqual({ 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).toStrictEqual({
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).toStrictEqual({
166+
padding: 10,
167+
});
168+
});
169+
170+
test("ScrollView: contentContainerClassName without contentContainerStyle", () => {
171+
registerCSS(`.bg-green { background-color: green; }`);
172+
173+
const component = render(
174+
<ScrollView testID={testID} contentContainerClassName="bg-green" />,
175+
).getByTestId(testID);
176+
177+
expect(component.props.contentContainerStyle).toStrictEqual({
178+
backgroundColor: "#008000",
179+
});
180+
});
181+
182+
// Path B: FlatList with columnWrapperClassName (another non-"style" array target)
183+
test("FlatList: contentContainerClassName with contentContainerStyle={undefined}", () => {
184+
registerCSS(`.p-4 { padding: 16px; }`);
185+
186+
const component = render(
187+
<FlatList
188+
testID={testID}
189+
data={[]}
190+
renderItem={() => null}
191+
contentContainerClassName="p-4"
192+
contentContainerStyle={undefined}
193+
/>,
194+
).getByTestId(testID);
195+
196+
expect(component.props.contentContainerStyle).toStrictEqual({
197+
padding: 16,
198+
});
199+
});
200+
201+
// Path B: custom styled() with string target (e.g. { className: { target: "style" } })
202+
test("custom styled() with string target: style={undefined} preserves styles", () => {
203+
registerCSS(`.bg-purple { background-color: purple; }`);
204+
205+
const mapping: StyledConfiguration<typeof RNView> = {
206+
className: {
207+
target: "style",
208+
},
209+
};
210+
211+
const StyledView = copyComponentProperties(
212+
RNView,
213+
(
214+
props: StyledProps<React.ComponentProps<typeof RNView>, typeof mapping>,
215+
) => {
216+
return useCssElement(RNView, props, mapping);
217+
},
218+
);
219+
220+
const component = render(
221+
<StyledView testID={testID} className="bg-purple" style={undefined} />,
222+
).getByTestId(testID);
223+
224+
expect(component.props.style).toStrictEqual({ backgroundColor: "#800080" });
225+
});
226+
227+
// Real-world: optional style prop forwarding
228+
test("optional style prop forwarding preserves className styles", () => {
229+
registerCSS(`
230+
.p-4 { padding: 16px; }
231+
.bg-white { background-color: white; }
232+
`);
233+
234+
// Simulates a reusable component that forwards optional contentContainerStyle
235+
function MyScrollView({
236+
contentContainerStyle,
237+
}: {
238+
contentContainerStyle?: React.ComponentProps<
239+
typeof ScrollView
240+
>["contentContainerStyle"];
241+
}) {
242+
return (
243+
<ScrollView
244+
testID={testID}
245+
contentContainerClassName="p-4 bg-white"
246+
contentContainerStyle={contentContainerStyle}
247+
/>
248+
);
249+
}
250+
251+
// Called without contentContainerStyle — implicitly undefined
252+
const component = render(<MyScrollView />).getByTestId(testID);
253+
254+
expect(component.props.contentContainerStyle).toStrictEqual({
255+
padding: 16,
256+
backgroundColor: "#fff",
257+
});
258+
});
259+
});

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)