-
Notifications
You must be signed in to change notification settings - Fork 16
Feat/deep merge config optimizations #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/deep merge config optimizations #224
Conversation
…nline parameter - Added comprehensive test suite (32 tests) for rightIsInline functionality - Fixed CSS variable filtering in deepMergeConfig for inline styles - Added filterCssVariables() to recursively remove VAR_SYMBOL objects - Added flattenStyleArray() to optimize style arrays when possible - Fixed null/undefined handling for inline styles - Fixed important styles merging with existing style arrays - All 960 existing tests + 32 new tests passing The rightIsInline parameter serves two critical purposes: 1. Strips CSS variable objects (VAR_SYMBOL) from inline styles to prevent runtime errors in React Native 2. Cleans up source properties (e.g., className) when mapped to targets This fix ensures that CSS variable objects never leak into React Native component props, which would cause crashes or unexpected behavior.
Critical fixes: - Add null safety checks to prevent crashes - Use hasOwnProperty to prevent prototype pollution attacks - Add depth limit (100) to prevent stack overflow on deep nesting - Fix falsy value filtering (preserve 0, false, empty strings) - Use Object.keys() to properly filter Symbol properties - Replace spread operator with reduce() for large array performance Performance improvements: - Extract hasNonOverlappingProperties() helper (DRY) - Single-pass array filtering in filterCssVariables() - Early exit in flattenStyleArray() - Handle arrays with 10k+ items without stack overflow Security hardening: - Prevent prototype pollution via for...in loops - Filter inherited properties with hasOwnProperty checks - Remove Symbol properties for React Native compatibility All 960 existing + 32 new tests passing with no regressions.
Red team validation tests covering: - Circular references with depth limit protection - Sparse arrays with undefined holes - Objects with only Symbol properties (RN filtering) - Mixed null/undefined/falsy values preservation - Frozen/sealed objects handling - Very deep nesting (105 levels) beyond limit - Prototype pollution prevention (__proto__) - Arrays with custom properties - Empty arrays and objects - Numeric string keys All 10 edge case tests pass, validating: ✅ No crashes on malformed input ✅ Prototype pollution prevention ✅ Circular reference handling ✅ Stack overflow prevention ✅ Symbol filtering for React Native compatibility ✅ Null safety across all code paths Total test coverage: 970 passing tests (42 rightIsInline-specific)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the CSS variable filtering logic in the native styles module to improve code maintainability and add comprehensive test coverage. The changes centralize the filtering logic, add proper null/undefined handling, and introduce style flattening optimizations.
- Extracts CSS variable filtering into reusable utility functions (
filterCssVariables,hasNonOverlappingProperties,flattenStyleArray) - Enhances handling of inline styles with improved null safety and edge case coverage
- Adds extensive test suite covering CSS variable stripping, important style interactions, and edge cases
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/native/styles/index.ts | Adds three new utility functions for style processing and refactors deepMergeConfig to use centralized filtering logic with better null handling |
| src/tests/native/rightIsInline.test.tsx | New comprehensive test file covering CSS variable filtering, inline style handling, and edge cases including circular references and prototype pollution |
| src/tests/native/className-with-style.test.tsx | Adds tests for important style overrides and multiple className properties with inline styles |
- Add comprehensive JSDoc for filterCssVariables with explicit return type - Clarify hasNonOverlappingProperties one-directional design is intentional - Document empty array handling convention in flattenStyleArray - Address GitHub Copilot review concerns with improved documentation All concerns were about documentation clarity, not logic bugs. Code remains functionally identical with 970 tests passing.
|
Seems like overkill but I'm with it |
Security Hardening and Performance Optimization for Style Filtering
Summary
This PR hardens the CSS variable filtering implementation with critical security fixes and performance optimizations based on red team review.
Changes
Commit 1:
3f1d395- Refactor: harden style filtering with security and performance fixesCritical fixes:
Performance improvements:
Security hardening:
Commit 2:
dffa268- Test: add comprehensive edge case tests for security hardeningRed team validation tests covering:
All 10 edge case tests pass, validating:
Test Results
Files Changed
src/native/styles/index.ts: +66, -48src/__tests__/native/rightIsInline.test.tsx: +175Total: 2 files changed, +241 insertions(+), -48 deletions(-)