Skip to content

fix: prevent style={undefined} from destroying computed styles in non-[style] targets#291

Open
YevheniiKotyrlo wants to merge 1 commit intonativewind:mainfrom
YevheniiKotyrlo:fix/deep-merge-config-undefined-styles
Open

fix: prevent style={undefined} from destroying computed styles in non-[style] targets#291
YevheniiKotyrlo wants to merge 1 commit intonativewind:mainfrom
YevheniiKotyrlo:fix/deep-merge-config-undefined-styles

Conversation

@YevheniiKotyrlo
Copy link

@YevheniiKotyrlo YevheniiKotyrlo commented Mar 4, 2026

Summary

Fixes remaining Object.assign calls in deepMergeConfig that allow style={undefined} to destroy computed NativeWind styles on non-["style"] targets.

Problem

PR #224 fixed the most common code path (config.target = ["style"]), but two Object.assign({}, left, right) calls remain unfixed at lines 362 and 365.

Object.assign copies all enumerable own properties from right, including those with value undefined. When a component passes contentContainerStyle={undefined} alongside contentContainerClassName, the computed styles from left are overwritten with undefined.

Affected components:

  • ScrollViewcontentContainerClassName + contentContainerStyle={undefined}
  • FlatListcontentContainerClassName / columnWrapperClassName + *Style={undefined}
  • VirtualizedListcontentContainerClassName + contentContainerStyle={undefined}
  • Any custom styled() with non-default target mappings (e.g. { className: { target: "style" } })

This is a common pattern in reusable components where style props are optional:

function MyScrollView({ contentContainerStyle, ...props }: ScrollViewProps) {
  return (
    <ScrollView
      contentContainerClassName="p-4"
      contentContainerStyle={contentContainerStyle}
      {...props}
    />
  );
}
// <MyScrollView /> — contentContainerStyle is implicitly undefined, className styles lost

Native-only — web uses array concatenation (no Object.assign).

Solution

Added a mergeDefinedProps helper that replaces both Object.assign({}, left, right) calls. It spreads left into a new object and iterates right via for-in, skipping keys with undefined values.

function mergeDefinedProps(
  left: Record<string, any> | undefined,
  right: Record<string, any>,
) {
  const result = left ? { ...left } : {};
  for (const key in right) {
    if (right[key] !== undefined) {
      result[key] = right[key];
    }
  }
  return result;
}

This covers all config.target types (array, string, false) at all recursion depths, and still allows explicit null values to override (only undefined is skipped). Uses for-in instead of Object.keys() to avoid intermediate array allocation. Both left and right are always plain object literals (JSX props or calculateProps output) — no prototype chain concerns.

Performance

Benchmarked 15 implementation variants of mergeDefinedProps in isolation (2M iterations, best of 5 runs) across 6 scenarios matching real component renders, then validated the winner in the full deepMergeConfig context (1M iterations each):

Scenario Original Fixed Ratio
ScrollView + style={undefined} (the bug) 129ms 113ms 0.87x (13% faster)
ScrollView + valid style 125ms 122ms 0.97x (same)
FlatList + many props + undefined styles 320ms 302ms 0.94x (6% faster)
Custom styled() + style={undefined} 172ms 161ms 0.94x (6% faster)

The fix is faster than the original Object.assign across all real-world scenarios.

Verification

  • yarn typecheck — pass
  • yarn lint — pass
  • yarn build — pass
  • yarn test — 977 passed, 3 failed (pre-existing babel plugin failures, same on main)

Added 8 test cases to className-with-style.test.tsx covering all config.target types:

  • Path A (["style"]): style={undefined} and style={null} preserve computed styles
  • Path B (["contentContainerStyle"]): ScrollView and FlatList with contentContainerStyle={undefined} preserve computed styles; valid contentContainerStyle merges correctly; omitted prop preserves styles
  • Path B (string target): custom styled() with { target: "style" } preserves styles with style={undefined}
  • Real-world: optional contentContainerStyle prop forwarding (implicitly undefined) preserves computed styles

Reproduction

Visual reproduction repo with before/after screenshots: nativewind-style-undefined-repro

Before (3.0.4 unpatched)After (patched)

Related

@YevheniiKotyrlo YevheniiKotyrlo force-pushed the fix/deep-merge-config-undefined-styles branch 2 times, most recently from 56daaa5 to 191f035 Compare March 4, 2026 17:30
@YevheniiKotyrlo YevheniiKotyrlo force-pushed the fix/deep-merge-config-undefined-styles branch from 191f035 to 6ef69c1 Compare March 4, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant