Skip to content

Conversation

@tassoevan
Copy link
Contributor

@tassoevan tassoevan commented Nov 14, 2025

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Bug Fixes

    • Fixed room type handling in channel selection components to prevent incorrect property overrides.
  • Chores

    • Updated internal dependencies and removed unused polyfills from peer dependencies.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 14, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 14, 2025

⚠️ No Changeset found

Latest commit: c74fdee

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 14, 2025

Walkthrough

This PR removes the @rocket.chat/fuselage-polyfills dependency, updates fuselage-related package versions in fuselage-ui-kit, and refactors autocomplete component typing by eliminating Option imports and inlining renderItem logic. Room object property ordering is adjusted to prevent type field overrides.

Changes

Cohort / File(s) Summary
Autocomplete component typing updates
apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx, apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx
Removed Option import and ComponentPropsWithoutRef dependency. AutoCompleteDepartmentMultiple inlines renderItem logic directly into PaginatedMultiSelectFiltered prop; AutoCompleteMonitors updates renderItem typing to ignore ref parameter while removing Option-based generic typing.
Fuselage UI Kit dependency updates
packages/fuselage-ui-kit/package.json
Updated version specifiers: @rocket.chat/fuselage to ~0.68.1, @rocket.chat/fuselage-hooks to ~0.38.1, @rocket.chat/icons to ~0.45.0. Removed @rocket.chat/fuselage-polyfills from dependencies, peerDependencies, and devDependencies.
Fuselage UI Kit documentation
packages/fuselage-ui-kit/README.md
Removed @rocket.chat/fuselage-polyfills from listed peer dependencies in installation instructions.
RoomAvatar room object property reordering
packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx, packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx
Reordered room object properties passed to RoomAvatar from {type: ..., _id: ..., ...label} to {_id: ..., ...label, type: ...} to ensure type field is set after spreading label, preventing unintended overrides.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Dependency version updates in package.json should be verified against compatibility notes and changelog
  • Property reordering in room objects appears straightforward but verify no unexpected type behavior changes across related components

Possibly related PRs

  • PR #37420: Directly related refactoring of renderItem implementation and typing in the same autocomplete components (AutoCompleteDepartmentMultiple.tsx and AutoCompleteMonitors.tsx).

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • MartinSchoeler
  • aleksandernsilva

Poem

🐰 Polyfills removed with care and grace,
Types refined in the proper place,
Dependencies dance to a newer tune,
Room objects shine beneath the moon! 🌙

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: Handle props spreading weaknesses' directly matches the PR's primary objective of addressing props spreading weaknesses across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/props-spreading-weakness

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e1682d2 and c74fdee.

📒 Files selected for processing (6)
  • apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (2 hunks)
  • apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (2 hunks)
  • packages/fuselage-ui-kit/README.md (1 hunks)
  • packages/fuselage-ui-kit/package.json (1 hunks)
  • packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1 hunks)
  • packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx
  • packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx
🔇 Additional comments (9)
packages/fuselage-ui-kit/README.md (1)

28-32: LGTM - Documentation updated to reflect dependency removal.

The removal of @rocket.chat/fuselage-polyfills from the installation instructions correctly aligns with its removal from package.json.

apps/meteor/client/views/omnichannel/components/AutoCompleteDepartmentMultiple.tsx (2)

4-4: LGTM - Import simplification.

The change from ReactElement, ComponentPropsWithoutRef to ComponentProps simplifies the typing while maintaining functionality.


69-81: LGTM - Inlined renderItem logic improves clarity.

Inlining the renderItem logic directly into the prop eliminates unnecessary function abstraction while preserving the conditional rendering behavior for CheckOption vs Option.

packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/ChannelsSelectElement.tsx (1)

37-37: Excellent fix - Property ordering prevents type field override.

The reordering from { type: label?.type || 'c', _id: value, ...label } to { _id: value, ...label, type: label?.type || 'c' } ensures that the type field is always explicitly set after spreading label. This prevents label from potentially containing a type property that overrides the fallback value 'c', which was the props spreading weakness this PR addresses.

apps/meteor/client/views/omnichannel/components/AutoCompleteMonitors.tsx (2)

3-3: LGTM - Import cleanup.

Removing the Option type import simplifies the component's type dependencies while maintaining functionality.


34-36: LGTM - Simplified renderItem typing.

The change removes explicit Option type dependency in renderItem while correctly ignoring the ref parameter with the _ref naming convention.

packages/fuselage-ui-kit/src/elements/ChannelsSelectElement/MultiChannelsSelectElement.tsx (2)

38-38: Excellent fix - Property ordering prevents type field override in renderSelected.

The reordering to { _id: value, ...label, type: label?.type || 'c' } ensures the type field is always explicitly set after spreading label, preventing props spreading from overriding the fallback value. This matches the fix applied in ChannelsSelectElement.tsx.


49-49: Excellent fix - Consistent property ordering in renderItem.

The same property ordering fix is correctly applied here, ensuring consistency across both renderSelected and renderItem functions.

packages/fuselage-ui-kit/package.json (1)

56-59: The review comment references a file that was not modified in this PR.

This PR only changes component files (AutoCompleteDepartmentMultiple.tsx and AutoCompleteMonitors.tsx). The file packages/fuselage-ui-kit/package.json has no modifications compared to the main branch, so the review comment is analyzing an unchanged file. The dependency versions listed (fuselage@~0.68.1, fuselage-hooks@~0.38.1, fuselage-tokens@~0.33.2, icons@~0.45.0) are already present in the codebase without any updates in this PR.

Likely an incorrect or invalid review comment.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tassoevan tassoevan force-pushed the chore/props-spreading-weakness branch from 4ebe5bc to c74fdee Compare November 15, 2025 04:17
@codecov
Copy link

codecov bot commented Nov 15, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 68.97%. Comparing base (0691514) to head (c74fdee).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #37509   +/-   ##
========================================
  Coverage    68.97%   68.97%           
========================================
  Files         3358     3358           
  Lines       114240   114228   -12     
  Branches     20537    20537           
========================================
- Hits         78792    78784    -8     
+ Misses       33359    33352    -7     
- Partials      2089     2092    +3     
Flag Coverage Δ
e2e 57.45% <75.00%> (+0.01%) ⬆️
e2e-api 42.79% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tassoevan tassoevan added this to the 7.13.0 milestone Nov 16, 2025
@tassoevan tassoevan marked this pull request as ready for review November 16, 2025 04:03
@tassoevan tassoevan requested a review from a team as a code owner November 16, 2025 04:03
@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Nov 17, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Nov 17, 2025
@kodiakhq kodiakhq bot merged commit 744756c into develop Nov 17, 2025
87 of 89 checks passed
@kodiakhq kodiakhq bot deleted the chore/props-spreading-weakness branch November 17, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants