Skip to content

refactor(livechat): finish TypeScript migration and convert components to hooks - #41559

Merged
ggazzo merged 39 commits into
developfrom
refactor/livechat-js
Jul 31, 2026
Merged

refactor(livechat): finish TypeScript migration and convert components to hooks#41559
ggazzo merged 39 commits into
developfrom
refactor/livechat-js

Conversation

@tassoevan

@tassoevan tassoevan commented Jul 24, 2026

Copy link
Copy Markdown
Member

Proposed changes (including videos or screenshots)

Finishes migrating the @rocket.chat/livechat widget to TypeScript and modernizes its components. This is a refactor — no end-user behavior change is intended; the class components were ported with care to preserve their exact runtime semantics (see Further comments).

TypeScript conversion — the tree under packages/livechat/src is now 100% .ts/.tsx:

  • routes/Chat, the lib modules (main, room, threads, triggers, uiKit), components/Sound, the components/uiKit index, and components/Messages (Message, MessageList).
  • A few StoreState fields these modules use at runtime but weren't typed were added (e.g. queueInfo), ScreenContext was fully typed (dropping a dead as ScreenContextValue cast and unused nameDefault/emailDefault/departmentDefault), stale global.d.ts augmentations were removed, and @types/emoji-mart was added. Several latent bugs surfaced by the type checker were fixed and called out in their commits.

Class → function components (hooks):

  • Chat route — collapsed the connector/container/component trio into a single function component, lifting state and handlers into hooks (useChatEffects, useChatSubscriptions, useStableCallback) and splitting the view into ChatContent and ChatFooter.
  • App — converted to a function component; translations now come from useTranslation instead of the withTranslation HOC, with the i18next instance provided via I18nextProvider at the connector root.
  • TriggerMessage, ChatFinished, GDPRAgreement — collapsed into single function components.
  • Messages (Message, MessageSeparator, MessageTime, Audio/VideoAttachment) and the Screen HeaderwithTranslationuseTranslation.

Tooling / build:

  • Typecheck runs against tsconfig.json (tsc --noEmit) instead of a duplicate tsconfig.typecheck.json.
  • The repo-root postcss.config.js (only consumed by livechat) is scoped into the package and postcss-css-variables is dropped — every browser in livechat's browserslist supports CSS custom properties natively, which also clears the "variable … used without a fallback" build warnings.
  • Storybook: fixed a node:stream unhandled-scheme build failure by normalizing node: specifiers in the webpack config, and added a reusable storeDecorator that feeds arbitrary store state to stories via StoreContext.

Issue(s)

N/A

Steps to test or reproduce

yarn workspace @rocket.chat/livechat typecheck
yarn workspace @rocket.chat/livechat eslint
yarn workspace @rocket.chat/livechat build
yarn workspace @rocket.chat/livechat build-storybook

All pass locally. Because the widget has no unit coverage for these paths, correctness rests on the omnichannel E2E suite (Test UI (CE) / Test UI (EE)), which exercises the widget end-to-end.

Further comments

No changeset: this is an internal refactor with no user-facing change, so nothing to surface in release notes.

The class → function conversions are behavior-preserving by design, which required two deliberate choices where the class semantics are subtle:

  • Lifecycle timingcomponentDidMount/componentDidUpdate were ported with useLayoutEffect, not useEffect. Preact runs useEffect asynchronously and coalesces it; the widget's edge-triggered logic (e.g. the waiting-queue one-shot) needs the synchronous, per-commit timing the class had.
  • Live state reads — a class method reads this.props live at call time, whereas a closure captures stale render values. Effects and the router handler read the current store.state at call time so post-init data (config, guest/room, triggers) is seen correctly.

There are two reasons for this pull request to exist. The first is i18next upgrades being blocked because hoisting dependencies doesn't work for Livechat (reason is unclear). The second is that moving away from JavaScript eases the migration for TypeScript 7.

Review in cubic

Summary by CodeRabbit

  • New Features

    • Introduced refreshed chat, GDPR consent, trigger-message, and chat-finished experiences.
    • Added improved chat messaging, typing indicators, emoji selection, file uploads, queue updates, and alert handling.
    • Added support for audio notifications and more flexible widget configuration.
    • Added localized content handling across chat screens and message attachments.
  • Bug Fixes

    • Prevented one-time triggers from firing repeatedly.
    • Corrected modal-close interaction handling and improved chat state resets.

Task: ARCH-2322

@dionisio-bot

dionisio-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

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

@changeset-bot

changeset-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 63efbe3

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

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The livechat package was migrated toward typed, hook-based components. The Chat route and supporting routes were rebuilt, screen/store/server contexts were expanded, message translation HOCs were removed, Storybook decorators were added, and build configuration was updated.

Changes

Livechat runtime and chat flow

Layer / File(s) Summary
Runtime providers and context
packages/livechat/src/components/App/*, packages/livechat/src/components/Screen/*, packages/livechat/src/providers/*, packages/livechat/src/store/*
Application initialization, server URL wiring, screen context typing, sound playback, store state typing, and global declarations were updated.
Chat route and lifecycle
packages/livechat/src/routes/Chat/*
The Chat route now composes typed content, footer, emoji picker, subscriptions, queue effects, message loading, uploads, and chat actions.
Message components
packages/livechat/src/components/Messages/*
Message components use useTranslation, explicit prop types, typed scrolling, and updated attachment/typing props.
Supporting libraries
packages/livechat/src/lib/*, packages/livechat/src/components/uiKit/*
Room, thread, trigger, and UI kit helpers received stronger typing and targeted interaction fixes.
Secondary routes
packages/livechat/src/routes/ChatFinished/*, GDPRAgreement/*, LeaveMessage/*, Register/*, SwitchDepartment/*, TriggerMessage/*
Secondary routes were replaced or refactored as direct typed components using store and translation hooks.
Storybook and build configuration
packages/livechat/.storybook/*, packages/livechat/webpack.config.ts, packages/livechat/postcss.config.js, packages/livechat/package.json
Storybook decorators and story args now use explicit state types; CSS, PostCSS, webpack, dependencies, and typecheck configuration were adjusted.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested labels: type: chore

Suggested reviewers: ggazzo, gabriellsh, dougfabris

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: completing the TypeScript migration and converting components to hooks.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks

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.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.72%. Comparing base (772d8ca) to head (63efbe3).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #41559      +/-   ##
===========================================
- Coverage    68.73%   68.72%   -0.02%     
===========================================
  Files         4151     4151              
  Lines       159513   159513              
  Branches     27923    27978      +55     
===========================================
- Hits        109644   109622      -22     
- Misses       44698    44716      +18     
- Partials      5171     5175       +4     
Flag Coverage Δ
e2e 58.82% <ø> (+0.01%) ⬆️
e2e-api 45.69% <ø> (-0.03%) ⬇️
unit 70.69% <ø> (-0.03%) ⬇️

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 force-pushed the refactor/livechat-js branch 5 times, most recently from 84c13aa to 0398d68 Compare July 29, 2026 05:31
tassoevan and others added 21 commits July 30, 2026 02:33
Drop the separate tsconfig.typecheck.json and point the `typecheck`
script at the package's tsconfig.json with `--noEmit`. The extra config
only added skipLibCheck (already in the shared base config) and noEmit
(now passed as a CLI flag), so it was redundant.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the Chat route's component and container modules from JS to TS.

Add @types/emoji-mart (pinned to ~3.0.14 to match emoji-mart@3; the v5
line is a deprecated stub) so the lazy-loaded Picker and its onSelect
EmojiData are properly typed. The real types surfaced that `native`
only exists on BaseEmoji, so the emoji-select handler now narrows before
reading it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert main, room, threads, triggers and uiKit under src/lib from JS to
TS. Add the store fields these modules use at runtime but that were
missing from StoreState (config.settings.clearLocalStorageWhenChatEnded,
config.settings.agentHiddenInfo, parentMessages, triggersRecords) and
widen `room` to allow `servedBy` and `null`.

Message payloads stay `any`, matching the store's existing convention,
and the strongly-typed SDK calls (config, loadMessages, message,
sendUiInteraction) are cast at the boundary since their REST-derived
param types are stricter than the loose runtime usage.

A few latent bugs surfaced by the type checker were fixed and flagged
(trigger record key `id` -> `_id`, always-false `_isValid` comparison,
`MODAL_ClOSE` typo); the no-op async `filter` in normalizeMessages was
left as-is behind a FIXME to avoid changing message normalization.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Type the Sound component's props and audio ref. Drop the `type` attribute
from the <audio> element (it is only meaningful on <source>, so the
browser ignored it) to satisfy the JSX types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…cript

Convert the two remaining JS modules under components/Messages (Message
and MessageList) to TSX. Message payloads stay `any`, matching the
store's convention.

Declare the props that sibling components already forward via spread but
did not type: `quoted` on Image/FileAttachment and `use` on
TypingIndicator. `avatarResolver` is passed with a `() => undefined`
fallback where those components type it as required (they have the same
runtime default).

The original passed `inverse={me}` to MessageTime, which only reads
`inverted` — so it was a no-op; dropped rather than enabling the
timestamp styling, to keep rendering identical.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the repo-root postcss.config.js into packages/livechat (its only
consumer) so it no longer acts as a global default for other PostCSS
consumers. Drop the postcss-css-variables plugin (and dependency): all
browsers in livechat's browserslist support CSS custom properties
natively, so build-time var() fallbacks are unnecessary. This also
removes the "variable ... is undefined and used without a fallback"
warnings emitted for third-party CSS during the build.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Storybook's preview build failed with "Reading from node:stream is not
handled by plugins". The config already stubs core modules via
resolve.fallback (stream: false), but that only matches the bare
specifier -- webpack 5 treats the node:-prefixed form as an unhandled
URI scheme. Strip the node: prefix in webpackFinal so those imports
(reached through the preact react-dom/server shim) hit the existing
fallbacks.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the withTranslation class component with a function component
using useTranslation/useContext/useRef, and simplify the container to
useStore(), dropping the now-unused agent/unread/theme/formatAgent/ref
plumbing. Stories updated to the new handleChatStartClick prop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Switch SwitchDepartment to the useStore() hook (dropping the manual
useContext(StoreContext) and the now-unneeded StoreState casts) and make
its route `path` prop optional. Update the story to inject config
(departments) and loading through the shared storeDecorator instead of
passing props the component ignores.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reference the exported RegisterProps type directly instead of deriving it
via ComponentProps<typeof Register>.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hoist the form values into a named LeaveMessageFormValues type and pass
it to useForm, which lets the submit handler drop the
`as unknown as JSXInternal.GenericEventHandler` cast. Make the route
`path` optional and reference LeaveMessageProps directly in the story.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ponent

Merge the GDPRAgreement component/container/index into one function
component that uses useTranslation and useStore, dropping the
withTranslation HOC. Behavior is unchanged: the container never passed
`instructions`, so the Trans fallback was always rendered. Story now
references GDPRAgreementProps and drops the props the component ignored.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…onent

Merge the ChatFinished component/container/index into one function
component that uses useTranslation and useStore (dropping the
withTranslation HOC and the separate container). Behavior is unchanged.
Story now injects the finished-conversation messages through the shared
storeDecorator instead of passing props the component no longer takes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Split the combined `.s?css` webpack and storybook rules into distinct
`.css` and `.scss` rules, and add a `*.css` module declaration so plain
CSS imports type-check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Make the Chat route's default export the connector, import the container
directly in the connector instead of via the barrel, and update App to
render the default Chat export.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Turn the Chat connector into a function component that reads state via
useStore and translations via useTranslation, passing t/i18n down to the
container (which also drops its own withTranslation wrapper), and extract
the room subscription hooks into a useChatSubscriptions hook. Widen the
presentational avatarResolver type to allow null and align the story
args with the current ChatProps.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Mark ChatContainer's methods private, hold the mutable inner state in a
ref instead of a plain field, and drop the no-op awaits on the
synchronous dispatch. Remove the unused onBottom prop/callback that was
never wired past the story.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the lazy emoji-mart Picker (and its CSS import) into its own module,
remove dead ref fields from the Chat component, and store the emoji
callback in a ref. Thread translations through the `t` prop only,
dropping the `i18n` prop from the container, and read the `uploads` prop
instead of reaching into store.state in handleUpload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop withTranslation from the Chat component and container (threading t
through props with an identity fallback), move the subscriptions and the
register/switch-department route handlers into the connector, and convert
the container/component methods to bound arrow fields. Type queueInfo in
StoreState and clear it with undefined instead of null in room.ts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the container's mutable inner state (connecting/queue dedup
tracking) into a useRef owned by the connector, passed down as an
innerStateRef prop, and forward explicit props to the presentational
Chat instead of spreading so container-only props don't leak through.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tassoevan and others added 15 commits July 30, 2026 02:33
Extract checkRoom and grantUser from the ChatContainer class into the
connector as useStableCallback hooks, passed down as props. The container
keeps its lifecycle but now calls these via props, and drops its direct
store import. Enable the useStableCallback helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Lift the remaining handlers and derived flags (getRoom, onSubmit, onUpload,
typing helpers, handleConnectingAgentAlert, handleQueueMessage,
checkConnectingAgent, can* flags, etc.) into the connector as stable
callbacks, passed to the container as props. The container keeps its class
lifecycle (componentDidMount/Update/Unmount) and just orchestrates them and
renders the presentational component.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The connector now renders the presentational component directly and keeps
ChatContainer purely for its class lifecycle (render returns null),
rendering both as siblings. Trim ChatContainerProps to just what the
lifecycle needs, dropping the handler props it no longer uses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the component's text/atBottom/emojiPickerActive state and its input
handlers (scroll, submit, change-text, upload-click, emoji toggle) into
the connector as useState + useStableCallback, passed down as props. The
Chat component becomes effectively stateless. Handler bodies are
preserved and use stable callbacks to avoid stale closures.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move handleEmojiSelect and handleEmojiClick out of the presentational
Chat class into the connector as stable callbacks, and drop the dead
onTop/onSubmit/onChangeText props.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collapse the Chat route's connector.tsx and component.tsx into a single
index.tsx, dropping the index.ts re-export shim. The presentational
markup now renders inline in the connector, and stories import from the
consolidated module.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace the lifecycle-only ChatContainer class with equivalent effects in
the Chat function component: a mount/unmount effect for
checkConnectingAgent/loadMessages/processUnread and the connecting-agent
teardown, and a firstRender-guarded effect mirroring componentDidUpdate,
using usePrevious for the previous messages/alerts.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the Chat route's mount/update side effects (connecting-agent and
waiting-queue handling, unread marking, message/room loading) out of the
component body into a dedicated useChatEffects hook, and split
useStableCallback into its own module. Both effects use useLayoutEffect
so the edge-triggered queue logic keeps its synchronous per-commit timing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extract the message list into ChatContent and the composer/options into
ChatFooter, and move the lifecycle hook into its own useChatEffects module.
Chat now owns the shared inputRef/notifyEmojiSelectRef, the emoji-picker
state, and the upload flow, passing handlers down to the two subcomponents.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Convert the Message, MessageSeparator, MessageTime, Audio/VideoAttachment
and Screen Header components to consume translations via the useTranslation
hook instead of the withTranslation HOC, dropping the injected `t` prop.
Unwrapping the components tightened their exported prop types, so type the
alerts field on ScreenContextValue and fix the MessageSeparator/MessageTime
stories to pass string/number args.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Port the App class to a function component using the useTranslation hook
instead of the withTranslation HOC. The mount/unmount and document-dir
lifecycle move to useLayoutEffect to preserve the class's synchronous
timing and the original initialize() ordering, and AppProps drops the
HOC-injected i18n prop while relaxing user/iframe to the store types the
connector actually passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Converting App to a function component replaced withTranslation with
useTranslation, which left the widget root without an i18next instance or
a Suspense boundary, crashing the widget on mount. Wrap the connector in
I18nextProvider and Suspense, load the i18next config there, and pass the
server URL down as a prop so ServerProvider no longer imports it from the
App module (removing the import cycle).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fully type the ScreenContext default value, narrow sound/modal/onRestore/
onDismissAlert, and remove the unused nameDefault/emailDefault/
departmentDefault fields. Drop the now-unused global.d.ts augmentations
(withTranslation, window handleIframeClose/expandCall, preact and storybook
module hacks) and type the uiKit BlockContext. Update the Storybook
screenProps helper to the new shape.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The App mount effect ran once with the mount-render closure, whose config
is still the initial empty state, so config.online/config.enabled were
undefined and Triggers.init() was never called — livechat triggers never
fired. Read config/minimized/iframe/undocked from store.state at call time
(after Connection.init has loaded the config), matching the class
component's live this.props behavior.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
handleRoute ran its checks inside a setTimeout and read user/config/gdpr
from the render closure. When setGuestToken logs a guest in, route('/')
fires synchronously from the room store subscription before App re-renders,
so the stale closure still saw no user token and redirected the logged-in
guest to /register — unmounting Chat before its loaded history could show
(breaking OC - Livechat API - setGuestToken). Read config/gdpr/user from
store.state at timeout time, matching the class component's live this.props.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tassoevan
tassoevan force-pushed the refactor/livechat-js branch from 795da4b to 63efbe3 Compare July 30, 2026 05:33
@tassoevan tassoevan added this to the 8.8.0 milestone Jul 30, 2026
@tassoevan tassoevan changed the title refactor(livechat): migrate remaining modules to TypeScript refactor(livechat): finish TypeScript migration and convert components to hooks Jul 30, 2026
@tassoevan
tassoevan marked this pull request as ready for review July 30, 2026 06:55
@tassoevan
tassoevan requested review from a team as code owners July 30, 2026 06:55

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5 issues found across 69 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="packages/livechat/.storybook/helpers.tsx">

<violation number="1" location="packages/livechat/.storybook/helpers.tsx:52">
P2: Story interactions that call `dispatch` update the singleton store but leave this Provider's context snapshot unchanged, so consumers do not reflect actions until Storybook happens to rerender. Subscribe the decorator to store changes (or provide a stateful wrapper) and rebuild its context value from current store state.</violation>
</file>

<file name="packages/livechat/src/routes/TriggerMessage/index.tsx">

<violation number="1" location="packages/livechat/src/routes/TriggerMessage/index.tsx:34">
P2: Triggered widgets now resize only after paint, so the first render can briefly use stale/default wrapper dimensions. Use `useLayoutEffect` for this synchronous DOM measurement and resize, matching the migrated lifecycle timing used elsewhere.</violation>
</file>

<file name="packages/livechat/src/routes/Chat/ChatFooter.tsx">

<violation number="1" location="packages/livechat/src/routes/Chat/ChatFooter.tsx:100">
P2: First message from an unregistered visitor sends typing-stop activity with an empty username. `queryRoomId()` updates the visitor during its await, but this callback retains the pre-grant `user`; read current store state after it resolves before notifying activity.</violation>
</file>

<file name="packages/livechat/src/components/App/index.tsx">

<violation number="1" location="packages/livechat/src/components/App/index.tsx:17">
P2: Widgets can remain in the browser locale when server-configured language differs, because i18next may initialize before `Connection.init()` has loaded config and never re-reads it. Initialize/change i18next only after configured language is available.</violation>
</file>

<file name="packages/livechat/src/routes/Chat/Picker.tsx">

<violation number="1" location="packages/livechat/src/routes/Chat/Picker.tsx:3">
P1: The lazy Picker will never render because the loader returns the component function directly instead of a module with a default export. lazy(PickerFunction) → .default evaluates to undefined, so the emoji picker component resolves to nothing at render time.

Return the import directly instead of destructuring it: return import('emoji-mart');</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@@ -0,0 +1,9 @@
import { lazy } from 'preact/compat';

const Picker = lazy(async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The lazy Picker will never render because the loader returns the component function directly instead of a module with a default export. lazy(PickerFunction) → .default evaluates to undefined, so the emoji picker component resolves to nothing at render time.

Return the import directly instead of destructuring it: return import('emoji-mart');

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/livechat/src/routes/Chat/Picker.tsx, line 3:

<comment>The lazy Picker will never render because the loader returns the component function directly instead of a module with a default export. lazy(PickerFunction) → .default evaluates to undefined, so the emoji picker component resolves to nothing at render time.

Return the import directly instead of destructuring it: return import('emoji-mart');</comment>

<file context>
@@ -0,0 +1,9 @@
+import { lazy } from 'preact/compat';
+
+const Picker = lazy(async () => {
+	await import('emoji-mart/css/emoji-mart.css');
+	const { Picker } = await import('emoji-mart');
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a valid finding — the current code is correct, and this change would break it.

The assumption here is React's lazy, which strictly requires { default: Component }. But this imports lazy from preact/compat, which resolves the loader result differently. From preact@10.25.4 (preact/compat, the lazy factory):

(e = t()).then(function (n) { r = n.default || n }, ...)
// ...
return createElement(r, props);

The key is r = n.default || n — Preact falls back to the resolved value itself when there's no .default. So returning the component directly works:

  • n = Picker
  • n.default = undefined
  • r = undefined || PickerPicker

So the ".default is undefined → renders nothing" chain doesn't hold; the || n fallback (which React doesn't have) covers it.

Applying the suggestion would actually break rendering. import('emoji-mart') resolves to the module namespace, and since emoji-mart v3 is CommonJS, under webpack interop namespace.default is the entire module, not Picker. Preact would then render createElement(<the emoji-mart module>) → nothing/error.

Keeping const { Picker } = await import('emoji-mart'); return Picker;, which is the correct pattern for preact/compat's lazy.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent comment was wrong here: preact/compat’s lazy accepts the resolved component directly via its n.default || n fallback, so return Picker; is correct. Returning import('emoji-mart') would be the broken version because it can resolve to the module namespace instead of the Picker component.

value={{
...store.state,
...args,
dispatch: store.setState.bind(store),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Story interactions that call dispatch update the singleton store but leave this Provider's context snapshot unchanged, so consumers do not reflect actions until Storybook happens to rerender. Subscribe the decorator to store changes (or provide a stateful wrapper) and rebuild its context value from current store state.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/livechat/.storybook/helpers.tsx, line 52:

<comment>Story interactions that call `dispatch` update the singleton store but leave this Provider's context snapshot unchanged, so consumers do not reflect actions until Storybook happens to rerender. Subscribe the decorator to store changes (or provide a stateful wrapper) and rebuild its context value from current store state.</comment>

<file context>
@@ -5,30 +5,59 @@ import { action } from 'storybook/actions';
+		value={{
+			...store.state,
+			...args,
+			dispatch: store.setState.bind(store),
+			on: store.on.bind(store),
+			off: store.off.bind(store),
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mechanism is described accurately — storeDecorator builds the context value once from store.state and doesn't subscribe to change, so a dispatch during a story's lifetime won't propagate through this Provider until Storybook re-renders (which it does on args/controls changes, not on store mutations). But this is intended, and reactivity isn't the right fix here:

  • It's a static fixture by design. The decorator's job (see the comment above it) is to seed useStore() consumers with an args-driven snapshot so a component can be rendered in a specific state — the same pattern as screenDecorator/screenProps() right above, which hands over static action() stubs. Stories render a state; they don't drive it.

  • args intentionally win over store state (...store.state, ...args). Making the Provider reactive wouldn't change that: dispatched updates to any args-controlled key would still be overridden, so a subscription only "helps" for keys a story deliberately left uncontrolled — which is not a scenario any current story relies on.

  • Interactivity is already bounded by the absent server, not by this snapshot. These stories render without ServerProvider, so the flows that dispatch (send message → grantUser/getRoom, subscriptions) are no-ops/early-returns anyway. dispatch/on/off are still provided so components that call them don't throw; nothing here expects a dispatch to re-render the tree.

  • A live subscription would make stories less deterministic. store is a module singleton, so subscribing would surface dispatched state that then bleeds into the next story's initial snapshot. The one-shot snapshot keeps each story's context stable for its lifetime.

If we later add play-function interaction tests that assert on post-dispatch UI, the right move is a small stateful wrapper that mirrors the production StoreProvider (subscribe to change, re-snapshot, re-merge args on top) — but that's an enhancement for that use case, not a correctness fix for the current stories. Leaving as-is.

theme: { color },
} = useContext(ScreenContext);

useEffect(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Triggered widgets now resize only after paint, so the first render can briefly use stale/default wrapper dimensions. Use useLayoutEffect for this synchronous DOM measurement and resize, matching the migrated lifecycle timing used elsewhere.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/livechat/src/routes/TriggerMessage/index.tsx, line 34:

<comment>Triggered widgets now resize only after paint, so the first render can briefly use stale/default wrapper dimensions. Use `useLayoutEffect` for this synchronous DOM measurement and resize, matching the migrated lifecycle timing used elsewhere.</comment>

<file context>
@@ -0,0 +1,75 @@
+		theme: { color },
+	} = useContext(ScreenContext);
+
+	useEffect(() => {
+		parentCall('resetDocumentStyle');
+	}, []);
</file context>


try {
stopTypingDebounced.stop();
await Promise.all([stopTyping({ rid, username: user?.username ?? '' }), Livechat.sendMessage({ msg, token, rid })]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: First message from an unregistered visitor sends typing-stop activity with an empty username. queryRoomId() updates the visitor during its await, but this callback retains the pre-grant user; read current store state after it resolves before notifying activity.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/livechat/src/routes/Chat/ChatFooter.tsx, line 100:

<comment>First message from an unregistered visitor sends typing-stop activity with an empty username. `queryRoomId()` updates the visitor during its await, but this callback retains the pre-grant `user`; read current store state after it resolves before notifying activity.</comment>

<file context>
@@ -0,0 +1,275 @@
+
+		try {
+			stopTypingDebounced.stop();
+			await Promise.all([stopTyping({ rid, username: user?.username ?? '' }), Livechat.sendMessage({ msg, token, rid })]);
+		} catch (error: any) {
+			const reason = error?.error ?? error.message;
</file context>

export const useSsl = Boolean((Array.isArray(host) ? host[0] : host)?.match(/^https:/));
const AppConnector = () => {
useEffect(() => {
void import('../../i18next');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Widgets can remain in the browser locale when server-configured language differs, because i18next may initialize before Connection.init() has loaded config and never re-reads it. Initialize/change i18next only after configured language is available.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/livechat/src/components/App/index.tsx, line 17:

<comment>Widgets can remain in the browser locale when server-configured language differs, because i18next may initialize before `Connection.init()` has loaded config and never re-reads it. Initialize/change i18next only after configured language is available.</comment>

<file context>
@@ -1,41 +1,66 @@
-export const useSsl = Boolean((Array.isArray(host) ? host[0] : host)?.match(/^https:/));
+const AppConnector = () => {
+	useEffect(() => {
+		void import('../../i18next');
+	}, []);
 
</file context>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/livechat/src/lib/threads.ts (1)

79-87: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Await normalization before filtering.

filter(async ...) keeps every item because each predicate returns a truthy Promise. loadMessages and loadMoreMessages therefore receive unnormalized thread containers, while child-thread normalization races in the background. Normalize with Promise.all, then remove null results.

Proposed fix
-export const normalizeMessages = (messages: any[] = []): Promise<any[]> =>
-	Promise.all(
-		// FIXME: the async predicate makes `filter` keep every message (a Promise is always truthy), so no
-		// filtering actually happens here. Preserved as-is during the JS->TS migration; revisit separately.
-		// eslint-disable-next-line `@typescript-eslint/no-misused-promises`
-		messages.filter(async (message) => {
-			const result = await normalizeMessage(message);
-			return result;
-		}),
-	);
+export const normalizeMessages = async (messages: any[] = []): Promise<any[]> => {
+	const normalizedMessages = await Promise.all(messages.map(normalizeMessage));
+	return normalizedMessages.filter((message) => message !== null);
+};
🤖 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 `@packages/livechat/src/lib/threads.ts` around lines 79 - 87, Update
normalizeMessages to await normalizeMessage for every message via Promise.all,
then filter out null results before returning the normalized array. Remove the
async predicate passed to messages.filter and preserve the existing default
empty-array behavior so loadMessages and loadMoreMessages receive completed
normalization results.

Source: Coding guidelines

packages/livechat/src/providers/ServerProvider.tsx (1)

147-161: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Satisfy ServerContextValue or narrow the context type.

The livechat provider still casts an incomplete object to ServerContextValue, which requires getStreamAll, writeStream, disconnect, and retryCount. The current implementation provides getSingleStream instead and omits the other required members, so removing the cast will break type-checking; replace it with a narrowed interface if livechat truly only needs these fields, or add no-op/real implementations for the remaining members.

🤖 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 `@packages/livechat/src/providers/ServerProvider.tsx` around lines 147 - 161,
The contextValue object in the livechat provider does not satisfy
ServerContextValue and relies on an unsafe cast. Either define and use a
narrowed context type containing only the fields livechat consumers require, or
implement the required getStreamAll, writeStream, disconnect, and retryCount
members while preserving the existing getSingleStream API; remove the unknown
cast and ensure the provider’s context type matches the returned value.
🧹 Nitpick comments (8)
packages/livechat/src/routes/Register/index.tsx (1)

134-134: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Keep the handleSubmit callbacks typed to their form values.

The broad SubmitHandler<FieldValues> cast discards the useForm<T> contracts and lets type errors in the callback parameters go unnoticed.

  • packages/livechat/src/routes/Register/index.tsx#L134: type onSubmit as SubmitHandler<RegisterFormValues> / RegisterFormValues and avoid defining customFields as a nested value; custom fields come from rest destructuring.
  • packages/livechat/src/routes/LeaveMessage/index.tsx#L107: pass onSubmit directly to handleSubmit using LeaveMessageFormValues instead of casting to SubmitHandler<FieldValues>.
🤖 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 `@packages/livechat/src/routes/Register/index.tsx` at line 134, Preserve the
form-value type contracts in both submit flows: in
packages/livechat/src/routes/Register/index.tsx at lines 134-134, type onSubmit
with SubmitHandler<RegisterFormValues> / RegisterFormValues, remove the broad
SubmitHandler<FieldValues> cast, and obtain customFields through rest
destructuring rather than a nested value; in
packages/livechat/src/routes/LeaveMessage/index.tsx at lines 107-107, pass
onSubmit directly to handleSubmit and use LeaveMessageFormValues instead of
casting to SubmitHandler<FieldValues>.
packages/livechat/.storybook/helpers.tsx (1)

45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the new implementation comments. The repository rule forbids comments in TS/TSX implementation code.

  • packages/livechat/.storybook/helpers.tsx#L45-L46: remove the store-decorator rationale comment.
  • packages/livechat/src/routes/Chat/stories.tsx#L18-L18: remove the agent-fixture rationale comment.

As per coding guidelines, “Avoid code comments in the implementation.”

🤖 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 `@packages/livechat/.storybook/helpers.tsx` around lines 45 - 46, Remove the
implementation comments at packages/livechat/.storybook/helpers.tsx lines 45-46
and packages/livechat/src/routes/Chat/stories.tsx line 18. Leave the surrounding
store decorator and agent-fixture code unchanged.

Source: Coding guidelines

packages/livechat/.storybook/main.ts (1)

114-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the added implementation comments.

These new comments violate the repository rule against implementation comments. As per coding guidelines, “Avoid code comments in the implementation.”

🤖 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 `@packages/livechat/.storybook/main.ts` around lines 114 - 116, Remove the
added implementation comment describing webpack 5, the node:stream scheme, and
resolve fallbacks, while leaving the surrounding configuration and behavior
unchanged.

Source: Coding guidelines

packages/livechat/src/components/Messages/ImageAttachment/index.tsx (1)

7-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Inconsistent typing approach vs. sibling AudioAttachmentProps.

AudioAttachmentProps in this same PR was changed to compose { url; className? } & MessageBubbleProps, but ImageAttachmentProps only tacks on a standalone quoted?: boolean field instead of intersecting with MessageBubbleProps. Functionally fine since the rest is spread through, but this diverges from the pattern established for AudioAttachment and leaves other bubble props (nude, inverse, style, etc.) untyped here.

♻️ Suggested consistency fix
+import type { MessageBubbleProps } from '../MessageBubble';
+
 export type ImageAttachmentProps = {
 	url: string;
 	className?: string;
-	quoted?: boolean;
-};
+} & MessageBubbleProps;
🤖 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 `@packages/livechat/src/components/Messages/ImageAttachment/index.tsx` around
lines 7 - 11, Update ImageAttachmentProps to intersect the existing { url;
className? } shape with MessageBubbleProps, matching AudioAttachmentProps;
remove the standalone quoted field and rely on MessageBubbleProps to type quoted
and the other bubble properties.
packages/livechat/src/components/Messages/VideoAttachment/index.tsx (1)

18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove implementation comments prohibited by project guidance.

  • packages/livechat/src/components/Messages/VideoAttachment/index.tsx#L18-L18: replace the inline suppression with actual caption support or a scoped lint configuration exception.
  • packages/livechat/src/lib/uiKit.ts#L26-L26: delete the stale commented declaration.
🤖 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 `@packages/livechat/src/components/Messages/VideoAttachment/index.tsx` at line
18, Remove the inline jsx-a11y suppression in the VideoAttachment component and
provide actual caption support, or replace it with an appropriately scoped lint
configuration exception. Also delete the stale commented declaration in
packages/livechat/src/lib/uiKit.ts at line 26; both locations must no longer
contain prohibited implementation comments.

Source: Coding guidelines

packages/livechat/src/components/App/App.tsx (2)

62-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

New multi-line explanatory comments added throughout the ported lifecycle logic.

As per coding guidelines, **/*.{ts,tsx,js} files should "Avoid code comments in the implementation," but this segment adds several new multi-sentence comment blocks (state-read rationale, componentDidMount/Update emulation notes, TODO). Consider moving this rationale into the PR description/commit message or a short doc rather than inline prose, per the guideline.

