fix: prevent crash when rendering Section blocks with fields#7423
fix: prevent crash when rendering Section blocks with fields#7423Shevilll wants to merge 2 commits into
Conversation
UiKit Section fields were each wrapped in a <Text> element, but parser.text() returns a non-Text node for select/section fields. React Native throws when a non-Text child is nested inside <Text>, so opening a modal or message containing a section with `fields` crashed the app. Wrap each field in a <View> instead, mirroring how the section's main `text` is already rendered. The redundant text-color override and the now-unused theme plumbing are removed, since parser.text() renders its own styled text. Snapshots for the UiKit Section stories were regenerated to reflect the wrapper change. Closes RocketChat#6951
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe ChangesSection fields rendering
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/containers/UIKit/Section.tsx (1)
30-30: Remove unnecessaryanycast to enforce TypeScript type safety.The
fieldparameter in the map callback is already properly typed asITextfrom thereadonly IText[]array. SinceITextincludes an optionaltypeproperty, theanycast is unnecessary and violates strict TypeScript mode.Proposed change
- <View key={`${(field as any).type || 'field'}-${index}`} style={[styles.text, styles.field]}> + <View key={`${field.type || 'field'}-${index}`} style={[styles.text, styles.field]}>Per coding guidelines: "
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types" and "**/*.{ts,tsx}: Use TypeScript with strict mode enabled."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/containers/UIKit/Section.tsx` at line 30, The key generation in the View component contains an unnecessary `(field as any)` cast that violates TypeScript strict mode. Since the `field` parameter is already properly typed as `IText` from the array iteration, and the `IText` interface includes an optional `type` property, remove the `as any` cast and access `field.type` directly in the template string. Update the line with the key prop to use `${field.type || 'field'}-${index}` instead of `${(field as any).type || 'field'}-${index}`.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app/containers/UIKit/Section.tsx`:
- Line 30: The key generation in the View component contains an unnecessary
`(field as any)` cast that violates TypeScript strict mode. Since the `field`
parameter is already properly typed as `IText` from the array iteration, and the
`IText` interface includes an optional `type` property, remove the `as any` cast
and access `field.type` directly in the template string. Update the line with
the key prop to use `${field.type || 'field'}-${index}` instead of `${(field as
any).type || 'field'}-${index}`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf6db105-e7f3-4ee5-969a-41d308792dd8
⛔ Files ignored due to path filters (2)
app/containers/UIKit/__snapshots__/UiKitMessage.test.tsx.snapis excluded by!**/*.snapapp/containers/UIKit/__snapshots__/UiKitModal.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
app/containers/UIKit/Section.tsxapp/containers/UIKit/interfaces.ts
💤 Files with no reviewable changes (1)
- app/containers/UIKit/interfaces.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/containers/UIKit/Section.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/containers/UIKit/Section.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/containers/UIKit/Section.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/containers/UIKit/Section.tsx
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in 'app/containers/' directory
Files:
app/containers/UIKit/Section.tsx
🧠 Learnings (1)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/containers/UIKit/Section.tsx
field is typed as IText, whose optional type property is directly accessible, so the as any cast is redundant and violates strict mode.
|
Hi team! I've cleaned up the TypeScript cast issue in UIKit Section by removing the unnecessary |
2 participants
Proposed changes
Opening a modal or message that contains a Section block with
fields(e.g. theModalSectionSelectsview) crashed the app.Root cause: in
app/containers/UIKit/Section.tsx, theFieldssub-component wrapped each field in a React Native<Text>:parser.text()returns a non-<Text>node (a block/markdown element) for section/select fields, and React Native throws when a non-<Text>child is nested inside a<Text>. The section's maintextis already rendered correctly inside a<View>(line 48); theFieldspath simply didn't follow the same pattern.This wraps each field in a
<View>instead, matching the existingtextrendering. The redundant text-color override — and the now-unusedtheme/themes/useThemeplumbing (including thethemeprop on theIFieldsinterface) — are removed, sinceparser.text()renders its own styled text.Issue(s)
Closes #6951
How to test or reproduce
sectionblock with afieldsarray (theModalSectionSelectsstory reproduces it).Automated:
TZ=UTC pnpm test --testPathPattern='UiKit'— all UiKit suites pass (46 tests). TheUiKitModal/UiKitMessageSection snapshots were regenerated to reflect the<Text>→<View>wrapper change (the inner text node rendered byparser.text()is unchanged).pnpm lint(ESLint +tsc) passes.Types of changes
Checklist
Summary by CodeRabbit