Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable react-compiler eslint to spot antipatterns #28652

Merged
merged 16 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -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"],
Expand Down Expand Up @@ -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: [
{
Expand Down Expand Up @@ -262,6 +264,7 @@ module.exports = {

// These are fine in tests
"no-restricted-globals": "off",
"react-compiler/react-compiler": "off",
},
},
{
Expand Down
1 change: 1 addition & 0 deletions __mocks__/maplibre-gl.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class MockMap extends EventEmitter {
setCenter = jest.fn();
setStyle = jest.fn();
fitBounds = jest.fn();
remove = jest.fn();
}
const MockMapInstance = new MockMap();

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/accessibility/RovingTabIndex.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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];
};
Expand Down
1 change: 1 addition & 0 deletions src/components/structures/AutocompleteInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
>
Expand Down
1 change: 1 addition & 0 deletions src/components/structures/ContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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];
};

Expand Down
4 changes: 1 addition & 3 deletions src/components/structures/FilePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/NotificationPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions src/components/structures/RoomSearchView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(() => {
Expand Down
5 changes: 2 additions & 3 deletions src/components/structures/RoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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}>
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/ThreadView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 && (
Expand Down
35 changes: 8 additions & 27 deletions src/components/utils/Box.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
/**
Expand Down Expand Up @@ -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
*/
Expand All @@ -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(
Expand All @@ -88,7 +69,7 @@ export function Box({
"mx_Box--shrink": !!shrink,
"mx_Box--grow": !!grow,
}),
ref,
style,
},
children,
);
Expand Down
23 changes: 12 additions & 11 deletions src/components/utils/Flex.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
/**
Expand Down Expand Up @@ -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);
}
31 changes: 10 additions & 21 deletions src/components/viewmodels/memberlist/MemberListViewModel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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(
Expand Down Expand Up @@ -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,
};
}
3 changes: 1 addition & 2 deletions src/components/views/elements/EffectsOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 5 additions & 5 deletions src/components/views/elements/Measured.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/components/views/right_panel/TimelineCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/components/views/rooms/ReadReceiptGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ export function ReadReceiptGroup({
readReceiptPosition = readReceiptMap[userId];
if (!readReceiptPosition) {
readReceiptPosition = {};
// eslint-disable-next-line react-compiler/react-compiler
readReceiptMap[userId] = readReceiptPosition;
}
}
Expand Down
Loading
Loading