VIDCS-3681: Bump up React version #154

behei-vonage · 2025-04-28T16:34:45Z

What is this PR doing?

This PR bumps up version of react used in the project from 18 to 19.

How should this be manually tested?

A few things need to be tested as a part of this PR.
Given that there is a version update of the framework that we use, it'd be great to check that the basic functionality works as expected - multiple people in the meeting room, participant list/chat/report issue functionality works, changing audio/video sources works etc.
CI passes

What are the relevant tickets?

A maintainer will add this ticket number.

Resolves VIDCS-3681

Checklist

[ ] Branch is based on develop (not main).
[ ] Resolves a Known Issue.
[ ] If yes, did you remove the item from the docs/KNOWN_ISSUES.md?
[ ] Resolves an item reported in Issues.
If yes, which issue? Issue Number?

behei-vonage · 2025-04-28T16:35:19Z

frontend/src/Context/PublisherProvider/usePublisherOptions/usePublisherOptions.tsx

@@ -17,7 +17,7 @@ import DeviceStore from '../../../utils/DeviceStore';
 const usePublisherOptions = (): PublisherProperties | null => {
  const { user } = useUserContext();
  const [publisherOptions, setPublisherOptions] = useState<PublisherProperties | null>(null);
-  const deviceStoreRef = useRef<DeviceStore>();
+  const deviceStoreRef = useRef<DeviceStore | null>(null);


Note: this was changed in a few places. it is enforced in React 19

behei-vonage · 2025-04-28T16:36:37Z

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx

-      expect((outputDevicesElement.firstChild as HTMLOptionElement).selected).toBe(true);
+      expect((outputDevicesElement.firstChild as Element).classList.contains('Mui-selected')).toBe(
+        true
+      );


Note: this was returning undefined after an update and needed to be changed on how we access the selected property.

Can we be more specific than Element? Should this be a div, an li, span, or is it unknown so an HTMLElement?

behei-vonage · 2025-04-28T16:37:27Z

frontend/src/pages/GoodBye/GoodBye.spec.tsx

@@ -22,16 +22,23 @@ describe('GoodBye', () => {
  });

  it('should render', () => {
-    render(<GoodBye />, { wrapper: BrowserRouter });


Note: wrapper: BrowserRouter is no longer supported. the change is fairly simple though, just had to directly wrap the Goodbye component with BrowserRouter

behei-vonage · 2025-04-28T16:38:25Z

frontend/vite.config.ts

+    react({
+      babel: {
+        plugins: isTest
+          ? [] // Disable babel-plugin-react-compiler in test environment
+          : [['babel-plugin-react-compiler', { target: '19' }]],
+      },
+    }),


Note: this was added because the React Compiler was breaking the tests by injecting things like UseMemoCache that are undefined in the vitest environment.
target set to 19 refers to the React version.

is this a recommended workaround? Are there more people dealing with this?

Copilot suggested this workaround. there isn't a whole lot of people dealing with this quite yet since it's a fairly new plugin

behei-vonage · 2025-04-28T16:39:22Z

frontend/package.json

    "@types/react-router-dom": "^5.3.3",
    "@types/ua-parser-js": "^0.7.39",
    "@types/uuid": "^9.0.8",
    "@vitest/coverage-v8": "^1.3.0",
    "@vitest/ui": "^1.3",
+    "babel-plugin-react-compiler": "19.1.0-rc.1",


Note: please do not be scared that the name of the React Compiler starts with babel-plugin. While it works in babel, it is also fully supported with vite - as you can see, no babel.config files have been introduced and it is all handled within vite.config.ts

behei-vonage · 2025-04-28T17:18:38Z

frontend/src/hooks/tests/useChat.spec.tsx

@@ -1,6 +1,6 @@
 import { act, renderHook } from '@testing-library/react';
 import { beforeEach, describe, expect, it, Mock, Mocked, vi } from 'vitest';
-import { MutableRefObject } from 'react';


Note: MutableRefObject has been discontinued in React 19. SonarCloud is very unhappy about keeping this even though it is not currently a breaking change. This has been replaced with RefObjects. All other changes related to MutableRefObject are for the same reason.

To confirm, there's no way to disambiguate between mutable and immutable ref objects?

correct, they're all refobjects now and it is mutable

Copilot

Pull Request Overview

This PR upgrades the React version from 18 to 19 and introduces the React Compiler via a Babel plugin, while also refactoring several type definitions by replacing MutableRefObject with RefObject in both production code and tests. Additionally, tests are updated to properly wrap components with BrowserRouter for rendering.

Reviewed Changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
frontend/vite.config.ts	Added "isTest" flag and Babel plugin configuration for React Compiler.
frontend/src/utils/helpers/getLayoutBoxes/getLayoutBoxes.ts	Replaced MutableRefObject with RefObject in ref type.
frontend/src/utils/helpers/getLayoutBoxes/getLayoutBoxes.spec.ts	Updated ref type in tests accordingly.
frontend/src/pages/UnsupportedBrowserPage/UnsupportedBrowserPage.spec.tsx	Wrapped component in BrowserRouter for improved rendering in tests.
frontend/src/pages/GoodBye/GoodBye.spec.tsx	Wrapped GoodBye in BrowserRouter and updated expectations for UI.
frontend/src/hooks/useToolbarButtons.tsx	Updated ref types to RefObject.
frontend/src/hooks/useElementDimensions.tsx	Updated ref type and initialized the ResizeObserver ref explicitly.
frontend/src/hooks/useChat.tsx	Changed default value logic using the nullish coalescing operator.
frontend/src/hooks/useArchives.tsx	Initialized the pollingIntervalRef with undefined.
frontend/src/hooks/tests/useChat.spec.tsx	Updated ref type in tests.
frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.tsx	Adjusted ref type in props for anchorRef to support null value.
frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx	Updated tests to check for CSS class "Mui-selected" instead of HTML select state.
frontend/src/components/MeetingRoom/DeviceControlButton/DeviceControlButton.tsx	Adjusted ref type in DeviceControlButton for anchorRef.
frontend/src/Context/PublisherProvider/usePublisherOptions/usePublisherOptions.tsx	Changed deviceStoreRef type to include null initially.

Files not reviewed (1)

frontend/package.json: Language not supported

Comments suppressed due to low confidence (1)

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx:115

[nitpick] If this assertion is synchronous, consider removing the 'await' keyword to improve test clarity and consistency.

await expect((outputDevicesElement.firstChild as Element).classList.contains('Mui-selected')).toBe(true);

maikthomas

Nice one, good to keep up to date 👍 Left a couple q's

maikthomas · 2025-04-30T12:37:19Z

frontend/src/components/MeetingRoom/ReportIssue/FeedbackForm/HelperText/HelperText.tsx

@@ -40,7 +40,7 @@ const HelperText = ({
  };

  return (
-    <Box display="flex" justifyContent="space-between" width="100%">
+    <Box component="span" display="flex" justifyContent="space-between" width="100%">


Was this a span before? I think technically spans aren't meant to contain child elements

this was just a <p> before with some <div>s inside of it and it was failing with:

In HTML, <div> cannot be a descendant of <p>. This will cause a hydration error.

This should be okay. The component will be a span containing more spans.

The semantics are odd. Box implies "block level" element. By contrast, span implies "inlined" elements, like Mike mentioned.

Do we need to add component="span"?

yes I think we need to add that component="span" here since this HelperText component is used inside of another component, FeedbackForm. the FeedbackForm consists out of a bunch of <p>s and nesting a <div> inside of it causes a hydration error (see comment above). That's why we need to set Box components to span so rendering looks like this:

<p> <span>...</span> <span>...</span> </p> <p> .... </p>

frontend/src/hooks/useChat.tsx

maikthomas · 2025-04-30T12:39:17Z

frontend/vite.config.ts

+    react({
+      babel: {
+        plugins: isTest
+          ? [] // Disable babel-plugin-react-compiler in test environment
+          : [['babel-plugin-react-compiler', { target: '19' }]],
+      },
+    }),


is this a recommended workaround? Are there more people dealing with this?

cpettet

Looks good so far! Just have a few comments/questions. Let me know what you think!

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx

cpettet · 2025-04-30T20:26:29Z

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx

-      expect((outputDevicesElement.firstChild as HTMLOptionElement).selected).toBe(true);
+      expect((outputDevicesElement.firstChild as Element).classList.contains('Mui-selected')).toBe(
+        true
+      );


Can we be more specific than Element? Should this be a div, an li, span, or is it unknown so an HTMLElement?

cpettet · 2025-04-30T20:27:53Z

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx

@@ -120,7 +123,9 @@ describe('DeviceSettingsMenu Component', () => {
      await act(() => (outputDevicesElement.children[2] as HTMLOptionElement).click?.());

      expect(mockSetAudioOutputDevice).toHaveBeenCalledWith(audioOutputDevices[2].deviceId);
-      await expect((outputDevicesElement.children[2] as HTMLOptionElement).selected).toBe(true);
+      expect((outputDevicesElement.children[2] as Element).classList.contains('Mui-selected')).toBe(


Is it possible to check the visual indicator we use is present for this output device? I'm not a big fan of checking the presence of a class.

I don't see a particular issue with checking for a class. I will add an extra expect to check that the following element is not currently selected, though, to be extra certain

cpettet · 2025-04-30T20:43:53Z

frontend/src/components/MeetingRoom/ReportIssue/FeedbackForm/HelperText/HelperText.tsx

@@ -40,7 +40,7 @@ const HelperText = ({
  };

  return (
-    <Box display="flex" justifyContent="space-between" width="100%">
+    <Box component="span" display="flex" justifyContent="space-between" width="100%">


This should be okay. The component will be a span containing more spans.

cpettet

LGTM Great job!

v-kpheng · 2025-05-02T17:04:12Z

@behei-vonage, some nice initiative here 💪

That said, some questions/comments:

Using a de-facto alpha release makes me feel iffy. I know we're using some beta packages too, but still
Do we have measurements to contrast how much more responsive the changes make VERA? If not, then maybe we can add telemetry to gauge the improvements these changes make

v-kpheng

Looks good! 💪

That said, left some questions/comments. Thanks! 🙏

v-kpheng · 2025-05-02T20:22:38Z

frontend/package.json

+    "@testing-library/jest-dom": "^6.6.3",
+    "@testing-library/react": "^16.3.0",
+    "@testing-library/user-event": "^14.6.1",
+    "@vitejs/plugin-react": "^4.4.1",


Do these need to be upgraded as well? If so, it's because the new version of React requires this, yes?

yes, react 19 requires these packages to be upgraded

v-kpheng · 2025-05-02T20:23:29Z

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx

@@ -1,6 +1,6 @@
 import { act, queryByText, render, screen, waitFor } from '@testing-library/react';
 import { describe, beforeEach, it, Mock, vi, expect, afterAll } from 'vitest';
-import { MutableRefObject } from 'react';
+import { RefObject } from 'react';


For React 19, all RefObjects are implicitly mutable now?

that is correct, mutablerefobject has been deprecated and it's all on refobject now

frontend/src/components/MeetingRoom/DeviceSettingsMenu/DeviceSettingsMenu.spec.tsx

v-kpheng · 2025-05-02T20:33:20Z

frontend/src/components/MeetingRoom/ReportIssue/FeedbackForm/HelperText/HelperText.tsx

@@ -40,7 +40,7 @@ const HelperText = ({
  };

  return (
-    <Box display="flex" justifyContent="space-between" width="100%">
+    <Box component="span" display="flex" justifyContent="space-between" width="100%">


The semantics are odd. Box implies "block level" element. By contrast, span implies "inlined" elements, like Mike mentioned.

Do we need to add component="span"?

v-kpheng · 2025-05-02T20:34:13Z

frontend/src/hooks/tests/useChat.spec.tsx

@@ -1,6 +1,6 @@
 import { act, renderHook } from '@testing-library/react';
 import { beforeEach, describe, expect, it, Mock, Mocked, vi } from 'vitest';
-import { MutableRefObject } from 'react';


To confirm, there's no way to disambiguate between mutable and immutable ref objects?

sonarqubecloud · 2025-05-05T19:55:35Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
96.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

v-kpheng

LGTM! 💪 🚀

cpettet

LGTM Great job!

dwivedisachin · 2025-05-06T15:19:40Z

Tested LGTM!! 🚀

behei-vonage added 2 commits April 25, 2025 15:55

update to 19 works

615b9bb

seems okay

166c83c

behei-vonage self-assigned this Apr 28, 2025

behei-vonage commented Apr 28, 2025

View reviewed changes

behei-vonage added the do-not-review Do not review label Apr 28, 2025

behei-vonage added 4 commits April 28, 2025 11:55

mutablerefobject replace in toolbar buttons

1878672

usechat mutableref

217b3ee

mutablerefobject stuff

69ca96f

keep sonar even happier

f92161f

behei-vonage commented Apr 28, 2025

View reviewed changes

put the comments back

8d24078

behei-vonage requested a review from Copilot April 28, 2025 17:20

Copilot AI reviewed Apr 28, 2025

View reviewed changes

behei-vonage removed the do-not-review Do not review label Apr 28, 2025

behei-vonage added 2 commits April 28, 2025 20:33

upgrade to version of mui that supports react 19

1b77bb2

another one

53e1891

maikthomas reviewed Apr 30, 2025

View reviewed changes

not today sonarcloud

49ad6e6

behei-vonage requested a review from maikthomas April 30, 2025 13:43

cpettet reviewed Apr 30, 2025

View reviewed changes

feedback

a178078

behei-vonage requested a review from cpettet April 30, 2025 21:03

cpettet previously approved these changes May 2, 2025

View reviewed changes

behei-vonage changed the title ~~VIDCS-3681: Bump up React version and introduce React Compiler~~ VIDCS-3681: Bump up React version May 2, 2025

react 19 upgrade only for now

41a1dc4

behei-vonage dismissed cpettet’s stale review via 41a1dc4 May 2, 2025 19:16

behei-vonage closed this May 2, 2025

behei-vonage reopened this May 2, 2025

upgrade vite while we are at it

1a2e7c8

behei-vonage requested review from cpettet and v-kpheng May 2, 2025 20:18

v-kpheng reviewed May 2, 2025

View reviewed changes

behei-vonage requested a review from v-kpheng May 5, 2025 19:13

remove comments

9fd0a3b

v-kpheng approved these changes May 5, 2025

View reviewed changes

cpettet approved these changes May 6, 2025

View reviewed changes

behei-vonage merged commit 56c9ee2 into develop May 6, 2025
6 of 7 checks passed

behei-vonage deleted the behei-vonage/vidcs-3681-bump branch May 6, 2025 15:43

VIDCS-3681: Bump up React version #154

VIDCS-3681: Bump up React version #154

Conversation

behei-vonage commented Apr 28, 2025 • edited Loading

What is this PR doing?

How should this be manually tested?

What are the relevant tickets?

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

maikthomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpettet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpettet left a comment

Choose a reason for hiding this comment

v-kpheng commented May 2, 2025

v-kpheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented May 5, 2025

Quality Gate passed

v-kpheng left a comment

Choose a reason for hiding this comment

cpettet left a comment

Choose a reason for hiding this comment

dwivedisachin commented May 6, 2025

behei-vonage commented Apr 28, 2025 •

edited

Loading