refactor(ui-composer): remove unnecessary ReactElement type annotations across components#40805
Conversation
|
|
Looks like this PR is ready to merge! 🎉 |
WalkthroughThis PR systematically removes explicit ChangesRemove explicit ReactElement return types from UI composer components
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (2)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40805 +/- ##
===========================================
- Coverage 70.07% 70.05% -0.03%
===========================================
Files 3337 3337
Lines 123506 123506
Branches 21996 22047 +51
===========================================
- Hits 86552 86517 -35
- Misses 33603 33644 +41
+ Partials 3351 3345 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
d816d1e to
43f312c
Compare
…ns across components
43f312c to
1900967
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In
`@packages/ui-composer/src/MessageFooterCallout/MessageFooterCalloutDivider.tsx`:
- Line 4: The forwardRef generic for MessageFooterCalloutDivider is incorrect:
update the ref type used in forwardRef<HTMLButtonElement> for the
MessageFooterCalloutDivider component to HTMLHRElement (or the appropriate
HTMLElement) to match the rendered <Box is='hr'> element; also update any local
ref typings or Prop types that reference HTMLButtonElement inside the
MessageFooterCalloutDivider function to use HTMLHRElement so consumers receive
the correct ref type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b6e6146-2fab-442c-a9ae-1f89ca7b4f16
📒 Files selected for processing (14)
packages/ui-composer/src/MessageComposer/MessageComposer.tsxpackages/ui-composer/src/MessageComposer/MessageComposerAction.tsxpackages/ui-composer/src/MessageComposer/MessageComposerActionsDivider.tsxpackages/ui-composer/src/MessageComposer/MessageComposerButton.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFileError.tsxpackages/ui-composer/src/MessageComposer/MessageComposerHint.tsxpackages/ui-composer/src/MessageComposer/MessageComposerIcon.tsxpackages/ui-composer/src/MessageComposer/MessageComposerSkeleton.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbar.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarActions.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarSubmit.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCallout.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCalloutDivider.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/ui-composer/src/MessageComposer/MessageComposerAction.tsxpackages/ui-composer/src/MessageComposer/MessageComposerHint.tsxpackages/ui-composer/src/MessageComposer/MessageComposerIcon.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarActions.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbar.tsxpackages/ui-composer/src/MessageComposer/MessageComposerButton.tsxpackages/ui-composer/src/MessageComposer/MessageComposerActionsDivider.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarSubmit.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCallout.tsxpackages/ui-composer/src/MessageComposer/MessageComposerSkeleton.tsxpackages/ui-composer/src/MessageComposer/MessageComposer.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFileError.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCalloutDivider.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:42.118Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs, removing endpoint types and validators from `rocket.chat/rest-typings` (e.g., `UserRegisterParamsPOST`, `/v1/users.register` entry) is the *required* migration pattern per RocketChat/Rocket.Chat-Open-API#150 Rule 7 ("No More rest-typings or Manual Typings"). The endpoint type is re-exposed via a module augmentation `.d.ts` file in the consuming package (e.g., `packages/web-ui-registration/src/users-register.d.ts`). This is NOT a breaking change — the correct changeset bump for `rocket.chat/rest-typings` in this scenario is `minor`, not `major`. Do not flag this as a breaking change during OpenAPI migration reviews.
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.
📚 Learning: 2026-03-04T14:16:49.202Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39304
File: packages/ui-contexts/src/ActionManagerContext.ts:26-26
Timestamp: 2026-03-04T14:16:49.202Z
Learning: In `packages/ui-contexts/src/ActionManagerContext.ts` (TypeScript, RocketChat/Rocket.Chat), the `disposeView` method in `IActionManager` uses an intentionally explicit union `UiKit.ModalView['id'] | UiKit.BannerView['viewId'] | UiKit.ContextualBarView['id']` to document which view types are accepted, even though all constituents resolve to the same primitive. The inline `// eslint-disable-next-line typescript-eslint/no-duplicate-type-constituents` comment is intentional and should not be flagged or removed.
Applied to files:
packages/ui-composer/src/MessageComposer/MessageComposerAction.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarActions.tsxpackages/ui-composer/src/MessageComposer/MessageComposerActionsDivider.tsxpackages/ui-composer/src/MessageComposer/MessageComposer.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsx
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
packages/ui-composer/src/MessageComposer/MessageComposerAction.tsxpackages/ui-composer/src/MessageComposer/MessageComposerHint.tsxpackages/ui-composer/src/MessageComposer/MessageComposerIcon.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbar.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
packages/ui-composer/src/MessageComposer/MessageComposerAction.tsxpackages/ui-composer/src/MessageComposer/MessageComposerHint.tsxpackages/ui-composer/src/MessageComposer/MessageComposerIcon.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarActions.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbar.tsxpackages/ui-composer/src/MessageComposer/MessageComposerButton.tsxpackages/ui-composer/src/MessageComposer/MessageComposerActionsDivider.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarSubmit.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCallout.tsxpackages/ui-composer/src/MessageComposer/MessageComposerSkeleton.tsxpackages/ui-composer/src/MessageComposer/MessageComposer.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFileError.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCalloutDivider.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/ui-composer/src/MessageComposer/MessageComposerAction.tsxpackages/ui-composer/src/MessageComposer/MessageComposerHint.tsxpackages/ui-composer/src/MessageComposer/MessageComposerIcon.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarActions.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbar.tsxpackages/ui-composer/src/MessageComposer/MessageComposerButton.tsxpackages/ui-composer/src/MessageComposer/MessageComposerActionsDivider.tsxpackages/ui-composer/src/MessageComposer/MessageComposerToolbarSubmit.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCallout.tsxpackages/ui-composer/src/MessageComposer/MessageComposerSkeleton.tsxpackages/ui-composer/src/MessageComposer/MessageComposer.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFileError.tsxpackages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsxpackages/ui-composer/src/MessageFooterCallout/MessageFooterCalloutDivider.tsx
📚 Learning: 2026-04-10T22:42:05.539Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 40075
File: apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx:69-71
Timestamp: 2026-04-10T22:42:05.539Z
Learning: In `apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx`, the submit handler converts an empty/whitespace-only description to `undefined` (`description?.trim() || undefined`) intentionally. All downstream image-rendering components (`AttachmentImage`, `ImagePreview`, `ImageItem`, `ImageGallery`) default `undefined` alt to `''`, so the `<img alt="">` attribute is always present. Do not flag this `undefined` conversion as a bug preventing alt text from being cleared.
Applied to files:
packages/ui-composer/src/MessageComposer/MessageComposerToolbarSubmit.tsx
📚 Learning: 2026-02-24T19:36:55.089Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/home-content.ts:60-82
Timestamp: 2026-02-24T19:36:55.089Z
Learning: In RocketChat/Rocket.Chat e2e tests (apps/meteor/tests/e2e/page-objects/fragments/home-content.ts), thread message preview listitems do not have aria-roledescription="message", so lastThreadMessagePreview locator cannot be scoped to messageListItems (which filters for aria-roledescription="message"). It should remain scoped to page.getByRole('listitem') or mainMessageList.getByRole('listitem').
Applied to files:
packages/ui-composer/src/MessageComposer/MessageComposerSkeleton.tsx
🔇 Additional comments (14)
packages/ui-composer/src/MessageComposer/MessageComposer.tsx (1)
2-2: LGTM!Also applies to: 11-11
packages/ui-composer/src/MessageComposer/MessageComposerAction.tsx (1)
2-2: LGTM!Also applies to: 4-4
packages/ui-composer/src/MessageComposer/MessageComposerActionsDivider.tsx (1)
2-2: LGTM!Also applies to: 4-4
packages/ui-composer/src/MessageComposer/MessageComposerButton.tsx (1)
2-2: LGTM!Also applies to: 4-4
packages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFile.tsx (1)
5-6: LGTM!Also applies to: 15-15
packages/ui-composer/src/MessageComposer/MessageComposerIcon.tsx (1)
3-3: LGTM!Also applies to: 5-5
packages/ui-composer/src/MessageComposer/MessageComposerFile/MessageComposerFileError.tsx (1)
1-1: LGTM!Also applies to: 10-10
packages/ui-composer/src/MessageComposer/MessageComposerHint.tsx (1)
3-3: LGTM!Also applies to: 11-11
packages/ui-composer/src/MessageComposer/MessageComposerSkeleton.tsx (1)
5-5: LGTM!packages/ui-composer/src/MessageComposer/MessageComposerToolbar.tsx (1)
2-2: LGTM!Also applies to: 4-4
packages/ui-composer/src/MessageComposer/MessageComposerToolbarActions.tsx (1)
3-3: LGTM!Also applies to: 5-5
packages/ui-composer/src/MessageComposer/MessageComposerToolbarSubmit.tsx (1)
2-2: LGTM!Also applies to: 4-4
packages/ui-composer/src/MessageFooterCallout/MessageFooterCallout.tsx (1)
2-2: LGTM!Also applies to: 13-13
packages/ui-composer/src/MessageFooterCallout/MessageFooterCalloutDivider.tsx (1)
2-2: LGTM!
…ns across components (#40805)
Proposed changes (including videos or screenshots)
As a first step towards upgrading to React 19, it handles types from
@types/reactlooking forward the next major.Issue(s)
Task: ARCH-2170
Steps to test or reproduce
Further comments
No runtime change is expected from it.
Summary by CodeRabbit