Skip to content

Commit 5d8680e

Browse files
committed
fix: preserve className styles on multi-config and non-style array targets
Two related bugs caused className-computed styles to be silently destroyed: 1. deepMergeConfig did not merge className + inline styles for length-1 array targets like ["contentContainerStyle"]. Inline values simply overwrote the className-computed value instead of producing a style array (the behavior already implemented for the ["style"] path). 2. getStyledProps iterates over multiple configs (e.g. ScrollView has className→style and contentContainerClassName→contentContainerStyle). Each iteration produced a full props object via Object.assign, so later iterations overwrote earlier iterations' correctly-merged target props. Fix (1) by adding a finalKey merge block for length-1 array targets in deepMergeConfig, mirroring the existing string-target merge logic. Fix (2) by saving each config iteration's computed target value and restoring them after the loop completes. Adds tests for both paths including ScrollView, FlatList, and custom styled() components with className + inline style combinations.
1 parent 7622a38 commit 5d8680e

File tree

2 files changed

+211
-5
lines changed

2 files changed

+211
-5
lines changed

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

Lines changed: 142 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,11 @@ describe("style={undefined} should not destroy computed className styles", () =>
160160
/>,
161161
).getByTestId(testID);
162162

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-
});
163+
// Both className and inline contentContainerStyle should coexist as array
164+
expect(component.props.contentContainerStyle).toStrictEqual([
165+
{ backgroundColor: "#008000" },
166+
{ padding: 10 },
167+
]);
168168
});
169169

170170
test("ScrollView: contentContainerClassName without contentContainerStyle", () => {
@@ -257,3 +257,140 @@ describe("style={undefined} should not destroy computed className styles", () =>
257257
});
258258
});
259259
});
260+
261+
/**
262+
* Tests for multi-config components (e.g. ScrollView with both className and
263+
* contentContainerClassName) where inline style on one target should not
264+
* destroy computed className styles on a different target.
265+
*
266+
* Bug: getStyledProps loops over configs, and each iteration calls deepMergeConfig
267+
* which produces a full props object via Object.assign({}, left, right). When a
268+
* later config iteration runs, it overwrites the correctly-merged target props
269+
* from earlier iterations.
270+
*
271+
* Example: ScrollView with className="bg-red" and style={{ paddingTop: 10 }}.
272+
* The first config (className→style) correctly merges both. The second config
273+
* (contentContainerClassName→contentContainerStyle) does Object.assign which
274+
* copies the inline style={{ paddingTop: 10 }} over the merged style, destroying
275+
* the backgroundColor from className.
276+
*/
277+
describe("multi-config: inline style should not destroy className styles on other targets", () => {
278+
test("ScrollView: className with style should preserve className styles", () => {
279+
registerCSS(`.bg-red { background-color: red; }`);
280+
281+
const component = render(
282+
<ScrollView
283+
testID={testID}
284+
className="bg-red"
285+
style={{ paddingTop: 10 }}
286+
/>,
287+
).getByTestId(testID);
288+
289+
// className backgroundColor should coexist with inline paddingTop
290+
expect(component.props.style).toStrictEqual([
291+
{ backgroundColor: "#f00" },
292+
{ paddingTop: 10 },
293+
]);
294+
});
295+
296+
test("ScrollView: className + contentContainerClassName + style preserves all", () => {
297+
registerCSS(`
298+
.bg-red { background-color: red; }
299+
.p-4 { padding: 16px; }
300+
`);
301+
302+
const component = render(
303+
<ScrollView
304+
testID={testID}
305+
className="bg-red"
306+
contentContainerClassName="p-4"
307+
style={{ paddingTop: 10 }}
308+
/>,
309+
).getByTestId(testID);
310+
311+
// className-derived style merged with inline style
312+
expect(component.props.style).toStrictEqual([
313+
{ backgroundColor: "#f00" },
314+
{ paddingTop: 10 },
315+
]);
316+
317+
// contentContainerClassName should be independently preserved
318+
expect(component.props.contentContainerStyle).toStrictEqual({
319+
padding: 16,
320+
});
321+
});
322+
323+
test("ScrollView: className + contentContainerClassName + both inline styles", () => {
324+
registerCSS(`
325+
.bg-red { background-color: red; }
326+
.p-4 { padding: 16px; }
327+
`);
328+
329+
const component = render(
330+
<ScrollView
331+
testID={testID}
332+
className="bg-red"
333+
contentContainerClassName="p-4"
334+
style={{ paddingTop: 10 }}
335+
contentContainerStyle={{ marginTop: 5 }}
336+
/>,
337+
).getByTestId(testID);
338+
339+
// Both targets should have merged className + inline styles
340+
expect(component.props.style).toStrictEqual([
341+
{ backgroundColor: "#f00" },
342+
{ paddingTop: 10 },
343+
]);
344+
345+
expect(component.props.contentContainerStyle).toStrictEqual([
346+
{ padding: 16 },
347+
{ marginTop: 5 },
348+
]);
349+
});
350+
351+
test("ScrollView: className without style still works (single-config path)", () => {
352+
registerCSS(`.bg-red { background-color: red; }`);
353+
354+
const component = render(
355+
<ScrollView testID={testID} className="bg-red" />,
356+
).getByTestId(testID);
357+
358+
expect(component.props.style).toStrictEqual({ backgroundColor: "#f00" });
359+
});
360+
361+
test("ScrollView: consumed className sources should be removed from props", () => {
362+
registerCSS(`.bg-red { background-color: red; }`);
363+
364+
const component = render(
365+
<ScrollView
366+
testID={testID}
367+
className="bg-red"
368+
contentContainerClassName="bg-red"
369+
style={{ paddingTop: 10 }}
370+
/>,
371+
).getByTestId(testID);
372+
373+
// className and contentContainerClassName should be consumed, not passed through
374+
expect(component.props.className).toBeUndefined();
375+
expect(component.props.contentContainerClassName).toBeUndefined();
376+
});
377+
378+
test("FlatList: contentContainerClassName with contentContainerStyle preserves both", () => {
379+
registerCSS(`.bg-blue { background-color: blue; }`);
380+
381+
const component = render(
382+
<FlatList
383+
testID={testID}
384+
data={[]}
385+
renderItem={() => null}
386+
contentContainerClassName="bg-blue"
387+
contentContainerStyle={{ height: 200 }}
388+
/>,
389+
).getByTestId(testID);
390+
391+
expect(component.props.contentContainerStyle).toStrictEqual([
392+
{ backgroundColor: "#00f" },
393+
{ height: 200 },
394+
]);
395+
});
396+
});

src/native/styles/index.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,14 @@ export function getStyledProps(
191191

192192
const styledProps = state.stylesObs?.get(state.styleEffect);
193193

194+
// When multiple configs exist (e.g. ScrollView with className→style and
195+
// contentContainerClassName→contentContainerStyle), each iteration of
196+
// deepMergeConfig produces a full props object via Object.assign({}, left, right).
197+
// Later iterations overwrite earlier ones' correctly-merged target props.
198+
// We save each iteration's target value and restore them after the loop.
199+
const computedTargets: Record<string, any> = {};
200+
const consumedSources: string[] = [];
201+
194202
for (const config of state.configs) {
195203
result = deepMergeConfig(
196204
config,
@@ -207,6 +215,21 @@ export function getStyledProps(
207215
);
208216
}
209217

218+
// Save the correctly-merged target prop from this iteration
219+
if (result && config.target) {
220+
const targetKey = Array.isArray(config.target)
221+
? config.target[config.target.length - 1]
222+
: config.target;
223+
if (targetKey && targetKey in result) {
224+
computedTargets[targetKey] = result[targetKey];
225+
}
226+
}
227+
228+
// Track consumed className sources for cleanup
229+
if (config.source !== config.target) {
230+
consumedSources.push(config.source);
231+
}
232+
210233
// Apply the handlers
211234
if (hoverFamily.has(state.ruleEffectGetter)) {
212235
result ??= {};
@@ -265,6 +288,17 @@ export function getStyledProps(
265288
}
266289
}
267290

291+
// Restore correctly-merged target props that may have been overwritten
292+
// by later config iterations' Object.assign({}, left, right)
293+
if (result) {
294+
for (const key in computedTargets) {
295+
result[key] = computedTargets[key];
296+
}
297+
for (const source of consumedSources) {
298+
delete result[source];
299+
}
300+
}
301+
268302
return result;
269303
}
270304

@@ -418,6 +452,41 @@ function deepMergeConfig(
418452
);
419453
}
420454

455+
// For length-1 array targets (e.g. ["contentContainerStyle"]), the loop
456+
// above runs 0 iterations. Merge the target prop so inline styles don't
457+
// silently overwrite className-computed styles (same as string target path).
458+
const finalKey = config.target[config.target.length - 1];
459+
if (finalKey && rightIsInline) {
460+
let rightValue = right?.[finalKey];
461+
if (rightValue !== undefined) {
462+
rightValue = filterCssVariables(rightValue);
463+
}
464+
if (rightValue === undefined || rightValue === null) {
465+
// Inline is empty — preserve className-computed value
466+
if (left && finalKey in left) {
467+
result[finalKey] = left[finalKey];
468+
}
469+
} else if (left && finalKey in left) {
470+
const leftValue = left[finalKey];
471+
const leftIsObj =
472+
typeof leftValue === "object" &&
473+
leftValue !== null &&
474+
!Array.isArray(leftValue);
475+
const rightIsObj =
476+
typeof rightValue === "object" &&
477+
rightValue !== null &&
478+
!Array.isArray(rightValue);
479+
if (leftIsObj && rightIsObj) {
480+
if (hasNonOverlappingProperties(leftValue, rightValue)) {
481+
result[finalKey] = [leftValue, rightValue];
482+
}
483+
// else: all left keys are in right, right overrides — already set by mergeDefinedProps
484+
} else {
485+
result[finalKey] = [leftValue, rightValue];
486+
}
487+
}
488+
}
489+
421490
return result;
422491
}
423492

0 commit comments

Comments
 (0)