Skip to content

Commit a4eda4b

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 5cb6a43 commit a4eda4b

2 files changed

Lines changed: 211 additions & 5 deletions

File tree

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

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

178-
// Non-"style" targets: inline contentContainerStyle overwrites className styles
179-
// (array coexistence is only implemented for the ["style"] target path)
180-
expect(component.props.contentContainerStyle).toStrictEqual({
181-
padding: 10,
182-
});
178+
// Both className and inline contentContainerStyle should coexist as array
179+
expect(component.props.contentContainerStyle).toStrictEqual([
180+
{ backgroundColor: "#008000" },
181+
{ padding: 10 },
182+
]);
183183
});
184184

185185
test("ScrollView: contentContainerClassName without contentContainerStyle", () => {
@@ -393,3 +393,140 @@ describe("style={{}} should not destroy computed className styles", () => {
393393
});
394394
});
395395
});
396+
397+
/**
398+
* Tests for multi-config components (e.g. ScrollView with both className and
399+
* contentContainerClassName) where inline style on one target should not
400+
* destroy computed className styles on a different target.
401+
*
402+
* Bug: getStyledProps loops over configs, and each iteration calls deepMergeConfig
403+
* which produces a full props object via Object.assign({}, left, right). When a
404+
* later config iteration runs, it overwrites the correctly-merged target props
405+
* from earlier iterations.
406+
*
407+
* Example: ScrollView with className="bg-red" and style={{ paddingTop: 10 }}.
408+
* The first config (className→style) correctly merges both. The second config
409+
* (contentContainerClassName→contentContainerStyle) does Object.assign which
410+
* copies the inline style={{ paddingTop: 10 }} over the merged style, destroying
411+
* the backgroundColor from className.
412+
*/
413+
describe("multi-config: inline style should not destroy className styles on other targets", () => {
414+
test("ScrollView: className with style should preserve className styles", () => {
415+
registerCSS(`.bg-red { background-color: red; }`);
416+
417+
const component = render(
418+
<ScrollView
419+
testID={testID}
420+
className="bg-red"
421+
style={{ paddingTop: 10 }}
422+
/>,
423+
).getByTestId(testID);
424+
425+
// className backgroundColor should coexist with inline paddingTop
426+
expect(component.props.style).toStrictEqual([
427+
{ backgroundColor: "#f00" },
428+
{ paddingTop: 10 },
429+
]);
430+
});
431+
432+
test("ScrollView: className + contentContainerClassName + style preserves all", () => {
433+
registerCSS(`
434+
.bg-red { background-color: red; }
435+
.p-4 { padding: 16px; }
436+
`);
437+
438+
const component = render(
439+
<ScrollView
440+
testID={testID}
441+
className="bg-red"
442+
contentContainerClassName="p-4"
443+
style={{ paddingTop: 10 }}
444+
/>,
445+
).getByTestId(testID);
446+
447+
// className-derived style merged with inline style
448+
expect(component.props.style).toStrictEqual([
449+
{ backgroundColor: "#f00" },
450+
{ paddingTop: 10 },
451+
]);
452+
453+
// contentContainerClassName should be independently preserved
454+
expect(component.props.contentContainerStyle).toStrictEqual({
455+
padding: 16,
456+
});
457+
});
458+
459+
test("ScrollView: className + contentContainerClassName + both inline styles", () => {
460+
registerCSS(`
461+
.bg-red { background-color: red; }
462+
.p-4 { padding: 16px; }
463+
`);
464+
465+
const component = render(
466+
<ScrollView
467+
testID={testID}
468+
className="bg-red"
469+
contentContainerClassName="p-4"
470+
style={{ paddingTop: 10 }}
471+
contentContainerStyle={{ marginTop: 5 }}
472+
/>,
473+
).getByTestId(testID);
474+
475+
// Both targets should have merged className + inline styles
476+
expect(component.props.style).toStrictEqual([
477+
{ backgroundColor: "#f00" },
478+
{ paddingTop: 10 },
479+
]);
480+
481+
expect(component.props.contentContainerStyle).toStrictEqual([
482+
{ padding: 16 },
483+
{ marginTop: 5 },
484+
]);
485+
});
486+
487+
test("ScrollView: className without style still works (single-config path)", () => {
488+
registerCSS(`.bg-red { background-color: red; }`);
489+
490+
const component = render(
491+
<ScrollView testID={testID} className="bg-red" />,
492+
).getByTestId(testID);
493+
494+
expect(component.props.style).toStrictEqual({ backgroundColor: "#f00" });
495+
});
496+
497+
test("ScrollView: consumed className sources should be removed from props", () => {
498+
registerCSS(`.bg-red { background-color: red; }`);
499+
500+
const component = render(
501+
<ScrollView
502+
testID={testID}
503+
className="bg-red"
504+
contentContainerClassName="bg-red"
505+
style={{ paddingTop: 10 }}
506+
/>,
507+
).getByTestId(testID);
508+
509+
// className and contentContainerClassName should be consumed, not passed through
510+
expect(component.props.className).toBeUndefined();
511+
expect(component.props.contentContainerClassName).toBeUndefined();
512+
});
513+
514+
test("FlatList: contentContainerClassName with contentContainerStyle preserves both", () => {
515+
registerCSS(`.bg-blue { background-color: blue; }`);
516+
517+
const component = render(
518+
<FlatList
519+
testID={testID}
520+
data={[]}
521+
renderItem={() => null}
522+
contentContainerClassName="bg-blue"
523+
contentContainerStyle={{ height: 200 }}
524+
/>,
525+
).getByTestId(testID);
526+
527+
expect(component.props.contentContainerStyle).toStrictEqual([
528+
{ backgroundColor: "#00f" },
529+
{ height: 200 },
530+
]);
531+
});
532+
});

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

@@ -437,6 +471,41 @@ function deepMergeConfig(
437471
);
438472
}
439473

474+
// For length-1 array targets (e.g. ["contentContainerStyle"]), the loop
475+
// above runs 0 iterations. Merge the target prop so inline styles don't
476+
// silently overwrite className-computed styles (same as string target path).
477+
const finalKey = config.target[config.target.length - 1];
478+
if (finalKey && rightIsInline) {
479+
let rightValue = right?.[finalKey];
480+
if (rightValue !== undefined) {
481+
rightValue = filterCssVariables(rightValue);
482+
}
483+
if (rightValue === undefined || rightValue === null) {
484+
// Inline is empty — preserve className-computed value
485+
if (left && finalKey in left) {
486+
result[finalKey] = left[finalKey];
487+
}
488+
} else if (left && finalKey in left) {
489+
const leftValue = left[finalKey];
490+
const leftIsObj =
491+
typeof leftValue === "object" &&
492+
leftValue !== null &&
493+
!Array.isArray(leftValue);
494+
const rightIsObj =
495+
typeof rightValue === "object" &&
496+
rightValue !== null &&
497+
!Array.isArray(rightValue);
498+
if (leftIsObj && rightIsObj) {
499+
if (hasNonOverlappingProperties(leftValue, rightValue)) {
500+
result[finalKey] = [leftValue, rightValue];
501+
}
502+
// else: all left keys are in right, right overrides — already set by mergeDefinedProps
503+
} else {
504+
result[finalKey] = [leftValue, rightValue];
505+
}
506+
}
507+
}
508+
440509
return result;
441510
}
442511

0 commit comments

Comments
 (0)