fix: preserve className styles on multi-config and non-style array targets#320
fix: preserve className styles on multi-config and non-style array targets#320Inv1x wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes cases where className-derived native styles were being lost when merging with inline props, especially for non-style array targets and multi-config components.
Changes:
- Update
getStyledPropsto preserve previously-merged target props across multiple config iterations. - Update
deepMergeConfigto mergeclassName+ inline styles for length-1 array targets (e.g.["contentContainerStyle"]). - Add/adjust tests covering ScrollView/FlatList and multi-config scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/native/styles/index.ts | Adjusts merge logic for multi-config iteration and length-1 array targets to avoid losing computed styles. |
| src/tests/native/className-with-style.test.tsx | Adds regression tests for non-style targets and multi-config components to ensure computed styles are preserved. |
9b4d914 to
1f1cc9b
Compare
|
@danstepanov done |
1f1cc9b to
2ee526f
Compare
|
Preemptive review to help triage for the 5.0 release, @marklawlor makes the final call. Both bugs check out on current main. getStyledProps rebuilds the result per config, so on multi config components like ScrollView the second config clobbers the first one's merged style, and length one targets like contentContainerStyle never merged className with inline styles at all. Branch is freshly rebased and the full suite passes on the merge with main, 1049 tests. One non blocking nit, the new finalKey block also reruns for the plain style target since the guard is only length === 1, worth excluding style in a follow up. Inline style silently destroying className styles on ScrollView is a v4 to v5 regression, so I'd treat this as a blocker. |
…rgets 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.
- Guard finalKey block with config.target.length === 1 so it only runs for length-1 array targets, not nested paths handled by recursion - Explicitly write filtered rightValue in all code paths to prevent unfiltered VAR_SYMBOL objects from leaking into RN props - Add defensive comment about leaf-key storage limitation in getStyledProps
2ee526f to
5533c16
Compare
Summary
Fixes two related bugs where
className-computed styles were silently destroyed:stylearray targets (e.g.["contentContainerStyle"]):deepMergeConfigdid not merge className + inline styles for length-1 array targets. Inline values simply overwrote the className-computed value instead of producing a style array (the behavior already implemented for the["style"]path).className→styleandcontentContainerClassName→contentContainerStyle):getStyledPropsiterates over multiple configs, and each iteration produced a full props object viaObject.assign. Later iterations overwrote earlier iterations' correctly-merged target props.Reproduction
Fix
finalKeymerge block for length-1 array targets indeepMergeConfig, mirroring the existing string-target merge logic.getStyledPropsand restore them after the loop completes.Test plan
contentContainerClassName+contentContainerStyle(non-overlapping props produce array, undefined/null preserves className)className+stylepreserved whencontentContainerClassNamealso presentcontentContainerClassName+contentContainerStylestyled()with string target andstyle={undefined}