refactor: React 19 - #40796
Merged
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
|
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 ignored due to path filters (1)
📒 Files selected for processing (17)
🚧 Files skipped from review as they are similar to previous changes (8)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
|
| Layer / File(s) | Summary |
|---|---|
Dependency updates package.json, apps/meteor/package.json, apps/uikit-playground/package.json, ee/apps/omnichannel-transcript/package.json, ee/packages/pdf-worker/package.json, packages/fuselage-ui-kit/package.json, packages/gazzodown/package.json, packages/livechat/package.json, packages/mock-providers/package.json, packages/storybook-config/package.json, packages/ui-avatar/package.json, packages/ui-client/package.json, packages/ui-composer/package.json, packages/ui-contexts/package.json, packages/ui-video-conf/package.json, packages/ui-voip/package.json, packages/web-ui-registration/package.json |
React runtime and type packages are upgraded to React 19, @react-pdf/renderer is bumped, Rocket.Chat UI package ranges are updated, and root @react-pdf/layout resolutions change. |
React event typing updates apps/meteor/client/components/FilterByText.tsx, apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx, apps/meteor/client/views/admin/settings/SettingsGroupPage/SettingsGroupPage.tsx, apps/meteor/client/views/mediaCallHistory/CallHistoryPageFilters.tsx, apps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/TemplatePlaceholderInput.tsx, apps/meteor/client/views/omnichannel/directory/contacts/RemoveContactModal.tsx, apps/meteor/client/views/omnichannel/realTimeMonitoring/counter/CounterRow.tsx, apps/meteor/client/views/room/MessageList/contexts/SelectedMessagesContext.tsx, apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx, packages/ui-client/src/components/MultiSelectCustom/MultiSelectCustom.tsx |
Form submit handlers switch to SubmitEvent, input change handlers switch to ChangeEvent, useToggleSelect switches to SyntheticEvent, and CounterRow updates its reduce accumulator typing. |
Error boundary typing changes apps/meteor/client/views/admin/engagementDashboard/EngagementDashboardCardErrorBoundary.tsx, apps/meteor/client/views/root/AppErrorPage.tsx, apps/meteor/client/views/root/OutermostErrorBoundary.tsx, packages/gazzodown/src/katex/KatexErrorBoundary.tsx |
Error state handling changes from concrete Error props/state to unknown-based handling, and the fallback component signatures and message rendering paths are adjusted accordingly. |
pdf-worker Jest and lint updates ee/packages/pdf-worker/jest.config.ts, ee/packages/pdf-worker/src/worker.spec.ts |
Jest transform ignore rules are expanded for @react-pdf and related packages, and the worker spec adds an ESLint disable comment. |
Messaging e2e refactor apps/meteor/tests/e2e/messaging.spec.ts, apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts |
The messaging spec now uses channelPage for setup, navigation, editing, and attachment assertions, and RoomToolbar.waitFor() is added for visibility checks. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~15 minutes
Possibly related PRs
- RocketChat/Rocket.Chat#40823: Overlaps on React handler event-typing changes in form-related components.
- RocketChat/Rocket.Chat#40974: Touches the same Rocket.Chat UI dependency upgrade area in
apps/meteor/package.json. - RocketChat/Rocket.Chat#41066: Also updates shared Rocket.Chat dependency ranges in
apps/meteor/package.json.
Suggested labels
type: chore
Suggested reviewers
- ggazzo
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly matches the main change: a React 19 upgrade/refactor across the frontend. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
Warning
Review ran into problems
🔥 Problems
Errors were encountered while retrieving linked issues.
Errors (2)
- ARCH-2195: Request failed with status code 401
- REACT-19: Request failed with status code 401
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 help to get the list of available commands.
Member
Author
|
/jira ARCH-2167 |
tassoevan
marked this pull request as ready for review
June 4, 2026 02:40
⏭️ Hacktron Security Check — SkippedReason: This PR exceeds Hacktron's 200-file review cap and will not be scanned. Split the PR into smaller changes for review coverage.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40796 +/- ##
========================================
Coverage 69.98% 69.99%
========================================
Files 3392 3392
Lines 130458 130457 -1
Branches 22702 22701 -1
========================================
+ Hits 91297 91307 +10
+ Misses 35804 35799 -5
+ Partials 3357 3351 -6
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
No issues found across 987 files
Note: This PR contains a large number of files. Free-tier reviews are limited to 40 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review. Paid plans can review more files.
Re-trigger cubic
This was referenced Jun 4, 2026
Merged
Merged
Merged
Merged
Merged
tassoevan
force-pushed
the
refactor/prepare-types-for-react-19
branch
2 times, most recently
from
June 30, 2026 01:14
a38dfc1 to
7b73f1a
Compare
tassoevan
marked this pull request as ready for review
June 30, 2026 02:36
Member
Author
|
/jira ARCH-2167 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/meteor/tests/e2e/messaging.spec.ts (1)
167-170: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse a web-first value assertion here.
expect(await channelPage.composer.inputMessage.inputValue()).toBe('msg2')snapshots once and can race the editor update. Preferawait expect(channelPage.composer.inputMessage).toHaveValue('msg2'). As per coding guidelines, "Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests".Suggested change
- expect(await channelPage.composer.inputMessage.inputValue()).toBe('msg2'); + await expect(channelPage.composer.inputMessage).toHaveValue('msg2');🤖 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 `@apps/meteor/tests/e2e/messaging.spec.ts` around lines 167 - 170, The assertion in the “focus on the second message” test is snapshotting the input value with inputValue(), which can race the editor update. Update the check on channelPage.composer.inputMessage to use Playwright’s web-first assertion pattern, replacing the direct value read with an awaited value assertion such as toHaveValue('msg2') so the test waits for the UI state to settle.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.
Inline comments:
In `@apps/meteor/tests/e2e/messaging.spec.ts`:
- Around line 16-18: The messaging e2e suite is sharing a mutable targetChannel
across tests, which makes later cases depend on earlier seeded messages and
causes flaky, order-dependent behavior. Update the setup around
createTargetChannel in messaging.spec.ts so each test or suite gets its own
fresh channel, or move message seeding into each suite’s setup instead of
relying on shared state. Keep the fix local to the test hooks and the affected
cases that currently read or modify seeded messages so the suite remains
isolated and safe to run in any order.
---
Nitpick comments:
In `@apps/meteor/tests/e2e/messaging.spec.ts`:
- Around line 167-170: The assertion in the “focus on the second message” test
is snapshotting the input value with inputValue(), which can race the editor
update. Update the check on channelPage.composer.inputMessage to use
Playwright’s web-first assertion pattern, replacing the direct value read with
an awaited value assertion such as toHaveValue('msg2') so the test waits for the
UI state to settle.
🪄 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: fd820de7-8987-4f3f-8048-4b1392c19f4b
⛔ Files ignored due to path filters (32)
.yarn/patches/@react-pdf-layout-npm-4.6.1-b12fdb71f4.patchis excluded by!**/.yarn/**apps/meteor/client/components/ABAC/ABACUpsellModal/__snapshots__/ABACUpsellModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/components/CreateDiscussion/__snapshots__/CreateDiscussion.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/components/GenericUpsellModal/__snapshots__/GenericUpsellModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/components/UserInfo/__snapshots__/UserInfo.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/components/message/toolbar/items/actions/Timestamp/TimestampPicker/__snapshots__/TimestampPicker.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/admin/ABAC/ABACAttributesTab/__snapshots__/AttributesForm.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/admin/ABAC/ABACRoomsTab/__snapshots__/DeleteRoomModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/admin/permissions/PermissionsTable/__snapshots__/PermissionsTable.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/admin/permissions/UsersInRole/UsersInRoleTable/__snapshots__/UsersInRoleTable.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/admin/settings/Setting/inputs/__snapshots__/RangeSettingInput.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/admin/users/UsersTable/__snapshots__/UsersTable.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/audit/components/__snapshots__/SecurityLogDisplayModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/e2e/EnterE2EPasswordModal/__snapshots__/EnterE2EPasswordModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppLogs/Filters/__snapshots__/AppLogsFilterCompact.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppLogs/Filters/__snapshots__/AppLogsFilterContextualBar.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppLogs/Filters/__snapshots__/AppLogsFilterExpanded.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/marketplace/AppDetailsPage/tabs/AppLogs/Filters/__snapshots__/DateTimeModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/omnichannel/cannedResponses/contextualBar/CannedResponse/__snapshots__/CannedResponseList.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/omnichannel/modals/__snapshots__/ForwardChatModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/omnichannel/modals/__snapshots__/ReturnChatQueueModal.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/room/contextualBar/BannedUsers/__snapshots__/BannedUsers.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/room/contextualBar/Discussions/__snapshots__/DiscussionsList.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/room/contextualBar/RoomFiles/__snapshots__/RoomFiles.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/room/contextualBar/RoomMembers/InviteUsers/__snapshots__/InviteUsers.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/room/contextualBar/RoomMembers/__snapshots__/RoomMembers.spec.tsx.snapis excluded by!**/*.snapapps/meteor/client/views/room/modals/FileUploadModal/__snapshots__/FileUploadModal.spec.tsx.snapis excluded by!**/*.snappackages/ui-client/src/components/Modal/GenericModal/__snapshots__/GenericModal.spec.tsx.snapis excluded by!**/*.snappackages/ui-voip/src/views/CallHistoryContextualbar/__snapshots__/CallHistoryContextualbar.spec.tsx.snapis excluded by!**/*.snappackages/ui-voip/src/views/MediaCallWidget/__snapshots__/MediaCallWidget.spec.tsx.snapis excluded by!**/*.snappackages/ui-voip/src/views/PermissionFlow/__snapshots__/PermissionFlowModal.spec.tsx.snapis excluded by!**/*.snapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
apps/meteor/client/components/FilterByText.tsxapps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsxapps/meteor/client/views/admin/settings/SettingsGroupPage/SettingsGroupPage.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPageFilters.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/TemplatePlaceholderInput.tsxapps/meteor/client/views/omnichannel/directory/contacts/RemoveContactModal.tsxapps/meteor/client/views/omnichannel/realTimeMonitoring/counter/CounterRow.tsxapps/meteor/client/views/room/MessageList/contexts/SelectedMessagesContext.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/package.jsonapps/meteor/tests/e2e/messaging.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/uikit-playground/package.jsonee/apps/omnichannel-transcript/package.jsonee/packages/pdf-worker/jest.config.tsee/packages/pdf-worker/package.jsonee/packages/pdf-worker/src/worker.spec.tspackage.jsonpackages/fuselage-ui-kit/package.jsonpackages/gazzodown/package.jsonpackages/livechat/package.jsonpackages/mock-providers/package.jsonpackages/storybook-config/package.jsonpackages/ui-avatar/package.jsonpackages/ui-client/package.jsonpackages/ui-client/src/components/MultiSelectCustom/MultiSelectCustom.tsxpackages/ui-composer/package.jsonpackages/ui-contexts/package.jsonpackages/ui-video-conf/package.jsonpackages/ui-voip/package.jsonpackages/web-ui-registration/package.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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:
ee/packages/pdf-worker/src/worker.spec.tsee/packages/pdf-worker/jest.config.tsapps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsxapps/meteor/client/views/omnichannel/realTimeMonitoring/counter/CounterRow.tsxapps/meteor/client/views/omnichannel/directory/contacts/RemoveContactModal.tsxapps/meteor/client/views/admin/settings/SettingsGroupPage/SettingsGroupPage.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPageFilters.tsxapps/meteor/client/components/FilterByText.tsxpackages/ui-client/src/components/MultiSelectCustom/MultiSelectCustom.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/room/MessageList/contexts/SelectedMessagesContext.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/TemplatePlaceholderInput.tsxapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/messaging.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
ee/packages/pdf-worker/src/worker.spec.tsapps/meteor/tests/e2e/messaging.spec.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/messaging.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/messaging.spec.ts
🧠 Learnings (12)
📚 Learning: 2026-06-16T14:13:34.463Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:31-31
Timestamp: 2026-06-16T14:13:34.463Z
Learning: In Rocket.Chat’s monorepo, when reviewing a dependency entry and flagging that a specific version “does not exist” (e.g., in package.json), first verify the exact package/version directly against the npm registry (use URLs like https://registry.npmjs.org/<package>/<version> or https://www.npmjs.com/package/<package>/v/<version>). Do not rely on web search results for this check, since they may be stale or cached and may not reflect the latest published versions.
Applied to files:
packages/ui-video-conf/package.jsonpackages/storybook-config/package.jsonpackages/ui-avatar/package.jsonpackages/gazzodown/package.jsonpackages/fuselage-ui-kit/package.jsonpackages/ui-composer/package.jsonpackages/ui-voip/package.jsonpackages/ui-contexts/package.jsonpackages/mock-providers/package.jsonpackages/web-ui-registration/package.jsonpackages/livechat/package.jsonpackages/ui-client/package.json
📚 Learning: 2026-06-16T14:13:49.795Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:26-26
Timestamp: 2026-06-16T14:13:49.795Z
Learning: During code reviews that check whether a dependency version exists in package.json (especially for Rocket.Chat’s rocket.chat/fuselage and related rocket.chat/fuselage-* packages), don’t rely on web search results. Instead, verify the version directly against the npm registry (e.g., via the npm registry API or the canonical package URL https://www.npmjs.com/package/<package>/v/<version>) before deciding that a version bump is invalid. If the version is present in the npm registry, do not flag it as invalid.
Applied to files:
packages/ui-video-conf/package.jsonpackages/storybook-config/package.jsonpackages/ui-avatar/package.jsonpackages/gazzodown/package.jsonpackages/fuselage-ui-kit/package.jsonpackages/ui-composer/package.jsonpackages/ui-voip/package.jsonpackages/ui-contexts/package.jsonpackages/mock-providers/package.jsonpackages/web-ui-registration/package.jsonpackages/livechat/package.jsonpackages/ui-client/package.json
📚 Learning: 2026-06-16T14:13:59.986Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/ui-video-conf/package.json:25-25
Timestamp: 2026-06-16T14:13:59.986Z
Learning: In the Rocket.Chat monorepo, when reviewing a dependency version bump for rocket.chat/fuselage in a package.json, do not flag the new version constraint as “non-existent” or invalid unless you verify the published versions directly from the npm registry (https://www.npmjs.com/package/rocket.chat/fuselage). Don’t rely on search/web results for available versions since they can be stale.
Applied to files:
packages/ui-video-conf/package.jsonpackages/storybook-config/package.jsonpackages/ui-avatar/package.jsonpackages/gazzodown/package.jsonpackages/fuselage-ui-kit/package.jsonpackages/ui-composer/package.jsonpackages/ui-voip/package.jsonpackages/ui-contexts/package.jsonpackages/mock-providers/package.jsonpackages/web-ui-registration/package.jsonpackages/livechat/package.jsonpackages/ui-client/package.json
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
ee/packages/pdf-worker/src/worker.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
ee/packages/pdf-worker/src/worker.spec.tsapps/meteor/tests/e2e/messaging.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
ee/packages/pdf-worker/src/worker.spec.tsee/packages/pdf-worker/jest.config.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/messaging.spec.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
ee/packages/pdf-worker/src/worker.spec.tsee/packages/pdf-worker/jest.config.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/messaging.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
ee/packages/pdf-worker/src/worker.spec.tsapps/meteor/tests/e2e/messaging.spec.ts
📚 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:
ee/packages/pdf-worker/src/worker.spec.tsee/packages/pdf-worker/jest.config.tsapps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsxapps/meteor/client/views/omnichannel/realTimeMonitoring/counter/CounterRow.tsxapps/meteor/client/views/omnichannel/directory/contacts/RemoveContactModal.tsxapps/meteor/client/views/admin/settings/SettingsGroupPage/SettingsGroupPage.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPageFilters.tsxapps/meteor/client/components/FilterByText.tsxpackages/ui-client/src/components/MultiSelectCustom/MultiSelectCustom.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/room/MessageList/contexts/SelectedMessagesContext.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/TemplatePlaceholderInput.tsxapps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/messaging.spec.ts
📚 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:
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsxapps/meteor/client/views/omnichannel/realTimeMonitoring/counter/CounterRow.tsxapps/meteor/client/views/omnichannel/directory/contacts/RemoveContactModal.tsxapps/meteor/client/views/admin/settings/SettingsGroupPage/SettingsGroupPage.tsxapps/meteor/client/views/mediaCallHistory/CallHistoryPageFilters.tsxapps/meteor/client/components/FilterByText.tsxpackages/ui-client/src/components/MultiSelectCustom/MultiSelectCustom.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/room/MessageList/contexts/SelectedMessagesContext.tsxapps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/TemplatePlaceholderInput.tsx
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2026-02-24T19:39:42.247Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/page-objects/fragments/message.ts:7-7
Timestamp: 2026-02-24T19:39:42.247Z
Learning: In RocketChat e2e tests, avoid using data-qa attributes to locate elements. Prefer semantic locators such as getByRole, getByLabel, getByText, getByTitle and ARIA-based selectors. Apply this rule to all TypeScript files under apps/meteor/tests/e2e to improve test reliability, accessibility, and maintainability.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/toolbar.tsapps/meteor/tests/e2e/messaging.spec.ts
🔇 Additional comments (24)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (1)
126-128: LGTM!apps/meteor/client/views/omnichannel/components/outboundMessage/components/TemplatePlaceholderSelector/TemplatePlaceholderInput.tsx (1)
4-4: LGTM!Also applies to: 19-21, 31-31
apps/meteor/client/views/room/MessageList/contexts/SelectedMessagesContext.tsx (1)
1-1: LGTM!Also applies to: 52-52
apps/meteor/client/views/admin/settings/SettingsGroupPage/SettingsGroupPage.tsx (1)
7-7: 🎯 Functional CorrectnessThis
SubmitEvent<HTMLFormElement>typing is already consistent with the repo’s React 19 types; no change needed.> Likely an incorrect or invalid review comment.apps/meteor/client/components/FilterByText.tsx (1)
3-3: 🎯 Functional CorrectnessNo change needed
SubmitEvent<HTMLFormElement>matches the repo’s current React type setup and is used consistently elsewhere.> Likely an incorrect or invalid review comment.apps/meteor/client/views/omnichannel/directory/contacts/RemoveContactModal.tsx (1)
6-6: 🎯 Functional Correctness
SubmitEventis valid here; no change needed.> Likely an incorrect or invalid review comment.apps/meteor/client/views/mediaCallHistory/CallHistoryPageFilters.tsx (1)
5-5: LGTM!Also applies to: 82-82
apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx (1)
16-16: LGTM!Also applies to: 52-52
apps/meteor/client/components/UserAutoCompleteMultiple/UserAutoCompleteMultiple.tsx (1)
104-104: LGTM!apps/meteor/client/views/omnichannel/realTimeMonitoring/counter/CounterRow.tsx (1)
14-22: LGTM!packages/ui-client/src/components/MultiSelectCustom/MultiSelectCustom.tsx (1)
5-5: LGTM!Also applies to: 83-83
apps/meteor/package.json (1)
276-283: 🎯 Functional CorrectnessNo action needed —
apps/meteor/.babelrcalready setsruntime: "automatic"andapps/meteor/tsconfig.jsonalready usesjsx: "react-jsx".ee/packages/pdf-worker/package.json (1)
21-27: LGTM!Also applies to: 41-48
packages/fuselage-ui-kit/package.json (1)
49-56: LGTM!Also applies to: 68-78
packages/gazzodown/package.json (1)
34-41: LGTM!Also applies to: 52-62
packages/ui-contexts/package.json (1)
26-42: LGTM!packages/ui-video-conf/package.json (1)
25-44: LGTM!packages/ui-voip/package.json (1)
34-42: LGTM!Also applies to: 56-65
packages/livechat/package.json (1)
60-62: LGTM!Also applies to: 71-71, 95-96
packages/mock-providers/package.json (1)
36-41: LGTM!packages/storybook-config/package.json (1)
23-23: LGTM!Also applies to: 36-42
packages/ui-avatar/package.json (1)
20-29: LGTM!packages/ui-client/package.json (1)
21-21: LGTM!Also applies to: 25-56
packages/ui-composer/package.json (1)
25-26: LGTM!Also applies to: 38-43
Comment on lines
16
to
18
| test.beforeAll(async ({ api }) => { | ||
| targetChannel = await createTargetChannel(api); | ||
| }); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid sharing one mutable channel across these tests.
targetChannel is created once for the whole file, but later cases read and edit messages seeded by earlier tests. That makes the suite order-dependent and fragile on retries. Create a fresh channel per test/suite or seed the required messages in each suite setup instead. As per coding guidelines, "Maintain test isolation between test cases in Playwright tests", "Ensure clean state for each test execution in Playwright tests", and "Ensure tests run reliably in parallel without shared state conflicts".
Also applies to: 30-34
🤖 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 `@apps/meteor/tests/e2e/messaging.spec.ts` around lines 16 - 18, The messaging
e2e suite is sharing a mutable targetChannel across tests, which makes later
cases depend on earlier seeded messages and causes flaky, order-dependent
behavior. Update the setup around createTargetChannel in messaging.spec.ts so
each test or suite gets its own fresh channel, or move message seeding into each
suite’s setup instead of relying on shared state. Keep the fix local to the test
hooks and the affected cases that currently read or modify seeded messages so
the suite remains isolated and safe to run in any order.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
6 issues found across 63 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
tassoevan
marked this pull request as draft
June 30, 2026 03:47
tassoevan
marked this pull request as ready for review
June 30, 2026 11:36
Contributor
There was a problem hiding this comment.
2 issues found across 67 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/uikit-playground/package.json">
<violation number="1" location="apps/uikit-playground/package.json:35">
P2: `rc-scrollbars@^1.1.6` peer deps exclude React 19. npm v7+ will warn/error. May need `--legacy-peer-deps` or replacement.</violation>
<violation number="2" location="apps/uikit-playground/package.json:35">
P1: `react-split-pane@^0.1.92` peer dep `react@^16.0.0-0` — incompatible with React 19. npm v7+ ERESOLVE error on install. Upgrade to v3.x.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "rc-scrollbars": "^1.1.6", | ||
| "react": "~18.3.1", | ||
| "react-dom": "~18.3.1", | ||
| "react": "~19.2.7", |
Contributor
There was a problem hiding this comment.
P1: react-split-pane@^0.1.92 peer dep react@^16.0.0-0 — incompatible with React 19. npm v7+ ERESOLVE error on install. Upgrade to v3.x.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/uikit-playground/package.json, line 35:
<comment>`react-split-pane@^0.1.92` peer dep `react@^16.0.0-0` — incompatible with React 19. npm v7+ ERESOLVE error on install. Upgrade to v3.x.</comment>
<file context>
@@ -18,33 +18,33 @@
"rc-scrollbars": "^1.1.6",
- "react": "~18.3.1",
- "react-dom": "~18.3.1",
+ "react": "~19.2.7",
+ "react-dom": "~19.2.7",
"react-router-dom": "^6.30.4",
</file context>
| "rc-scrollbars": "^1.1.6", | ||
| "react": "~18.3.1", | ||
| "react-dom": "~18.3.1", | ||
| "react": "~19.2.7", |
Contributor
There was a problem hiding this comment.
P2: rc-scrollbars@^1.1.6 peer deps exclude React 19. npm v7+ will warn/error. May need --legacy-peer-deps or replacement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/uikit-playground/package.json, line 35:
<comment>`rc-scrollbars@^1.1.6` peer deps exclude React 19. npm v7+ will warn/error. May need `--legacy-peer-deps` or replacement.</comment>
<file context>
@@ -18,33 +18,33 @@
"rc-scrollbars": "^1.1.6",
- "react": "~18.3.1",
- "react-dom": "~18.3.1",
+ "react": "~19.2.7",
+ "react-dom": "~19.2.7",
"react-router-dom": "^6.30.4",
</file context>
ggazzo
approved these changes
Jun 30, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
This was referenced Jun 30, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Proposed changes (including videos or screenshots)
It upgrades the frontend to React 19.
Issue(s)
Steps to test or reproduce
Further comments
It might unexpectedly affect behavior in development mode due to changes on how Strict Mode works in React.
Task: ARCH-2195
Summary by CodeRabbit
Bug Fixes
Tests
Chores