Enable react-compiler eslint to spot antipatterns (#28652)

* Switch to React18 useId Signed-off-by: Michael Telatynski <[email protected]> * Enable react-compiler eslint Signed-off-by: Michael Telatynski <[email protected]> * Fix an easy one Signed-off-by: Michael Telatynski <[email protected]> * Disable in tests Signed-off-by: Michael Telatynski <[email protected]> * Fix usage of useRef as memoization Signed-off-by: Michael Telatynski <[email protected]> * Fix mutation of external values in hooks Signed-off-by: Michael Telatynski <[email protected]> * Make React compiler happy about some frankly non-issues Signed-off-by: Michael Telatynski <[email protected]> * Fix MapMock Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Revert MemberListViewModel.tsx changes and disable linter per line Signed-off-by: Michael Telatynski <[email protected]> * Make viewmodel compatible with react-compiler linter - Remove searchQuery ref/state and instead pass this query to the loadMember function. - Now we no longer need a separate search function --------- Signed-off-by: Michael Telatynski <[email protected]> Co-authored-by: R Midhun Suresh <[email protected]>
element-hq · Jan 16, 2025 · ef1597f · ef1597f
1 parent e5ca795
commit ef1597f
Show file tree

Hide file tree

Showing 35 changed files with 146 additions and 136 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -1,5 +1,5 @@
 module.exports = {
-    plugins: ["matrix-org"],
+    plugins: ["matrix-org", "eslint-plugin-react-compiler"],
     extends: ["plugin:matrix-org/babel", "plugin:matrix-org/react", "plugin:matrix-org/a11y"],
     parserOptions: {
         project: ["./tsconfig.json"],
@@ -170,6 +170,8 @@ module.exports = {
         "jsx-a11y/role-supports-aria-props": "off",
 
         "matrix-org/require-copyright-header": "error",
+
+        "react-compiler/react-compiler": "error",
     },
     overrides: [
         {
@@ -262,6 +264,7 @@ module.exports = {
 
                 // These are fine in tests
                 "no-restricted-globals": "off",
+                "react-compiler/react-compiler": "off",
             },
         },
         {

diff --git a/__mocks__/maplibre-gl.js b/__mocks__/maplibre-gl.js
@@ -17,6 +17,7 @@ class MockMap extends EventEmitter {
     setCenter = jest.fn();
     setStyle = jest.fn();
     fitBounds = jest.fn();
+    remove = jest.fn();
 }
 const MockMapInstance = new MockMap();
 

diff --git a/package.json b/package.json
@@ -237,6 +237,7 @@
         "eslint-plugin-jsx-a11y": "^6.5.1",
         "eslint-plugin-matrix-org": "^2.0.2",
         "eslint-plugin-react": "^7.28.0",
+        "eslint-plugin-react-compiler": "^19.0.0-beta-df7b47d-20241124",
         "eslint-plugin-react-hooks": "^5.0.0",
         "eslint-plugin-unicorn": "^56.0.0",
         "express": "^4.18.2",

diff --git a/src/accessibility/RovingTabIndex.tsx b/src/accessibility/RovingTabIndex.tsx
@@ -392,6 +392,7 @@ export const useRovingTabIndex = <T extends HTMLElement>(
         });
     }, []); // eslint-disable-line react-hooks/exhaustive-deps
 
+    // eslint-disable-next-line react-compiler/react-compiler
     const isActive = context.state.activeNode === nodeRef.current;
     return [onFocus, isActive, ref, nodeRef];
 };

diff --git a/src/components/structures/AutocompleteInput.tsx b/src/components/structures/AutocompleteInput.tsx
@@ -142,6 +142,7 @@ export const AutocompleteInput: React.FC<AutocompleteInputProps> = ({
             {isFocused && suggestions.length ? (
                 <div
                     className="mx_AutocompleteInput_matches"
+                    // eslint-disable-next-line react-compiler/react-compiler
                     style={{ top: editorContainerRef.current?.clientHeight }}
                     data-testid="autocomplete-matches"
                 >

diff --git a/src/components/structures/ContextMenu.tsx b/src/components/structures/ContextMenu.tsx
@@ -607,6 +607,7 @@ export const useContextMenu = <T extends any = HTMLElement>(inputRef?: RefObject
         setIsOpen(false);
     };
 
+    // eslint-disable-next-line react-compiler/react-compiler
     return [button.current ? isOpen : false, button, open, close, setIsOpen];
 };
 

diff --git a/src/components/structures/FilePanel.tsx b/src/components/structures/FilePanel.tsx
@@ -286,9 +286,7 @@ class FilePanel extends React.Component<IProps, IState> {
                         ref={this.card}
                         header={_t("right_panel|files_button")}
                     >
-                        {this.card.current && (
-                            <Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />
-                        )}
+                        <Measured sensor={this.card} onMeasurement={this.onMeasurement} />
                         <SearchWarning isRoomEncrypted={isRoomEncrypted} kind={WarningKind.Files} />
                         <TimelinePanel
                             manageReadReceipts={false}

diff --git a/src/components/structures/NotificationPanel.tsx b/src/components/structures/NotificationPanel.tsx
@@ -95,7 +95,7 @@ export default class NotificationPanel extends React.PureComponent<IProps, IStat
                     onClose={this.props.onClose}
                     withoutScrollContainer={true}
                 >
-                    {this.card.current && <Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />}
+                    <Measured sensor={this.card} onMeasurement={this.onMeasurement} />
                     {content}
                 </BaseCard>
             </ScopedRoomContextProvider>

diff --git a/src/components/structures/RoomSearchView.tsx b/src/components/structures/RoomSearchView.tsx
@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import React, { forwardRef, useCallback, useContext, useEffect, useRef, useState } from "react";
+import React, { forwardRef, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react";
 import {
     ISearchResults,
     IThreadBundledRelationship,
@@ -58,7 +58,7 @@ export const RoomSearchView = forwardRef<ScrollPanel, Props>(
         const [results, setResults] = useState<ISearchResults | null>(null);
         const aborted = useRef(false);
         // A map from room ID to permalink creator
-        const permalinkCreators = useRef(new Map<string, RoomPermalinkCreator>()).current;
+        const permalinkCreators = useMemo(() => new Map<string, RoomPermalinkCreator>(), []);
         const innerRef = useRef<ScrollPanel | null>();
 
         useEffect(() => {

diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx
@@ -273,6 +273,7 @@ function LocalRoomView(props: LocalRoomViewProps): ReactElement {
     }
 
     const onRetryClicked = (): void => {
+        // eslint-disable-next-line react-compiler/react-compiler
         room.state = LocalRoomState.NEW;
         defaultDispatcher.dispatch({
             action: "local_room_event",
@@ -2514,9 +2515,7 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
                 mainSplitContentClassName = "mx_MainSplit_timeline";
                 mainSplitBody = (
                     <>
-                        {this.roomViewBody.current && (
-                            <Measured sensor={this.roomViewBody.current} onMeasurement={this.onMeasurement} />
-                        )}
+                        <Measured sensor={this.roomViewBody} onMeasurement={this.onMeasurement} />
                         {auxPanel}
                         {pinnedMessageBanner}
                         <main className={timelineClasses}>

diff --git a/src/components/structures/ThreadPanel.tsx b/src/components/structures/ThreadPanel.tsx
@@ -204,7 +204,7 @@ const ThreadPanel: React.FC<IProps> = ({ roomId, onClose, permalinkCreator }) =>
                 ref={card}
                 closeButtonRef={closeButonRef}
             >
-                {card.current && <Measured sensor={card.current} onMeasurement={setNarrow} />}
+                <Measured sensor={card} onMeasurement={setNarrow} />
                 {timelineSet ? (
                     <TimelinePanel
                         key={filterOption + ":" + (timelineSet.getFilter()?.filterId ?? roomId)}

diff --git a/src/components/structures/ThreadView.tsx b/src/components/structures/ThreadView.tsx
@@ -443,7 +443,7 @@ export default class ThreadView extends React.Component<IProps, IState> {
                         PosthogTrackers.trackInteraction("WebThreadViewBackButton", ev);
                     }}
                 >
-                    {this.card.current && <Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />}
+                    <Measured sensor={this.card} onMeasurement={this.onMeasurement} />
                     <div className="mx_ThreadView_timelinePanelWrapper">{timeline}</div>
 
                     {ContentMessages.sharedInstance().getCurrentUploads(threadRelation).length > 0 && (

diff --git a/src/components/utils/Box.tsx b/src/components/utils/Box.tsx
@@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import classNames from "classnames";
-import React, { useEffect, useRef } from "react";
+import React, { useMemo } from "react";
 
 type FlexProps = {
     /**
@@ -40,25 +40,6 @@ type FlexProps = {
     grow?: string | null;
 };
 
-/**
- * Set or remove a CSS property
- * @param ref the reference
- * @param name the CSS property name
- * @param value the CSS property value
- */
-function addOrRemoveProperty(
-    ref: React.MutableRefObject<HTMLElement | undefined>,
-    name: string,
-    value?: string | null,
-): void {
-    const style = ref.current!.style;
-    if (value) {
-        style.setProperty(name, value);
-    } else {
-        style.removeProperty(name);
-    }
-}
-
 /**
  * A flex child helper
  */
@@ -71,12 +52,12 @@ export function Box({
     children,
     ...props
 }: React.PropsWithChildren<FlexProps>): JSX.Element {
-    const ref = useRef<HTMLElement>();
-
-    useEffect(() => {
-        addOrRemoveProperty(ref, `--mx-box-flex`, flex);
-        addOrRemoveProperty(ref, `--mx-box-shrink`, shrink);
-        addOrRemoveProperty(ref, `--mx-box-grow`, grow);
+    const style = useMemo(() => {
+        const style: Record<string, any> = {};
+        if (flex) style["--mx-box-flex"] = flex;
+        if (shrink) style["--mx-box-shrink"] = shrink;
+        if (grow) style["--mx-box-grow"] = grow;
+        return style;
     }, [flex, grow, shrink]);
 
     return React.createElement(
@@ -88,7 +69,7 @@ export function Box({
                 "mx_Box--shrink": !!shrink,
                 "mx_Box--grow": !!grow,
             }),
-            ref,
+            style,
         },
         children,
     );

diff --git a/src/components/utils/Flex.tsx b/src/components/utils/Flex.tsx
@@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import classNames from "classnames";
-import React, { useEffect, useRef } from "react";
+import React, { useMemo } from "react";
 
 type FlexProps = {
     /**
@@ -64,15 +64,16 @@ export function Flex({
     children,
     ...props
 }: React.PropsWithChildren<FlexProps>): JSX.Element {
-    const ref = useRef<HTMLElement>();
+    const style = useMemo(
+        () => ({
+            "--mx-flex-display": display,
+            "--mx-flex-direction": direction,
+            "--mx-flex-align": align,
+            "--mx-flex-justify": justify,
+            "--mx-flex-gap": gap,
+        }),
+        [align, direction, display, gap, justify],
+    );
 
-    useEffect(() => {
-        ref.current!.style.setProperty(`--mx-flex-display`, display);
-        ref.current!.style.setProperty(`--mx-flex-direction`, direction);
-        ref.current!.style.setProperty(`--mx-flex-align`, align);
-        ref.current!.style.setProperty(`--mx-flex-justify`, justify);
-        ref.current!.style.setProperty(`--mx-flex-gap`, gap);
-    }, [align, direction, display, gap, justify]);
-
-    return React.createElement(as, { ...props, className: classNames("mx_Flex", className), ref }, children);
+    return React.createElement(as, { ...props, className: classNames("mx_Flex", className), style }, children);
 }
diff --git a/src/components/viewmodels/memberlist/MemberListViewModel.tsx b/src/components/viewmodels/memberlist/MemberListViewModel.tsx
@@ -19,7 +19,7 @@ import {
     UserEvent,
 } from "matrix-js-sdk/src/matrix";
 import { KnownMembership } from "matrix-js-sdk/src/types";
-import { useCallback, useContext, useEffect, useMemo, useRef, useState } from "react";
+import { useCallback, useContext, useEffect, useMemo, useState } from "react";
 import { throttle } from "lodash";
 
 import { RoomMember } from "../../../models/rooms/RoomMember";
@@ -120,19 +120,16 @@ export function useMemberListViewModel(roomId: string): MemberListViewState {
     const sdkContext = useContext(SDKContext);
     const [memberMap, setMemberMap] = useState<Map<string, Member>>(new Map());
     const [isLoading, setIsLoading] = useState<boolean>(true);
-
     // This is the last known total number of members in this room.
-    const totalMemberCount = useRef<number>(0);
-
-    const searchQuery = useRef("");
+    const [totalMemberCount, setTotalMemberCount] = useState(0);
 
     const loadMembers = useMemo(
         () =>
             throttle(
-                async (): Promise<void> => {
+                async (searchQuery?: string): Promise<void> => {
                     const { joined: joinedSdk, invited: invitedSdk } = await sdkContext.memberListStore.loadMemberList(
                         roomId,
-                        searchQuery.current,
+                        searchQuery,
                     );
                     const newMemberMap = new Map<string, Member>();
                     // First add the invited room members
@@ -141,7 +138,7 @@ export function useMemberListViewModel(roomId: string): MemberListViewState {
                         newMemberMap.set(member.userId, roomMember);
                     }
                     // Then add the third party invites
-                    const threePidInvited = getPending3PidInvites(room, searchQuery.current);
+                    const threePidInvited = getPending3PidInvites(room, searchQuery);
                     for (const invited of threePidInvited) {
                         const key = invited.threePidInvite!.event.getContent().display_name;
                         newMemberMap.set(key, invited);
@@ -152,26 +149,18 @@ export function useMemberListViewModel(roomId: string): MemberListViewState {
                         newMemberMap.set(member.userId, roomMember);
                     }
                     setMemberMap(newMemberMap);
-                    if (!searchQuery.current) {
+                    if (!searchQuery) {
                         /**
                          * Since searching for members only gives you the relevant
                          * members matching the query, do not update the totalMemberCount!
                          **/
-                        totalMemberCount.current = newMemberMap.size;
+                        setTotalMemberCount(newMemberMap.size);
                     }
                 },
                 500,
                 { leading: true, trailing: true },
             ),
-        [roomId, sdkContext.memberListStore, room],
-    );
-
-    const search = useCallback(
-        (query: string) => {
-            searchQuery.current = query;
-            loadMembers();
-        },
-        [loadMembers],
+        [sdkContext.memberListStore, roomId, room],
     );
 
     const isPresenceEnabled = useMemo(
@@ -252,12 +241,12 @@ export function useMemberListViewModel(roomId: string): MemberListViewState {
 
     return {
         members: Array.from(memberMap.values()),
-        search,
+        search: loadMembers,
         shouldShowInvite,
         isPresenceEnabled,
         isLoading,
         onInviteButtonClick,
-        shouldShowSearch: totalMemberCount.current >= 20,
+        shouldShowSearch: totalMemberCount >= 20,
         canInvite,
     };
 }
diff --git a/src/components/views/elements/EffectsOverlay.tsx b/src/components/views/elements/EffectsOverlay.tsx
@@ -58,11 +58,10 @@ const EffectsOverlay: FunctionComponent<IProps> = ({ roomWidth }) => {
         if (canvas) canvas.height = UIStore.instance.windowHeight;
         UIStore.instance.on(UI_EVENTS.Resize, resize);
 
+        const currentEffects = effectsRef.current; // this is not a react node ref, warning can be safely ignored
         return () => {
             dis.unregister(dispatcherRef);
             UIStore.instance.off(UI_EVENTS.Resize, resize);
-            // eslint-disable-next-line react-hooks/exhaustive-deps
-            const currentEffects = effectsRef.current; // this is not a react node ref, warning can be safely ignored
             for (const effect in currentEffects) {
                 const effectModule: ICanvasEffect = currentEffects.get(effect)!;
                 if (effectModule && effectModule.isRunning) {

diff --git a/src/components/views/elements/Measured.tsx b/src/components/views/elements/Measured.tsx
@@ -6,12 +6,12 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import React from "react";
+import React, { RefObject } from "react";
 
 import UIStore, { UI_EVENTS } from "../../../stores/UIStore";
 
 interface IProps {
-    sensor: Element;
+    sensor: RefObject<Element>;
     breakpoint: number;
     onMeasurement(narrow: boolean): void;
 }
@@ -35,14 +35,14 @@ export default class Measured extends React.PureComponent<IProps> {
     }
 
     public componentDidUpdate(prevProps: Readonly<IProps>): void {
-        const previous = prevProps.sensor;
-        const current = this.props.sensor;
+        const previous = prevProps.sensor.current;
+        const current = this.props.sensor.current;
         if (previous === current) return;
         if (previous) {
             UIStore.instance.stopTrackingElementDimensions(`Measured${this.instanceId}`);
         }
         if (current) {
-            UIStore.instance.trackElementDimensions(`Measured${this.instanceId}`, this.props.sensor);
+            UIStore.instance.trackElementDimensions(`Measured${this.instanceId}`, this.props.sensor.current);
         }
     }
 

diff --git a/src/components/views/right_panel/TimelineCard.tsx b/src/components/views/right_panel/TimelineCard.tsx
@@ -213,7 +213,7 @@ export default class TimelineCard extends React.Component<IProps, IState> {
                     header={_t("right_panel|video_room_chat|title")}
                     ref={this.card}
                 >
-                    {this.card.current && <Measured sensor={this.card.current} onMeasurement={this.onMeasurement} />}
+                    <Measured sensor={this.card} onMeasurement={this.onMeasurement} />
                     <div className="mx_TimelineCard_timeline">
                         {jumpToBottom}
                         <TimelinePanel

diff --git a/src/components/views/rooms/ReadReceiptGroup.tsx b/src/components/views/rooms/ReadReceiptGroup.tsx
@@ -109,6 +109,7 @@ export function ReadReceiptGroup({
                 readReceiptPosition = readReceiptMap[userId];
                 if (!readReceiptPosition) {
                     readReceiptPosition = {};
+                    // eslint-disable-next-line react-compiler/react-compiler
                     readReceiptMap[userId] = readReceiptPosition;
                 }
             }
-Original file line number
+Diff line change
@@ Expand Up @@
             setIsOpen(false);
         };
+        // eslint-disable-next-line react-compiler/react-compiler
         return [button.current ? isOpen : false, button, open, close, setIsOpen];
     };
@@ Expand Down @@