🤖 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 `@packages/livechat/src/components/App/App.tsx` around lines 62 - 168, Remove
the newly added multi-line explanatory comments from handleRoute and the
lifecycle useLayoutEffect blocks, including the lifecycle emulation notes and
TODO. Keep the implementation behavior unchanged and retain only comments
required by tooling or essential concise context.

Source: Coding guidelines


57-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

AppProps fields other than dispatch are no longer consumed by App. Since App now reads config/gdpr/user/etc. fresh from store.state inside handleRoute and the mount effect instead of via props, most of what StoreConsumer forwards through AppConnector is now dead plumbing that only causes App to re-render on every store change without using the data.

  • packages/livechat/src/components/App/App.tsx#L57-L102: narrow AppProps down to just { dispatch: Dispatch } (or whatever subset is genuinely still read) now that the rest is unused.
  • packages/livechat/src/components/App/index.tsx#L15-L65: once AppProps is trimmed, stop destructuring/forwarding the now-unused fields (config, gdpr, triggered, user, sound, undocked, minimized, expanded, alerts, modal, iframe) from StoreConsumer into <App>.
🤖 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 `@packages/livechat/src/components/App/App.tsx` around lines 57 - 102, The App
component receives unused store-derived props, causing unnecessary re-renders.
In packages/livechat/src/components/App/App.tsx lines 57-102, narrow AppProps to
dispatch or the minimal fields actually read; in
packages/livechat/src/components/App/index.tsx lines 15-65, stop destructuring
and forwarding config, gdpr, triggered, user, sound, undocked, minimized,
expanded, alerts, modal, and iframe from StoreConsumer, while preserving the
dispatch prop passed to App.
packages/livechat/src/lib/main.ts (1)

