fix: allow className and style props to coexist when properties differ #223
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, when both className and style props were provided, className-derived styles would be completely overwritten by inline styles due to Object.assign() behavior in deepMergeConfig(). This refined solution: - Only creates style arrays when className and inline styles have non-overlapping properties - Maintains CSS precedence rules (inline styles override className for same properties) - Preserves backward compatibility with existing behavior - Passes all existing tests while enabling the new functionality Enables combining NativeWind className styling with React Native Reanimated animated styles when they target different CSS properties. Fixes issue where className was ignored when style prop was also present, while preserving expected CSS specificity behavior.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where className-derived styles were completely overwritten by inline style props in React Native components. The fix enables both to coexist when they target different CSS properties while maintaining proper CSS precedence rules (inline styles override className for the same properties).
Key Changes:
- Modified
deepMergeConfig()to detect non-overlapping properties between className and inline styles - Creates style arrays only when different properties should coexist, preserving backward compatibility
- Added comprehensive test coverage for the new merging behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/native/styles/index.ts | Implements special style merging logic to support coexistence of className and inline style props |
| src/tests/native/className-with-style.test.tsx | Adds test cases verifying the new style merging behavior for different scenarios |
Address code review feedback by improving the style merging logic: - Remove Set creation and array spreading for better performance - Add early exit when finding non-overlapping properties - Simplify logic flow making it more readable and maintainable - Maintain identical functionality while reducing computational overhead Performance improvements are especially beneficial for components with many style properties, reducing unnecessary object iterations.
|
looking forward to this to get Nativewind v5 working |
|
Thanks @tyrauber 🫡 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, when both className and style props were provided, className-derived styles would be completely overwritten by inline styles due to Object.assign() behavior in deepMergeConfig().
As documented in nativewind/nativewind#1647
This solution:
Enables combining NativeWind className styling with React Native Reanimated animated styles when they target different CSS properties.
Fixes issue where className was ignored when style prop was also present, while preserving expected CSS specificity behavior.
Test locally with a patch.