diff --git a/src/__tests__/native/className-with-style.test.tsx b/src/__tests__/native/className-with-style.test.tsx index e5144146..e4e003b5 100644 --- a/src/__tests__/native/className-with-style.test.tsx +++ b/src/__tests__/native/className-with-style.test.tsx @@ -1,7 +1,17 @@ +import { View as RNView } from "react-native"; + import { render } from "@testing-library/react-native"; +import { copyComponentProperties } from "react-native-css/components/copyComponentProperties"; +import { FlatList } from "react-native-css/components/FlatList"; +import { ScrollView } from "react-native-css/components/ScrollView"; import { Text } from "react-native-css/components/Text"; import { View } from "react-native-css/components/View"; import { registerCSS, testID } from "react-native-css/jest"; +import { + useCssElement, + type StyledConfiguration, + type StyledProps, +} from "react-native-css/native"; test("className with inline style props should coexist when different properties", () => { registerCSS(`.text-red { color: red; }`); @@ -88,3 +98,158 @@ test("View with multiple className properties where inline style takes precedenc }, ]); }); + +/** + * Tests for style={undefined} not destroying computed className styles. + * + * Object.assign({}, left, right) copies all enumerable own properties from right, + * including those with value undefined. When a component passes style={undefined} + * (common when forwarding optional style props), the computed NativeWind styles + * from className are overwritten. + * + * PR #224 fixed the default ["style"] target path. These tests cover the remaining + * paths: non-"style" array targets (e.g. ["contentContainerStyle"]) and string targets. + */ +describe("style={undefined} should not destroy computed className styles", () => { + // Path A: config.target = ["style"] (fixed in PR #224) + test("View: className with style={undefined}", () => { + registerCSS(`.text-red { color: red; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toEqual({ color: "#f00" }); + }); + + test("View: className with style={null}", () => { + registerCSS(`.text-red { color: red; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.style).toEqual({ color: "#f00" }); + }); + + // Path B: config.target = ["contentContainerStyle"] (non-"style" array target) + test("ScrollView: contentContainerClassName with contentContainerStyle={undefined}", () => { + registerCSS(`.bg-green { background-color: green; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.contentContainerStyle).toEqual({ + backgroundColor: "#008000", + }); + }); + + test("ScrollView: contentContainerClassName preserves styles with valid contentContainerStyle", () => { + registerCSS(`.bg-green { background-color: green; }`); + + const component = render( + , + ).getByTestId(testID); + + // Non-"style" targets: inline contentContainerStyle overwrites className styles + // (array coexistence is only implemented for the ["style"] target path) + expect(component.props.contentContainerStyle).toEqual({ padding: 10 }); + }); + + test("ScrollView: contentContainerClassName without contentContainerStyle", () => { + registerCSS(`.bg-green { background-color: green; }`); + + const component = render( + , + ).getByTestId(testID); + + expect(component.props.contentContainerStyle).toEqual({ + backgroundColor: "#008000", + }); + }); + + // Path B: FlatList with columnWrapperClassName (another non-"style" array target) + test("FlatList: contentContainerClassName with contentContainerStyle={undefined}", () => { + registerCSS(`.p-4 { padding: 16px; }`); + + const component = render( + null} + contentContainerClassName="p-4" + contentContainerStyle={undefined} + />, + ).getByTestId(testID); + + expect(component.props.contentContainerStyle).toEqual({ padding: 16 }); + }); + + // Path B: custom styled() with string target (e.g. { className: { target: "style" } }) + test("custom styled() with string target: style={undefined} 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).toEqual({ backgroundColor: "#800080" }); + }); + + // Real-world: optional style prop forwarding + test("optional style prop forwarding preserves className styles", () => { + registerCSS(` + .p-4 { padding: 16px; } + .bg-white { background-color: white; } + `); + + // Simulates a reusable component that forwards optional contentContainerStyle + function MyScrollView({ + contentContainerStyle, + }: { + contentContainerStyle?: React.ComponentProps< + typeof ScrollView + >["contentContainerStyle"]; + }) { + return ( + + ); + } + + // Called without contentContainerStyle — implicitly undefined + const component = render().getByTestId(testID); + + expect(component.props.contentContainerStyle).toEqual({ + padding: 16, + backgroundColor: "#fff", + }); + }); +}); diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index b10e50bd..63795863 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -268,6 +268,31 @@ export function getStyledProps( return result; } +/** + * Merges two prop objects, skipping keys from `right` whose value is `undefined`. + * + * `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. + * + * @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` + */ +function mergeDefinedProps( + left: Record | undefined, + right: Record, +) { + const result = left ? { ...left } : {}; + for (const key in right) { + if (right[key] !== undefined) { + result[key] = right[key]; + } + } + return result; +} + function deepMergeConfig( config: Config, left: Record | undefined, @@ -359,10 +384,10 @@ function deepMergeConfig( } } } else { - result = Object.assign({}, left, right); + result = mergeDefinedProps(left, right); } } else { - result = Object.assign({}, left, right); + result = mergeDefinedProps(left, right); } if (