fix: Add viewContainer styles to android headless wrapper component #680
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
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe update modifies the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Component
participant TouchableWrapper
Caller->>Component: renderAndroidHeadless(props)
Component->>TouchableWrapper: Merge styles (touchableWrapperProps.style + defaultStyles.viewContainer + style.viewContainer)
Component->>Caller: Return TouchableWrapper with merged styles
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/index.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,jsx,ts,tsx}`: Review the TypeScript, React Native, and React code for conformity with best practices in React Native, React, and TypeScript.
For default values try to su...
**/*.{js,jsx,ts,tsx}: Review the TypeScript, React Native, and React code for conformity with best practices in React Native, React, and TypeScript.For default values try to suggest using constants instead of adding comments on code.
Check for accessibility and semantic code.
Translate with i18n where possible.
Check for common security vulnerabilities such as:
- XSS (Cross-Site Scripting)
- OWASP vulnerabilities
- Memory leaks
- Insecure dependencies
- Sensitive data exposure
src/index.js
🧬 Code Graph Analysis (1)
src/index.js (1)
src/styles.js (2)
defaultStyles(3-84)defaultStyles(3-84)
🪛 ESLint
src/index.js
[error] 520-520: Insert ,
(prettier/prettier)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
Comment on lines
+517
to
+521
| style={{ | ||
| ...touchableWrapperProps.style, | ||
| ...defaultStyles.viewContainer, | ||
| ...style.viewContainer | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Object-spread risks a crash and introduces a duplicate style prop
touchableWrapperProps.styleis spread directly. If the consumer omits thestylekey (default is{}), the value will beundefined, and{ ...undefined }throwsTypeError: Cannot convert undefined or null to objectat runtime.- Because
touchableWrapperPropsis already spread earlier ({...touchableWrapperProps}), you now pass twostyleprops – one from the spread, one explicitly. This violatesreact/jsx-no-duplicate-propsand whichever comes last wins, making the first silently ignored. - React-Native encourages merging styles with an array, not by object-spreading. Arrays keep the original objects intact, play nicely with
StyleSheet.flatten, and are more familiar to library consumers. - ESLint / Prettier is already complaining about a missing trailing comma here (
Insert ,). Switching to an array also resolves that.
Proposed fix:
- {...touchableWrapperProps}
- style={{
- ...touchableWrapperProps.style,
- ...defaultStyles.viewContainer,
- ...style.viewContainer
- }}
+ {...(() => {
+ // separate out style to avoid duplicate props
+ const { style: wrapperStyle, ...rest } = touchableWrapperProps;
+ return {
+ ...rest,
+ style: [
+ defaultStyles.viewContainer,
+ style.viewContainer,
+ wrapperStyle, // may be undefined – safe in an array
+ ],
+ };
+ })()}This:
• Prevents the crash when wrapperStyle is undefined.
• Removes the duplicate style prop.
• Uses the canonical React-Native array-merge pattern.
• Keeps later entries (wrapperStyle) overriding earlier ones, preserving existing semantics.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ESLint
[error] 520-520: Insert ,
(prettier/prettier)
🤖 Prompt for AI Agents
In src/index.js around lines 517 to 521, avoid spreading
touchableWrapperProps.style directly as it can be undefined causing a runtime
crash, and also avoid passing duplicate style props by spreading
touchableWrapperProps and then separately spreading style. Instead, merge styles
using a React-Native style array combining touchableWrapperProps.style,
defaultStyles.viewContainer, and style.viewContainer in order, ensuring no
duplicate style props and preventing crashes from undefined values. This also
resolves the missing trailing comma ESLint/prettier issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
viewContainer is not applied to android headless like on iOs or android native pickers.
This simply spread the viewContainer style to the wrapper component