8-19: 🚀 Performance & Scalability | 🔵 Trivial

Consider a shared typed adapter for Livechat.config() responses.

Both updateBusinessUnit and loadConfig independently cast the SDK response as any and destructure overlapping fields. Extracting a small typed normalizer (e.g. normalizeConfigResponse(raw): Partial<StoreState['config']> & {...}) would remove the duplicated any casts and give both call sites real typing.

Also applies to: 44-56

🤖 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 `@packages/livechat/src/lib/main.ts` around lines 8 - 19, Extract a shared
typed normalizer for Livechat.config() responses and use it in both
updateBusinessUnit and loadConfig. Replace their duplicated as-any casts and
overlapping destructuring with the normalizer’s typed result, returning the
appropriate Partial<StoreState['config']> shape while preserving existing field
behavior.
🤖 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/livechat/src/components/App/App.tsx`:
- Around line 104-160: Guard the asynchronous initialization in the mount
useLayoutEffect with an active/mounted flag, skipping initWidget,
setInitialized, and parentCall('ready') when Connection.init() resolves after
cleanup. Store the beforeunload callback in a named function or variable within
initWidget, remove it during effect cleanup, and ensure listeners are removed
whether initialization completes before or after unmount.

In `@packages/livechat/src/components/App/index.tsx`:
- Around line 12-13: Update the serverURL initialization in App so the
parse(window.location.search).serverUrl fallback is narrowed to a single string,
excluding null and string[] values before it reaches SDKProvider and
ServerProvider. Preserve the existing window.SERVER_URL precedence and
development fallback behavior.

In `@packages/livechat/src/components/Messages/Message/index.tsx`:
- Around line 139-140: Remove the explanatory NOTE comment above the MessageTime
render in the message component, leaving the existing conditional rendering and
props unchanged.

In `@packages/livechat/src/components/Screen/ScreenProvider.tsx`:
- Line 45: Restore the promise-based contract for ScreenContextValue.onRestore
and handleRestore: type onRestore as an async function returning a promise, make
handleRestore await the loadConfig/loadMessages/dispatch work directly, and
remove the detached async IIFE. Preserve TriggerMessage.handleStartChatClick’s
await behavior so route('/') runs only after restoration completes.

In `@packages/livechat/src/lib/room.ts`:
- Around line 21-36: Update closeChat to read department from
store.state.iframe.guest.department, using the nested defaults shown in the
review, instead of destructuring a top-level department from StoreState.
Preserve this value when setting the cleared state’s iframe.guest.

In `@packages/livechat/src/routes/Chat/index.tsx`:
- Around line 115-135: Update the error handling in doFileUpload to safely
handle rejected uploads whose error lacks a data object, defaulting reason and
sizeAllowed when necessary. Preserve the existing specialized messages for known
server reasons and ensure all other failures display the generic
fileupload_error alert without throwing from the catch block.
- Around line 62-74: Update grantUser to avoid assigning defaultDepartment
directly to guest. Build a new visitor/guest object with the default department
when needed, preserving the existing guest data and using that immutable object
in Livechat.grantVisitor without mutating store state.

In `@packages/livechat/src/routes/TriggerMessage/index.tsx`:
- Around line 40-51: Update the Screen component’s ref handling to use a
supported Preact forwardRef and attach the ref to its screen DOM element, then
update the TriggerMessage useEffect to measure screenRef.current.children
directly instead of relying on .base. Preserve the existing height calculation
and resizeWidget call.

---

Outside diff comments:
In `@packages/livechat/src/lib/threads.ts`:
- Around line 79-87: Update normalizeMessages to await normalizeMessage for
every message via Promise.all, then filter out null results before returning the
normalized array. Remove the async predicate passed to messages.filter and
preserve the existing default empty-array behavior so loadMessages and
loadMoreMessages receive completed normalization results.

In `@packages/livechat/src/providers/ServerProvider.tsx`:
- Around line 147-161: The contextValue object in the livechat provider does not
satisfy ServerContextValue and relies on an unsafe cast. Either define and use a
narrowed context type containing only the fields livechat consumers require, or
implement the required getStreamAll, writeStream, disconnect, and retryCount
members while preserving the existing getSingleStream API; remove the unknown
cast and ensure the provider’s context type matches the returned value.

---

Nitpick comments:
In `@packages/livechat/.storybook/helpers.tsx`:
- Around line 45-46: Remove the implementation comments at
packages/livechat/.storybook/helpers.tsx lines 45-46 and
packages/livechat/src/routes/Chat/stories.tsx line 18. Leave the surrounding
store decorator and agent-fixture code unchanged.

In `@packages/livechat/.storybook/main.ts`:
- Around line 114-116: Remove the added implementation comment describing
webpack 5, the node:stream scheme, and resolve fallbacks, while leaving the
surrounding configuration and behavior unchanged.

In `@packages/livechat/src/components/App/App.tsx`:
- Around line 62-168: Remove the newly added multi-line explanatory comments
from handleRoute and the lifecycle useLayoutEffect blocks, including the
lifecycle emulation notes and TODO. Keep the implementation behavior unchanged
and retain only comments required by tooling or essential concise context.
- Around line 57-102: The App component receives unused store-derived props,
causing unnecessary re-renders. In packages/livechat/src/components/App/App.tsx
lines 57-102, narrow AppProps to dispatch or the minimal fields actually read;
in packages/livechat/src/components/App/index.tsx lines 15-65, stop
destructuring and forwarding config, gdpr, triggered, user, sound, undocked,
minimized, expanded, alerts, modal, and iframe from StoreConsumer, while
preserving the dispatch prop passed to App.

In `@packages/livechat/src/components/Messages/ImageAttachment/index.tsx`:
- Around line 7-11: Update ImageAttachmentProps to intersect the existing { url;
className? } shape with MessageBubbleProps, matching AudioAttachmentProps;
remove the standalone quoted field and rely on MessageBubbleProps to type quoted
and the other bubble properties.

In `@packages/livechat/src/components/Messages/VideoAttachment/index.tsx`:
- Line 18: Remove the inline jsx-a11y suppression in the VideoAttachment
component and provide actual caption support, or replace it with an
appropriately scoped lint configuration exception. Also delete the stale
commented declaration in packages/livechat/src/lib/uiKit.ts at line 26; both
locations must no longer contain prohibited implementation comments.

In `@packages/livechat/src/lib/main.ts`:
- Around line 8-19: Extract a shared typed normalizer for Livechat.config()
responses and use it in both updateBusinessUnit and loadConfig. Replace their
duplicated as-any casts and overlapping destructuring with the normalizer’s
typed result, returning the appropriate Partial<StoreState['config']> shape
while preserving existing field behavior.

In `@packages/livechat/src/routes/Register/index.tsx`:
- Line 134: Preserve the form-value type contracts in both submit flows: in
packages/livechat/src/routes/Register/index.tsx at lines 134-134, type onSubmit
with SubmitHandler<RegisterFormValues> / RegisterFormValues, remove the broad
SubmitHandler<FieldValues> cast, and obtain customFields through rest
destructuring rather than a nested value; in
packages/livechat/src/routes/LeaveMessage/index.tsx at lines 107-107, pass
onSubmit directly to handleSubmit and use LeaveMessageFormValues instead of
casting to SubmitHandler<FieldValues>.
🪄 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 Plus

Run ID: d03137fa-1d81-43d1-bf6d-e8db18cfa2d8

📥 Commits

Reviewing files that changed from the base of the PR and between 772d8ca and 63efbe3.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (68)
  • packages/livechat/.storybook/helpers.tsx
  • packages/livechat/.storybook/main.ts
  • packages/livechat/package.json
  • packages/livechat/postcss.config.js
  • packages/livechat/src/components/App/App.tsx
  • packages/livechat/src/components/App/index.tsx
  • packages/livechat/src/components/Messages/AudioAttachment/index.tsx
  • packages/livechat/src/components/Messages/FileAttachment/index.tsx
  • packages/livechat/src/components/Messages/ImageAttachment/index.tsx
  • packages/livechat/src/components/Messages/Message/index.tsx
  • packages/livechat/src/components/Messages/MessageList/index.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/index.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/stories.tsx
  • packages/livechat/src/components/Messages/MessageTime/index.tsx
  • packages/livechat/src/components/Messages/MessageTime/stories.tsx
  • packages/livechat/src/components/Messages/TypingIndicator/index.tsx
  • packages/livechat/src/components/Messages/VideoAttachment/index.tsx
  • packages/livechat/src/components/Screen/Header.tsx
  • packages/livechat/src/components/Screen/Screen.tsx
  • packages/livechat/src/components/Screen/ScreenProvider.tsx
  • packages/livechat/src/components/Sound/index.js
  • packages/livechat/src/components/Sound/index.tsx
  • packages/livechat/src/components/uiKit/index.ts
  • packages/livechat/src/components/uiKit/message/Block.tsx
  • packages/livechat/src/definitions/css.d.ts
  • packages/livechat/src/definitions/global.d.ts
  • packages/livechat/src/lib/connection.ts
  • packages/livechat/src/lib/main.ts
  • packages/livechat/src/lib/room.ts
  • packages/livechat/src/lib/threads.ts
  • packages/livechat/src/lib/triggers.ts
  • packages/livechat/src/lib/uiKit.ts
  • packages/livechat/src/providers/ServerProvider.tsx
  • packages/livechat/src/routes/Chat/ChatContent.tsx
  • packages/livechat/src/routes/Chat/ChatFooter.tsx
  • packages/livechat/src/routes/Chat/Picker.tsx
  • packages/livechat/src/routes/Chat/component.js
  • packages/livechat/src/routes/Chat/connector.tsx
  • packages/livechat/src/routes/Chat/container.js
  • packages/livechat/src/routes/Chat/index.ts
  • packages/livechat/src/routes/Chat/index.tsx
  • packages/livechat/src/routes/Chat/stories.tsx
  • packages/livechat/src/routes/Chat/useChatEffects.ts
  • packages/livechat/src/routes/Chat/useChatSubscriptions.ts
  • packages/livechat/src/routes/Chat/useStableCallback.ts
  • packages/livechat/src/routes/ChatFinished/container.tsx
  • packages/livechat/src/routes/ChatFinished/index.ts
  • packages/livechat/src/routes/ChatFinished/index.tsx
  • packages/livechat/src/routes/ChatFinished/stories.tsx
  • packages/livechat/src/routes/GDPRAgreement/component.tsx
  • packages/livechat/src/routes/GDPRAgreement/container.tsx
  • packages/livechat/src/routes/GDPRAgreement/index.ts
  • packages/livechat/src/routes/GDPRAgreement/index.tsx
  • packages/livechat/src/routes/GDPRAgreement/stories.tsx
  • packages/livechat/src/routes/LeaveMessage/index.tsx
  • packages/livechat/src/routes/LeaveMessage/stories.tsx
  • packages/livechat/src/routes/Register/index.tsx
  • packages/livechat/src/routes/Register/stories.tsx
  • packages/livechat/src/routes/SwitchDepartment/index.tsx
  • packages/livechat/src/routes/SwitchDepartment/stories.tsx
  • packages/livechat/src/routes/TriggerMessage/component.tsx
  • packages/livechat/src/routes/TriggerMessage/container.tsx
  • packages/livechat/src/routes/TriggerMessage/index.ts
  • packages/livechat/src/routes/TriggerMessage/index.tsx
  • packages/livechat/src/routes/TriggerMessage/stories.tsx
  • packages/livechat/src/store/index.tsx
  • packages/livechat/tsconfig.typecheck.json
  • packages/livechat/webpack.config.ts
💤 Files with no reviewable changes (17)
  • packages/livechat/src/routes/GDPRAgreement/component.tsx
  • packages/livechat/tsconfig.typecheck.json
  • packages/livechat/src/routes/ChatFinished/container.tsx
  • packages/livechat/src/components/Sound/index.js
  • packages/livechat/src/routes/Chat/index.ts
  • packages/livechat/src/routes/ChatFinished/index.ts
  • packages/livechat/src/routes/TriggerMessage/index.ts
  • packages/livechat/src/routes/GDPRAgreement/container.tsx
  • packages/livechat/postcss.config.js
  • packages/livechat/src/routes/TriggerMessage/container.tsx
  • packages/livechat/src/routes/Chat/component.js
  • packages/livechat/src/lib/connection.ts
  • packages/livechat/src/routes/Chat/connector.tsx
  • packages/livechat/src/routes/Chat/container.js
  • packages/livechat/src/routes/TriggerMessage/component.tsx
  • packages/livechat/src/routes/GDPRAgreement/index.ts
  • packages/livechat/src/definitions/global.d.ts
📜 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 (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/livechat/src/definitions/css.d.ts
  • packages/livechat/src/routes/Chat/useStableCallback.ts
  • packages/livechat/src/components/uiKit/index.ts
  • packages/livechat/src/routes/ChatFinished/index.tsx
  • packages/livechat/src/components/Messages/ImageAttachment/index.tsx
  • packages/livechat/src/components/uiKit/message/Block.tsx
  • packages/livechat/src/components/Screen/Header.tsx
  • packages/livechat/src/routes/GDPRAgreement/index.tsx
  • packages/livechat/src/routes/Chat/useChatSubscriptions.ts
  • packages/livechat/src/components/Messages/MessageTime/stories.tsx
  • packages/livechat/src/components/Messages/VideoAttachment/index.tsx
  • packages/livechat/src/routes/Register/stories.tsx
  • packages/livechat/src/components/Sound/index.tsx
  • packages/livechat/src/routes/TriggerMessage/stories.tsx
  • packages/livechat/src/components/Messages/MessageTime/index.tsx
  • packages/livechat/src/components/Messages/FileAttachment/index.tsx
  • packages/livechat/src/components/Messages/AudioAttachment/index.tsx
  • packages/livechat/src/routes/Chat/useChatEffects.ts
  • packages/livechat/src/components/App/index.tsx
  • packages/livechat/src/routes/LeaveMessage/stories.tsx
  • packages/livechat/src/routes/TriggerMessage/index.tsx
  • packages/livechat/webpack.config.ts
  • packages/livechat/src/routes/Chat/ChatContent.tsx
  • packages/livechat/src/lib/main.ts
  • packages/livechat/src/routes/SwitchDepartment/index.tsx
  • packages/livechat/src/components/Screen/Screen.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/index.tsx
  • packages/livechat/src/routes/SwitchDepartment/stories.tsx
  • packages/livechat/src/routes/LeaveMessage/index.tsx
  • packages/livechat/src/routes/Chat/index.tsx
  • packages/livechat/src/routes/ChatFinished/stories.tsx
  • packages/livechat/src/components/Messages/Message/index.tsx
  • packages/livechat/src/store/index.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/stories.tsx
  • packages/livechat/src/lib/threads.ts
  • packages/livechat/src/lib/uiKit.ts
  • packages/livechat/src/routes/Chat/ChatFooter.tsx
  • packages/livechat/src/routes/Chat/stories.tsx
  • packages/livechat/src/components/App/App.tsx
  • packages/livechat/src/providers/ServerProvider.tsx
  • packages/livechat/src/routes/GDPRAgreement/stories.tsx
  • packages/livechat/src/routes/Register/index.tsx
  • packages/livechat/src/components/Screen/ScreenProvider.tsx
  • packages/livechat/src/components/Messages/MessageList/index.tsx
  • packages/livechat/src/lib/triggers.ts
  • packages/livechat/src/components/Messages/TypingIndicator/index.tsx
  • packages/livechat/src/routes/Chat/Picker.tsx
  • packages/livechat/src/lib/room.ts
🧠 Learnings (7)
📚 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:

  • packages/livechat/src/definitions/css.d.ts
  • packages/livechat/src/routes/Chat/useStableCallback.ts
  • packages/livechat/src/components/uiKit/index.ts
  • packages/livechat/src/routes/Chat/useChatSubscriptions.ts
  • packages/livechat/src/routes/Chat/useChatEffects.ts
  • packages/livechat/webpack.config.ts
  • packages/livechat/src/lib/main.ts
  • packages/livechat/src/lib/threads.ts
  • packages/livechat/src/lib/uiKit.ts
  • packages/livechat/src/lib/triggers.ts
  • packages/livechat/src/lib/room.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:

  • packages/livechat/src/definitions/css.d.ts
  • packages/livechat/src/routes/Chat/useStableCallback.ts
  • packages/livechat/src/components/uiKit/index.ts
  • packages/livechat/src/routes/Chat/useChatSubscriptions.ts
  • packages/livechat/src/routes/Chat/useChatEffects.ts
  • packages/livechat/webpack.config.ts
  • packages/livechat/src/lib/main.ts
  • packages/livechat/src/lib/threads.ts
  • packages/livechat/src/lib/uiKit.ts
  • packages/livechat/src/lib/triggers.ts
  • packages/livechat/src/lib/room.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:

  • packages/livechat/src/definitions/css.d.ts
  • packages/livechat/src/routes/Chat/useStableCallback.ts
  • packages/livechat/src/components/uiKit/index.ts
  • packages/livechat/src/routes/ChatFinished/index.tsx
  • packages/livechat/src/components/Messages/ImageAttachment/index.tsx
  • packages/livechat/src/components/uiKit/message/Block.tsx
  • packages/livechat/src/components/Screen/Header.tsx
  • packages/livechat/src/routes/GDPRAgreement/index.tsx
  • packages/livechat/src/routes/Chat/useChatSubscriptions.ts
  • packages/livechat/src/components/Messages/MessageTime/stories.tsx
  • packages/livechat/src/components/Messages/VideoAttachment/index.tsx
  • packages/livechat/src/routes/Register/stories.tsx
  • packages/livechat/src/components/Sound/index.tsx
  • packages/livechat/src/routes/TriggerMessage/stories.tsx
  • packages/livechat/src/components/Messages/MessageTime/index.tsx
  • packages/livechat/src/components/Messages/FileAttachment/index.tsx
  • packages/livechat/src/components/Messages/AudioAttachment/index.tsx
  • packages/livechat/src/routes/Chat/useChatEffects.ts
  • packages/livechat/src/components/App/index.tsx
  • packages/livechat/src/routes/LeaveMessage/stories.tsx
  • packages/livechat/src/routes/TriggerMessage/index.tsx
  • packages/livechat/webpack.config.ts
  • packages/livechat/src/routes/Chat/ChatContent.tsx
  • packages/livechat/src/lib/main.ts
  • packages/livechat/src/routes/SwitchDepartment/index.tsx
  • packages/livechat/src/components/Screen/Screen.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/index.tsx
  • packages/livechat/src/routes/SwitchDepartment/stories.tsx
  • packages/livechat/src/routes/LeaveMessage/index.tsx
  • packages/livechat/src/routes/Chat/index.tsx
  • packages/livechat/src/routes/ChatFinished/stories.tsx
  • packages/livechat/src/components/Messages/Message/index.tsx
  • packages/livechat/src/store/index.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/stories.tsx
  • packages/livechat/src/lib/threads.ts
  • packages/livechat/src/lib/uiKit.ts
  • packages/livechat/src/routes/Chat/ChatFooter.tsx
  • packages/livechat/src/routes/Chat/stories.tsx
  • packages/livechat/src/components/App/App.tsx
  • packages/livechat/src/providers/ServerProvider.tsx
  • packages/livechat/src/routes/GDPRAgreement/stories.tsx
  • packages/livechat/src/routes/Register/index.tsx
  • packages/livechat/src/components/Screen/ScreenProvider.tsx
  • packages/livechat/src/components/Messages/MessageList/index.tsx
  • packages/livechat/src/lib/triggers.ts
  • packages/livechat/src/components/Messages/TypingIndicator/index.tsx
  • packages/livechat/src/routes/Chat/Picker.tsx
  • packages/livechat/src/lib/room.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:

  • packages/livechat/src/routes/ChatFinished/index.tsx
  • packages/livechat/src/components/Messages/ImageAttachment/index.tsx
  • packages/livechat/src/components/uiKit/message/Block.tsx
  • packages/livechat/src/components/Screen/Header.tsx
  • packages/livechat/src/routes/GDPRAgreement/index.tsx
  • packages/livechat/src/components/Messages/MessageTime/stories.tsx
  • packages/livechat/src/components/Messages/VideoAttachment/index.tsx
  • packages/livechat/src/routes/Register/stories.tsx
  • packages/livechat/src/components/Sound/index.tsx
  • packages/livechat/src/routes/TriggerMessage/stories.tsx
  • packages/livechat/src/components/Messages/MessageTime/index.tsx
  • packages/livechat/src/components/Messages/FileAttachment/index.tsx
  • packages/livechat/src/components/Messages/AudioAttachment/index.tsx
  • packages/livechat/src/components/App/index.tsx
  • packages/livechat/src/routes/LeaveMessage/stories.tsx
  • packages/livechat/src/routes/TriggerMessage/index.tsx
  • packages/livechat/src/routes/Chat/ChatContent.tsx
  • packages/livechat/src/routes/SwitchDepartment/index.tsx
  • packages/livechat/src/components/Screen/Screen.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/index.tsx
  • packages/livechat/src/routes/SwitchDepartment/stories.tsx
  • packages/livechat/src/routes/LeaveMessage/index.tsx
  • packages/livechat/src/routes/Chat/index.tsx
  • packages/livechat/src/routes/ChatFinished/stories.tsx
  • packages/livechat/src/components/Messages/Message/index.tsx
  • packages/livechat/src/store/index.tsx
  • packages/livechat/src/components/Messages/MessageSeparator/stories.tsx
  • packages/livechat/src/routes/Chat/ChatFooter.tsx
  • packages/livechat/src/routes/Chat/stories.tsx
  • packages/livechat/src/components/App/App.tsx
  • packages/livechat/src/providers/ServerProvider.tsx
  • packages/livechat/src/routes/GDPRAgreement/stories.tsx
  • packages/livechat/src/routes/Register/index.tsx
  • packages/livechat/src/components/Screen/ScreenProvider.tsx
  • packages/livechat/src/components/Messages/MessageList/index.tsx
  • packages/livechat/src/components/Messages/TypingIndicator/index.tsx
  • packages/livechat/src/routes/Chat/Picker.tsx
📚 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/livechat/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/livechat/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/livechat/package.json
🔇 Additional comments (50)
packages/livechat/src/routes/ChatFinished/index.tsx (1)

1-31: LGTM!

Also applies to: 44-44

packages/livechat/src/routes/GDPRAgreement/index.tsx (1)

1-53: LGTM!

packages/livechat/src/routes/LeaveMessage/index.tsx (1)

23-31: LGTM!

Also applies to: 56-60, 72-72

packages/livechat/src/routes/Register/index.tsx (1)

27-27: LGTM!

Also applies to: 100-100

packages/livechat/src/routes/SwitchDepartment/index.tsx (1)

15-20: LGTM!

Also applies to: 38-38, 65-65, 82-82, 105-107

packages/livechat/src/routes/TriggerMessage/index.tsx (1)

1-36: LGTM!

Also applies to: 53-75

packages/livechat/.storybook/helpers.tsx (2)

8-43: LGTM!


47-59: 🩺 Stability & Availability

Check whether there is no .storybook/helpers.tsx here.

The repo does not contain packages/livechat/.storybook/helpers.tsx, but packages/livechat/src/store/index.tsx uses a functional StoreProvider; if this decorator is meant to be part of that runtime store context provider, the concern may need adjustment.

packages/livechat/src/routes/Chat/stories.tsx (1)

3-17: LGTM!

Also applies to: 19-87

packages/livechat/src/routes/ChatFinished/stories.tsx (1)

3-32: LGTM!

packages/livechat/src/routes/GDPRAgreement/stories.tsx (1)

3-15: LGTM!

packages/livechat/src/routes/TriggerMessage/stories.tsx (1)

3-45: LGTM!

packages/livechat/src/routes/SwitchDepartment/stories.tsx (1)

3-32: LGTM!

packages/livechat/.storybook/main.ts (1)

4-4: LGTM!

Also applies to: 16-36, 117-121

packages/livechat/package.json (1)

24-24: LGTM!

Also applies to: 71-71

packages/livechat/src/components/Messages/MessageSeparator/stories.tsx (1)

3-25: LGTM!

packages/livechat/src/components/Messages/MessageTime/stories.tsx (1)

3-3: LGTM!

Also applies to: 15-28

packages/livechat/src/routes/LeaveMessage/stories.tsx (1)

3-3: LGTM!

Also applies to: 14-16

packages/livechat/src/routes/Register/stories.tsx (1)

3-3: LGTM!

Also applies to: 14-16

packages/livechat/webpack.config.ts (1)

86-106: LGTM!

packages/livechat/src/components/Messages/AudioAttachment/index.tsx (1)

2-27: LGTM!

packages/livechat/src/components/Messages/Message/index.tsx (1)

1-4: LGTM!

Also applies to: 31-41, 69-78, 80-94, 96-107, 108-138, 141-145

packages/livechat/src/components/Messages/MessageList/index.tsx (2)

3-18: LGTM!

Also applies to: 37-118, 178-224, 226-238


19-35: 🩺 Stability & Availability

No change needed. The livechat store initializes messages to [], and the state type declares it as any[], so this render path does not introduce an undefined messages access.

packages/livechat/src/components/Messages/MessageSeparator/index.tsx (1)

1-52: LGTM!

packages/livechat/src/components/Messages/MessageTime/index.tsx (1)

5-5: LGTM!

Also applies to: 27-42, 44-44

packages/livechat/src/components/Messages/TypingIndicator/index.tsx (1)

9-14: LGTM!

packages/livechat/src/components/Messages/FileAttachment/index.tsx (1)

9-23: LGTM!

packages/livechat/src/components/Messages/VideoAttachment/index.tsx (1)

2-17: LGTM!

Also applies to: 19-26

packages/livechat/src/lib/threads.ts (1)

2-76: LGTM!

packages/livechat/src/lib/triggers.ts (1)

1-1: LGTM!

Also applies to: 11-26, 40-46, 76-126, 143-154, 168-207

packages/livechat/src/lib/uiKit.ts (1)

24-25: LGTM!

Also applies to: 28-41, 99-146

packages/livechat/src/components/uiKit/index.ts (1)

1-1: LGTM!

packages/livechat/src/providers/ServerProvider.tsx (1)

94-104: 🎯 Functional Correctness

Verify the removed callback cast still type-checks.

The previous callback as (...args: any[]) => void cast was dropped from the sdk.stream(...) call. If sdk.stream's callback parameter type isn't structurally compatible with (...args: StreamerCallbackArgs<N, K>) => void (e.g. expects (...args: any[]) => void invariantly), this could reintroduce a type error now that the package tsconfig is used for typechecking.

packages/livechat/src/components/Screen/ScreenProvider.tsx (1)

3-11: LGTM!

Also applies to: 77-97, 172-201

packages/livechat/src/components/Screen/Screen.tsx (1)

27-45: LGTM!

packages/livechat/src/components/Sound/index.tsx (1)

1-47: LGTM!

packages/livechat/src/components/Screen/Header.tsx (1)

3-3: LGTM!

Also applies to: 142-142

packages/livechat/src/components/uiKit/message/Block.tsx (1)

6-9: LGTM!

packages/livechat/src/definitions/css.d.ts (1)

1-3: LGTM!

packages/livechat/src/routes/Chat/Picker.tsx (1)

1-10: LGTM!

packages/livechat/src/store/index.tsx (1)

58-59: LGTM!

Also applies to: 112-129

packages/livechat/src/lib/room.ts (2)

46-233: LGTM!

Also applies to: 303-312


246-249: 🎯 Functional Correctness

No change needed. GETLivechatMessagesHistoryRidParams extends a paginated request with only optional request fields, and LivechatClientImpl.loadMessages always merges token from the client; {} therefore preserves the initial-load query behavior.

packages/livechat/src/routes/Chat/index.tsx (1)

92-107: LGTM!

packages/livechat/src/routes/Chat/ChatContent.tsx (1)

25-98: LGTM!

packages/livechat/src/routes/Chat/ChatFooter.tsx (1)

91-134: LGTM!

Also applies to: 136-157

packages/livechat/src/routes/Chat/useChatEffects.ts (1)

13-158: LGTM!

packages/livechat/src/routes/Chat/useChatSubscriptions.ts (1)

11-21: LGTM!

packages/livechat/src/routes/Chat/useStableCallback.ts (1)

3-9: LGTM!

Comment on lines +104 to +160
// Emulates componentDidMount / componentWillUnmount. useLayoutEffect (not
// useEffect) keeps the init/teardown synchronous with the commit, matching
// the class lifecycle these behaviors were ported from.
useLayoutEffect(() => {
const handleVisibilityChange = async () => {
dispatch({ visible: !visibility.hidden });
};

i18next.on('languageChanged', this.handleLanguageChange);
}
// Reads from the store at call time (after Connection.init has loaded the
// config), not from the mount-render closure whose config is still the
// initial empty state — otherwise Triggers.init() never runs.
const handleTriggers = () => {
const { config } = store.state;
if (config.online && config.enabled) {
Triggers.init();
}

protected async initialize() {
// TODO: split these behaviors into composable components
await Connection.init();
CustomFields.init();
userPresence.init();
Hooks.init();
this.handleTriggers();
this.initWidget();
this.setState({ initialized: true });
parentCall('ready');
}
void Triggers.processTriggers();
};

protected async finalize() {
CustomFields.reset();
userPresence.reset();
visibility.removeListener(this.handleVisibilityChange);
}
const initWidget = () => {
const { config, minimized, iframe, undocked } = store.state;
if (!undocked) {
parentCall(minimized ? 'minimizeWindow' : 'restoreWindow');
parentCall(iframe.visible ? 'showWidget' : 'hideWidget');
parentCall('setWidgetPosition', config.theme.position || 'right');
}

override componentDidMount() {
void this.initialize();
}
visibility.addListener(handleVisibilityChange);

override componentWillUnmount() {
void this.finalize();
}
void handleVisibilityChange();

override componentDidUpdate() {
const { i18n } = this.props;
window.addEventListener('beforeunload', () => {
visibility.removeListener(handleVisibilityChange);
dispatch({ minimized: true, undocked: false });
});
};

if (i18n.t) {
document.dir = isRTL(i18n.t('yes')) ? 'rtl' : 'ltr';
void (async () => {
// TODO: split these behaviors into composable components
await Connection.init();
CustomFields.init();
userPresence.init();
Hooks.init();
handleTriggers();
initWidget();
setInitialized(true);
parentCall('ready');
})();

return () => {
CustomFields.reset();
userPresence.reset();
visibility.removeListener(handleVisibilityChange);
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Mount effect cleanup doesn't cover work started asynchronously after unmount.

The async IIFE inside the mount useLayoutEffect calls initWidget() (which adds a visibility listener and a beforeunload listener) and setInitialized(true) after await Connection.init() resolves. If the component unmounts before that resolves, the cleanup (which already ran) can't remove listeners added afterward, and setInitialized fires on an unmounted component. Separately, the beforeunload listener registered inside initWidget() uses an inline anonymous function, so it's never removed even in the normal unmount path.

🐛 Proposed guard + listener cleanup
 	useLayoutEffect(() => {
+		let mounted = true;
+
 		const handleVisibilityChange = async () => {
 			dispatch({ visible: !visibility.hidden });
 		};
 
+		const handleBeforeUnload = () => {
+			visibility.removeListener(handleVisibilityChange);
+			dispatch({ minimized: true, undocked: false });
+		};
+
 		const handleTriggers = () => {
 			...
 		};
 
 		const initWidget = () => {
 			...
-			window.addEventListener('beforeunload', () => {
-				visibility.removeListener(handleVisibilityChange);
-				dispatch({ minimized: true, undocked: false });
-			});
+			window.addEventListener('beforeunload', handleBeforeUnload);
 		};
 
 		void (async () => {
 			await Connection.init();
 			CustomFields.init();
 			userPresence.init();
 			Hooks.init();
 			handleTriggers();
+			if (!mounted) {
+				return;
+			}
 			initWidget();
 			setInitialized(true);
 			parentCall('ready');
 		})();
 
 		return () => {
+			mounted = false;
 			CustomFields.reset();
 			userPresence.reset();
 			visibility.removeListener(handleVisibilityChange);
+			window.removeEventListener('beforeunload', handleBeforeUnload);
 		};
 	}, []);
🤖 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 `@packages/livechat/src/components/App/App.tsx` around lines 104 - 160, Guard
the asynchronous initialization in the mount useLayoutEffect with an
active/mounted flag, skipping initWidget, setInitialized, and
parentCall('ready') when Connection.init() resolves after cleanup. Store the
beforeunload callback in a named function or variable within initWidget, remove
it during effect cleanup, and ensure listeners are removed whether
initialization completes before or after unmount.

Comment on lines +12 to 13
const serverURL =
window.SERVER_URL ?? parse(window.location.search).serverUrl ?? (process.env.NODE_ENV === 'development' ? 'http://localhost:3000' : null);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate the Window.SERVER_URL type declaration and any narrowing/casts near serverURL.
rg -n 'SERVER_URL' packages/livechat/src -C3
rg -n 'serverURL' packages/livechat/src/components/App/index.tsx -C3

Repository: RocketChat/Rocket.Chat

Length of output: 2469


🏁 Script executed:

#!/bin/bash
set -e

echo "== package metadata =="
for f in package.json packages/livechat/package.json; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    sed -n '1,220p' "$f"
  fi
done

echo
echo "== relevant declarations/files =="
for f in packages/livechat/src/definitions/global.d.ts packages/livechat/src/components/App/index.tsx packages/livechat/src/components/App.tsx packages/livechat/src/providers/ServerProvider.tsx; do
  if [ -f "$f" ]; then
    echo "--- $f ($(wc -l < "$f") lines) ---"
    sed -n '1,180p' "$f"
  fi
done

echo
echo "== tsconfig files =="
find . -path '*/tsconfig*.json' -print | sed -n '1,40p'
for f in package.json packages/livechat/tsconfig.json tsconfig.json; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    sed -n '1,180p' "$f"
  fi
done

Repository: RocketChat/Rocket.Chat

Length of output: 30492


🏁 Script executed:

#!/bin/bash
set -e

echo "== package lock/query-string version hints =="
for f in yarn.lock .yarn/lock.lock package-lock.json pnpm-lock.yaml; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    rg -n "query-string|`@types/query-string`" "$f" -C2 | head -80
  fi
done

echo
echo "== installed declarations if present =="
for f in node_modules/query-string/index.d.ts node_modules/query-string/dts/index.d.ts node_modules/@types/query-string/index.d.ts; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    sed -n '1,180p' "$f"
  fi
done

echo
echo "== SDKProvider relevant declarations =="
fd -a 'SDKProvider' packages/livechat/src
for f in packages/livechat/src/providers/SDKProvider.tsx packages/livechat/src/providers/sdk-provider.tsx; do
  if [ -f "$f" ]; then
    echo "--- $f ($(wc -l < "$f") lines) ---"
    sed -n '1,180p' "$f"
  fi
done

echo
echo "== tsconfig inheritance/source paths =="
for f in packages/livechat/node_modules/@rocket.chat/tsconfig/client.json .yarn/cache/@rocket.chat-tsconfig*.zip; do
  if [ -f "$f" ]; then
    echo "--- $f ---"
    if [ "${f##*.}" = "zip" ]; then unzip -Z1 "$f" | head -40; else sed -n '1,120p' "$f"; fi
  fi
done

echo
echo "== deterministic type-shape probe =="
node - <<'JS'
const fs = require('fs');
const files = [
  'packages/livechat/src/definitions/global.d.ts',
  'packages/livechat/src/providers/SDKProvider.tsx',
  'packages/livechat/src/providers/ServerProvider.tsx',
];
for (const file of files) {
  const text = fs.existsSync(file) ? fs.readFileSync(file, 'utf8') : '';
  for (const needle of [
    'interface Window', 'SERVER_URL: string',
    'export type SDKProviderProps', 'serverURL: string',
    'export type ServerProviderProps',
  ]) {
    if (text.includes(needle)) console.log(`${file}: contains ${JSON.stringify(needle)}`);
  }
}
JS

Repository: RocketChat/Rocket.Chat

Length of output: 2539


🌐 Web query:

query-string v7.1.3 TypeScript types parse return type

💡 Result:

In query-string v7.1.3, the parse function returns a ParsedQuery object [1][2]. The ParsedQuery type is defined in the package's TypeScript declarations as follows [1][3]: export type ParsedQuery = Record<string, string | null | Array<string | null>>; Key characteristics of the return value include: - It is an object created with Object.create(null), meaning it does not have a prototype [4][5]. - The keys are the query parameters, and the values can be a string, null (if no value is assigned), or an array of these types [1][3]. - When specific parsing options like parseNumbers or parseBooleans are enabled, the return type remains ParsedQuery, though the values within the object may be typed differently depending on the specific overload signature used in the TypeScript definitions [1][3]. In TypeScript, the library provides various overloads to attempt to reflect these runtime transformations [1][3].

Citations:


🌐 Web query:

query-string v7.1.3 parse null is undefined defaultOptions parseNumbers parseBoolean

💡 Result:

In query-string v7.1.3, the behavior for parsing values is as follows: Regarding parseNumbers and parseBooleans: These are options passed to the.parse method [1][2]. When set to true, they enable the conversion of query string values into their respective types (number or boolean) rather than keeping them as strings [2]. By default, both options are set to false [2]. Regarding null and undefined: Query-string does not have a parseNull option [1][3]. During the parsing process, values in the query string are generally treated as strings (e.g., 'foo=null' results in {foo: 'null'}) unless specific type parsing is applied [1][2]. Conversely, when stringifying objects using.stringify, keys with undefined values are always skipped, while keys with null values are skipped only if the skipNull option is set to true [4][5]. Regarding defaultOptions: The query-string package does not support a global defaultOptions configuration object to set default values for parse methods [1][3]. Options must be passed explicitly as the second argument to functions like.parse(string, options) [1][3].

Citations:


Narrow the query-param fallback before passing serverURL to providers.

parse(window.location.search).serverUrl is a query-string value that can still be null | string[], while SDKProvider and ServerProvider both require serverURL: string. Clamp this fallback to a single string before passing it to those providers.

🤖 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 `@packages/livechat/src/components/App/index.tsx` around lines 12 - 13, Update
the serverURL initialization in App so the
parse(window.location.search).serverUrl fallback is narrowed to a single string,
excluding null and string[] values before it reaches SDKProvider and
ServerProvider. Preserve the existing window.SERVER_URL precedence and
development fallback behavior.

Comment on lines +139 to +140
{/* NOTE: the original passed `inverse={me}`, which MessageTime ignores (its prop is `inverted`); dropped to keep behavior identical. */}
{!compact && !message.type && <MessageTime normal={!me} ts={message.ts} />}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove explanatory implementation comment.

The NOTE comment explaining the dropped inverse prop is implementation-detail prose rather than a functional directive (e.g., an eslint-disable). As per coding guidelines, **/*.{ts,tsx,js} should avoid code comments in the implementation.

🧹 Suggested fix
-			{/* NOTE: the original passed `inverse={me}`, which MessageTime ignores (its prop is `inverted`); dropped to keep behavior identical. */}
 			{!compact && !message.type && <MessageTime normal={!me} ts={message.ts} />}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{/* NOTE: the original passed `inverse={me}`, which MessageTime ignores (its prop is `inverted`); dropped to keep behavior identical. */}
{!compact && !message.type && <MessageTime normal={!me} ts={message.ts} />}
{!compact && !message.type && <MessageTime normal={!me} ts={message.ts} />}
🤖 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 `@packages/livechat/src/components/Messages/Message/index.tsx` around lines 139
- 140, Remove the explanatory NOTE comment above the MessageTime render in the
message component, leaving the existing conditional rendering and props
unchanged.

Source: Coding guidelines

onDisableNotifications: () => unknown;
onMinimize: () => unknown;
onRestore: () => Promise<void>;
onRestore: () => void;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

onRestore was changed from async to fire-and-forget, breaking a downstream await onRestore() caller.

ScreenContextValue.onRestore is now typed () => void and handleRestore fires the loadConfig/loadMessages/dispatch work inside a detached void (async () => {...})() IIFE, so callers no longer get a promise that resolves once that work completes. TriggerMessage's handleStartChatClick still does await onRestore(); route('/'); expecting the previous awaited behavior — with this change, route('/') now fires before loadConfig()/loadMessages() complete when restoring from an undocked state, so the Chat route can render with stale/missing config or messages.

🐛 Proposed fix: keep `handleRestore` async so callers can await completion
-	onRestore: () => void;
+	onRestore: () => Promise<void>;
...
-	const handleRestore = () => {
-		parentCall('restoreWindow');
-
-		void (async () => {
-			if (undocked) {
-				// Cross-tab communication will not work here due cross origin (usually the widget parent and the RC server will have different urls)
-				// So we manually update the widget to get the messages and actions done while undocked
-				await loadConfig();
-				await loadMessages();
-			}
-
-			dispatch({ minimized: false, undocked: false });
-
-			Triggers.callbacks?.emit('chat-opened-by-visitor');
-		})();
-	};
+	const handleRestore = async () => {
+		parentCall('restoreWindow');
+
+		if (undocked) {
+			// Cross-tab communication will not work here due cross origin (usually the widget parent and the RC server will have different urls)
+			// So we manually update the widget to get the messages and actions done while undocked
+			await loadConfig();
+			await loadMessages();
+		}
+
+		dispatch({ minimized: false, undocked: false });
+
+		Triggers.callbacks?.emit('chat-opened-by-visitor');
+	};

Also applies to: 135-150

🤖 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 `@packages/livechat/src/components/Screen/ScreenProvider.tsx` at line 45,
Restore the promise-based contract for ScreenContextValue.onRestore and
handleRestore: type onRestore as an async function returning a promise, make
handleRestore await the loadConfig/loadMessages/dispatch work directly, and
remove the detached async IIFE. Preserve TriggerMessage.handleStartChatClick’s
await behavior so route('/') runs only after restoration completes.

Comment on lines +21 to +36
export const closeChat = async ({ transcriptRequested }: { transcriptRequested?: boolean } = {}) => {
if (!transcriptRequested) {
await handleTranscript();
}

const { department, config: { settings: { clearLocalStorageWhenChatEnded } = {} } = {} } = store.state;
const { department, config: { settings: { clearLocalStorageWhenChatEnded } = {} } = {} } = store.state as StoreState & {
department?: string;
};

await store.setState({ room: null, renderedTriggers: [] });

if (clearLocalStorageWhenChatEnded) {
// exclude UI-affecting flags
const { iframe: currentIframe } = store.state;
const { minimized, visible, undocked, expanded, businessUnit, config, iframe, ...initial } = initialState();
initial.iframe = { ...currentIframe, guest: { department } };
await store.setState(initial);
await store.setState({ ...initial, iframe: { ...currentIframe, guest: { department } } });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for any code path that sets a top-level `department` on store.state
rg -n "setState\(\{[^}]*\bdepartment\b" packages/livechat/src -g '!*.test.*'
rg -n "iframe:.*department|guest:.*department" packages/livechat/src/lib/room.ts packages/livechat/src/lib/main.ts

Repository: RocketChat/Rocket.Chat

Length of output: 709


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== room.ts outline =="
ast-grep outline packages/livechat/src/lib/room.ts --view compact || true

echo "== room.ts relevant section =="
cat -n packages/livechat/src/lib/room.ts | sed -n '1,80p'

echo "== store state definitions =="
fd -a 'store|types|store' packages/livechat/src | sed 's#^\./##'
rg -n "type StoreState|interface StoreState|StoreState|initialState|defaultAgent|iframe:" packages/livechat/src packages/livechat/src/lib -g '!**/*.test.*' | head -200

Repository: RocketChat/Rocket.Chat

Length of output: 18933


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== package livechat store/index.tsx =="
cat -n packages/livechat/src/store/index.tsx | sed -n '1,240p'

echo "== package livechat stores occurrences of department outside tests =="
rg -n "\bdepartment\b" packages/livechat/src/store packages/livechat/src/lib packages/livechat/src/hooks.ts packages/livechat/src -g '!**/*.test.*' -g '!**/stories.*'

echo "== package livechat setState calls that include department =="
rg -n "setState\([^)]*\bdepartment\b|department:\s*" packages/livechat/src -g '!**/*.test.*' -g '!**/stories.*'

Repository: RocketChat/Rocket.Chat

Length of output: 17937


Read department from iframe.guest.department in closeChat.

StoreState does not define a top-level department, and department is stored on iframe.guest, so this read is typed as any and will be undefined at runtime. The local-storage-clear flow then clears the previously selected department instead of preserving it:

const { iframe: { guest: { department } = {} } = {} } = store.state;
🤖 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 `@packages/livechat/src/lib/room.ts` around lines 21 - 36, Update closeChat to
read department from store.state.iframe.guest.department, using the nested
defaults shown in the review, instead of destructuring a top-level department
from StoreState. Preserve this value when setting the cleared state’s
iframe.guest.

Comment on lines +62 to +74
const grantUser = useStableCallback(async () => {
if (user) {
return;
}

if (!guest?.department && defaultDepartment && guest) {
guest.department = defaultDepartment;
}

const visitor = { token, ...guest };
const { visitor: newUser } = await Livechat.grantVisitor({ visitor });
dispatch({ user: newUser });
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Avoid mutating guest from store state in place.

guest.department = defaultDepartment; mutates the object referenced by iframe.guest in the store directly. Prefer building a new object so the store's data isn't mutated outside of dispatch.

🛠️ Proposed fix
-		if (!guest?.department && defaultDepartment && guest) {
-			guest.department = defaultDepartment;
-		}
-
-		const visitor = { token, ...guest };
+		const visitor = {
+			token,
+			...guest,
+			...(!guest?.department && defaultDepartment && guest ? { department: defaultDepartment } : {}),
+		};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const grantUser = useStableCallback(async () => {
if (user) {
return;
}
if (!guest?.department && defaultDepartment && guest) {
guest.department = defaultDepartment;
}
const visitor = { token, ...guest };
const { visitor: newUser } = await Livechat.grantVisitor({ visitor });
dispatch({ user: newUser });
});
const grantUser = useStableCallback(async () => {
if (user) {
return;
}
const visitor = {
token,
...guest,
...(!guest?.department && defaultDepartment && guest ? { department: defaultDepartment } : {}),
};
const { visitor: newUser } = await Livechat.grantVisitor({ visitor });
dispatch({ user: newUser });
});
🤖 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 `@packages/livechat/src/routes/Chat/index.tsx` around lines 62 - 74, Update
grantUser to avoid assigning defaultDepartment directly to guest. Build a new
visitor/guest object with the default department when needed, preserving the
existing guest data and using that immutable object in Livechat.grantVisitor
without mutating store state.

Comment on lines +115 to +135
const doFileUpload = useStableCallback(async (rid: string, file: File) => {
try {
await Livechat.uploadFile(rid, file);
} catch (error: any) {
const {
data: { reason, sizeAllowed },
} = error;

let message = t('fileupload_error');
switch (reason) {
case 'error-type-not-allowed':
message = t('media_types_not_accepted');
break;
case 'error-size-not-allowed':
message = t('file_exceeds_allowed_size_of_size', { size: sizeAllowed });
}

const alert = { id: createToken(), children: message, error: true, timeout: 5000 };
dispatch({ alerts: (alerts.push(alert), alerts) });
}
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

doFileUpload will throw if the caught error has no .data.

const { data: { reason, sizeAllowed } } = error; assumes error.data always exists. If Livechat.uploadFile rejects with a plain Error (network failure, timeout), this destructure throws, and since doFileUpload is invoked fire-and-forget (void doFileUpload(...)), the real upload failure is replaced by an unhandled rejection and the user never sees an upload-failed alert.

🛠️ Proposed fix
-			const {
-				data: { reason, sizeAllowed },
-			} = error;
+			const { reason, sizeAllowed } = error?.data ?? {};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const doFileUpload = useStableCallback(async (rid: string, file: File) => {
try {
await Livechat.uploadFile(rid, file);
} catch (error: any) {
const {
data: { reason, sizeAllowed },
} = error;
let message = t('fileupload_error');
switch (reason) {
case 'error-type-not-allowed':
message = t('media_types_not_accepted');
break;
case 'error-size-not-allowed':
message = t('file_exceeds_allowed_size_of_size', { size: sizeAllowed });
}
const alert = { id: createToken(), children: message, error: true, timeout: 5000 };
dispatch({ alerts: (alerts.push(alert), alerts) });
}
});
const doFileUpload = useStableCallback(async (rid: string, file: File) => {
try {
await Livechat.uploadFile(rid, file);
} catch (error: any) {
const { reason, sizeAllowed } = error?.data ?? {};
let message = t('fileupload_error');
switch (reason) {
case 'error-type-not-allowed':
message = t('media_types_not_accepted');
break;
case 'error-size-not-allowed':
message = t('file_exceeds_allowed_size_of_size', { size: sizeAllowed });
}
const alert = { id: createToken(), children: message, error: true, timeout: 5000 };
dispatch({ alerts: (alerts.push(alert), alerts) });
}
});
🤖 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 `@packages/livechat/src/routes/Chat/index.tsx` around lines 115 - 135, Update
the error handling in doFileUpload to safely handle rejected uploads whose error
lacks a data object, defaulting reason and sizeAllowed when necessary. Preserve
the existing specialized messages for known server reasons and ensure all other
failures display the generic fileupload_error alert without throwing from the
catch block.

Comment on lines +40 to +51
useEffect(() => {
let height = 0;

for (const el of screenRef.current.base.children) {
height += el.scrollHeight;
}

parentCall('resizeWidget', height);
});

return (
<Screen title={t('messages')} triggered ref={screenRef}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

rg -n -C 4 'ref\?:|ref=' packages/livechat/src/components/Screen/Screen.tsx
rg -n -C 4 'screenRef\.current|<Screen.*ref=' packages/livechat/src/routes/TriggerMessage/index.tsx

Repository: RocketChat/Rocket.Chat

Length of output: 934


🏁 Script executed:

#!/bin/bash
set -eu

echo '--- Screen outline ---'
ast-grep outline packages/livechat/src/components/Screen/Screen.tsx || true

echo '--- Screen relevant content ---'
sed -n '1,220p' packages/livechat/src/components/Screen/Screen.tsx

echo '--- TriggerMessage relevant content ---'
sed -n '1,120p' packages/livechat/src/routes/TriggerMessage/index.tsx

echo '--- references to Screen ref usage ---'
rg -n 'Screen \{.*ref|<Screen .*ref=|\.base|screenRef' packages/livechat/src

Repository: RocketChat/Rocket.Chat

Length of output: 6421


Use a supported DOM ref for widget measurement.

Screen accepts ref as a prop but never forwards or attaches it, so this route can pass the ref into a component without exposing .base or any DOM node. Expose the screen DOM through a supported Preact forwardRef ref, then measure screenRef.current.children.

🤖 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 `@packages/livechat/src/routes/TriggerMessage/index.tsx` around lines 40 - 51,
Update the Screen component’s ref handling to use a supported Preact forwardRef
and attach the ref to its screen DOM element, then update the TriggerMessage
useEffect to measure screenRef.current.children directly instead of relying on
.base. Preserve the existing height calculation and resizeWidget call.

@tassoevan

Copy link
Copy Markdown
Member Author

/jira ARCH-2200

@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Jul 31, 2026
@dionisio-bot dionisio-bot Bot added the stat: ready to merge PR tested and approved waiting for merge label Jul 31, 2026
@ggazzo
ggazzo merged commit 3ee9475 into develop Jul 31, 2026
99 of 101 checks passed
@ggazzo
ggazzo deleted the refactor/livechat-js branch July 31, 2026 15:05
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 type: chore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants