Add lint rule to protect against this access on unbound methods (#32578)
* Add Actions to ViewModel utility types and specify `this: void` signature Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add https://typescript-eslint.io/rules/unbound-method/ linter to shared-components also fix stray lint config which doesn't apply to SC Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add https://typescript-eslint.io/rules/unbound-method/ linter to element-web Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix genuine issues identified by the linter Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Specify this:void on i18napi Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Update Module API Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Add comment for MapToVoidThis Added utility type to map VM actions to unbound functions. --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
committed by
GitHub
parent
87b28b725c
commit
77670eb369
@@ -9,9 +9,10 @@ module.exports = {
|
||||
root: true,
|
||||
plugins: ["matrix-org", "eslint-plugin-react-compiler"],
|
||||
extends: [
|
||||
"plugin:matrix-org/babel",
|
||||
"plugin:matrix-org/react",
|
||||
"plugin:matrix-org/a11y",
|
||||
"plugin:matrix-org/typescript",
|
||||
"plugin:matrix-org/react",
|
||||
"plugin:storybook/recommended",
|
||||
],
|
||||
parserOptions: {
|
||||
@@ -42,37 +43,35 @@ module.exports = {
|
||||
],
|
||||
},
|
||||
],
|
||||
|
||||
"@typescript-eslint/unbound-method": ["error", { ignoreStatic: true }],
|
||||
"@typescript-eslint/explicit-function-return-type": [
|
||||
"error",
|
||||
{
|
||||
allowExpressions: true,
|
||||
},
|
||||
],
|
||||
|
||||
// We're okay being explicit at the moment
|
||||
// "@typescript-eslint/no-empty-interface": "off",
|
||||
// We'd rather not do this but we do
|
||||
// "@typescript-eslint/ban-ts-comment": "off",
|
||||
// We're okay with assertion errors when we ask for them
|
||||
"@typescript-eslint/no-non-null-assertion": "off",
|
||||
"@typescript-eslint/no-empty-object-type": [
|
||||
"error",
|
||||
{
|
||||
// We do this sometimes to brand interfaces
|
||||
allowInterfaces: "with-single-extends",
|
||||
},
|
||||
],
|
||||
},
|
||||
overrides: [
|
||||
{
|
||||
files: ["src/**/*.{ts,tsx}", "test/**/*.{ts,tsx}"],
|
||||
extends: ["plugin:matrix-org/typescript", "plugin:matrix-org/react"],
|
||||
files: ["src/**/*.test.{ts,tsx}"],
|
||||
rules: {
|
||||
"@typescript-eslint/explicit-function-return-type": [
|
||||
"error",
|
||||
{
|
||||
allowExpressions: true,
|
||||
},
|
||||
],
|
||||
|
||||
// Remove Babel things manually due to override limitations
|
||||
"@babel/no-invalid-this": ["off"],
|
||||
|
||||
// We're okay being explicit at the moment
|
||||
"@typescript-eslint/no-empty-interface": "off",
|
||||
// We disable this while we're transitioning
|
||||
"@typescript-eslint/unbound-method": "off",
|
||||
"@typescript-eslint/no-explicit-any": "off",
|
||||
// We'd rather not do this but we do
|
||||
"@typescript-eslint/ban-ts-comment": "off",
|
||||
// We're okay with assertion errors when we ask for them
|
||||
"@typescript-eslint/no-non-null-assertion": "off",
|
||||
"@typescript-eslint/no-empty-object-type": [
|
||||
"error",
|
||||
{
|
||||
// We do this sometimes to brand interfaces
|
||||
allowInterfaces: "with-single-extends",
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
|
||||
@@ -70,7 +70,7 @@ export interface AudioPlayerViewActions {
|
||||
/**
|
||||
* The view model for the audio player.
|
||||
*/
|
||||
export type AudioPlayerViewModel = ViewModel<AudioPlayerViewSnapshot> & AudioPlayerViewActions;
|
||||
export type AudioPlayerViewModel = ViewModel<AudioPlayerViewSnapshot, AudioPlayerViewActions>;
|
||||
|
||||
interface AudioPlayerViewProps {
|
||||
/**
|
||||
|
||||
+1
-1
@@ -84,7 +84,7 @@ interface DecryptionFailureBodyViewProps {
|
||||
/**
|
||||
* React ref to attach to any React components returned
|
||||
*/
|
||||
ref?: React.RefObject<any>;
|
||||
ref?: React.RefObject<HTMLDivElement>;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+4
-2
@@ -57,8 +57,10 @@ export interface DisambiguatedProfileViewActions {
|
||||
/**
|
||||
* The view model for DisambiguatedProfileView.
|
||||
*/
|
||||
export type DisambiguatedProfileViewModel = ViewModel<DisambiguatedProfileViewSnapshot> &
|
||||
DisambiguatedProfileViewActions;
|
||||
export type DisambiguatedProfileViewModel = ViewModel<
|
||||
DisambiguatedProfileViewSnapshot,
|
||||
DisambiguatedProfileViewActions
|
||||
>;
|
||||
|
||||
interface DisambiguatedProfileViewProps {
|
||||
/**
|
||||
|
||||
+1
-1
@@ -89,7 +89,7 @@ export interface WidgetContextMenuAction {
|
||||
onMoveButton: (direction: number) => void;
|
||||
}
|
||||
|
||||
export type WidgetContextMenuViewModel = ViewModel<WidgetContextMenuSnapshot> & WidgetContextMenuAction;
|
||||
export type WidgetContextMenuViewModel = ViewModel<WidgetContextMenuSnapshot, WidgetContextMenuAction>;
|
||||
|
||||
interface WidgetContextMenuViewProps {
|
||||
vm: WidgetContextMenuViewModel;
|
||||
|
||||
@@ -103,7 +103,7 @@ export interface RoomListHeaderViewActions {
|
||||
/**
|
||||
* The view model for the room list header component.
|
||||
*/
|
||||
export type RoomListHeaderViewModel = ViewModel<RoomListHeaderViewSnapshot> & RoomListHeaderViewActions;
|
||||
export type RoomListHeaderViewModel = ViewModel<RoomListHeaderViewSnapshot, RoomListHeaderViewActions>;
|
||||
|
||||
interface RoomListHeaderViewProps {
|
||||
/**
|
||||
|
||||
+1
-1
@@ -25,7 +25,7 @@ import type { RoomListItemSnapshot, RoomListItemActions } from "./RoomListItemVi
|
||||
/**
|
||||
* View model type for room list item
|
||||
*/
|
||||
export type RoomItemViewModel = ViewModel<RoomListItemSnapshot> & RoomListItemActions;
|
||||
export type RoomItemViewModel = ViewModel<RoomListItemSnapshot, RoomListItemActions>;
|
||||
|
||||
/**
|
||||
* Props for RoomListItemMoreOptionsMenu component
|
||||
|
||||
+1
-1
@@ -21,7 +21,7 @@ import type { RoomListItemSnapshot, RoomListItemActions } from "./RoomListItemVi
|
||||
/**
|
||||
* View model type for room list item
|
||||
*/
|
||||
export type RoomItemViewModel = ViewModel<RoomListItemSnapshot> & RoomListItemActions;
|
||||
export type RoomItemViewModel = ViewModel<RoomListItemSnapshot, RoomListItemActions>;
|
||||
|
||||
/**
|
||||
* Props for RoomListItemNotificationMenu component
|
||||
|
||||
@@ -105,7 +105,7 @@ export interface RoomListItemActions {
|
||||
/**
|
||||
* The view model type for a room list item
|
||||
*/
|
||||
export type RoomItemViewModel = ViewModel<RoomListItemSnapshot> & RoomListItemActions;
|
||||
export type RoomItemViewModel = ViewModel<RoomListItemSnapshot, RoomListItemActions>;
|
||||
|
||||
/**
|
||||
* Props for RoomListItemView component
|
||||
|
||||
@@ -50,7 +50,7 @@ export interface RoomListSearchViewActions {
|
||||
/**
|
||||
* The view model for the room list search component.
|
||||
*/
|
||||
export type RoomListSearchViewModel = ViewModel<RoomListSearchViewSnapshot> & RoomListSearchViewActions;
|
||||
export type RoomListSearchViewModel = ViewModel<RoomListSearchViewSnapshot, RoomListSearchViewActions>;
|
||||
|
||||
interface RoomListSearchViewProps {
|
||||
/**
|
||||
|
||||
@@ -12,7 +12,7 @@ import { RoomListPrimaryFilters, type FilterId } from "../RoomListPrimaryFilters
|
||||
import { RoomListLoadingSkeleton } from "./RoomListLoadingSkeleton";
|
||||
import { RoomListEmptyStateView } from "./RoomListEmptyStateView";
|
||||
import { VirtualizedRoomListView, type RoomListViewState } from "../VirtualizedRoomListView";
|
||||
import { type Room } from "../RoomListItemView";
|
||||
import { type Room, type RoomItemViewModel } from "../RoomListItemView";
|
||||
|
||||
/**
|
||||
* Snapshot for the room list view
|
||||
@@ -49,7 +49,7 @@ export interface RoomListViewActions {
|
||||
/** Called to create a new room */
|
||||
createRoom: () => void;
|
||||
/** Get view model for a specific room (virtualization API) */
|
||||
getRoomItemViewModel: (roomId: string) => any;
|
||||
getRoomItemViewModel: (roomId: string) => RoomItemViewModel;
|
||||
/** Called when the visible range changes (virtualization API) */
|
||||
updateVisibleRooms: (startIndex: number, endIndex: number) => void;
|
||||
}
|
||||
@@ -57,7 +57,7 @@ export interface RoomListViewActions {
|
||||
/**
|
||||
* The view model type for the room list view
|
||||
*/
|
||||
export type RoomListViewModel = ViewModel<RoomListSnapshot> & RoomListViewActions;
|
||||
export type RoomListViewModel = ViewModel<RoomListSnapshot, RoomListViewActions>;
|
||||
|
||||
/**
|
||||
* Props for RoomListView component
|
||||
|
||||
@@ -8,7 +8,7 @@
|
||||
import React from "react";
|
||||
import { fn } from "storybook/test";
|
||||
|
||||
import { type Room, type RoomListItemSnapshot, RoomNotifState } from "./RoomListItemView";
|
||||
import { type Room, type RoomItemViewModel, type RoomListItemSnapshot, RoomNotifState } from "./RoomListItemView";
|
||||
|
||||
/**
|
||||
* Mock avatar component for stories
|
||||
@@ -39,6 +39,7 @@ export const mockAvatar = (name: string): React.ReactElement => (
|
||||
*/
|
||||
export const renderAvatar = (room: Room): React.ReactElement => {
|
||||
// Cast to any to access properties - in real usage, the room object from the SDK will have these
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
return mockAvatar((room as any)?.name || "Room");
|
||||
};
|
||||
|
||||
@@ -102,8 +103,8 @@ export const createMockRoomSnapshot = (id: string, name: string, index: number):
|
||||
/**
|
||||
* Create a mock getRoomItemViewModel function for stories
|
||||
*/
|
||||
export const createGetRoomItemViewModel = (roomIds: string[]): ((roomId: string) => any) => {
|
||||
const viewModels = new Map();
|
||||
export const createGetRoomItemViewModel = (roomIds: string[]): ((roomId: string) => RoomItemViewModel) => {
|
||||
const viewModels = new Map<string, RoomItemViewModel>();
|
||||
roomIds.forEach((roomId, index) => {
|
||||
const name = roomNames[index % roomNames.length];
|
||||
const snapshot = createMockRoomSnapshot(roomId, name, index);
|
||||
@@ -125,7 +126,7 @@ export const createGetRoomItemViewModel = (roomIds: string[]): ((roomId: string)
|
||||
viewModels.set(roomId, mockViewModel);
|
||||
});
|
||||
|
||||
return (roomId: string) => viewModels.get(roomId);
|
||||
return (roomId: string) => viewModels.get(roomId)!;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -100,7 +100,7 @@ export type RoomStatusBarViewSnapshot =
|
||||
/**
|
||||
* The view model for RoomStatusBarView.
|
||||
*/
|
||||
export type RoomStatusBarViewModel = ViewModel<RoomStatusBarViewSnapshot> & RoomStatusBarViewActions;
|
||||
export type RoomStatusBarViewModel = ViewModel<RoomStatusBarViewSnapshot, RoomStatusBarViewActions>;
|
||||
|
||||
interface RoomStatusBarViewProps {
|
||||
/**
|
||||
|
||||
@@ -55,7 +55,7 @@ export function Box({
|
||||
...props
|
||||
}: React.PropsWithChildren<BoxProps>): JSX.Element {
|
||||
const style = useMemo(() => {
|
||||
const style: Record<string, any> = {};
|
||||
const style: Record<string, string> = {};
|
||||
if (flex) style["--mx-box-flex"] = flex;
|
||||
if (shrink) style["--mx-box-shrink"] = shrink;
|
||||
if (grow) style["--mx-box-grow"] = grow;
|
||||
|
||||
@@ -11,6 +11,7 @@ import React, { type JSX, type ComponentProps, type JSXElementConstructor, useMe
|
||||
|
||||
import styles from "./Flex.module.css";
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
type FlexProps<T extends keyof JSX.IntrinsicElements | JSXElementConstructor<any>> = {
|
||||
/**
|
||||
* The type of the HTML element
|
||||
@@ -60,6 +61,7 @@ type FlexProps<T extends keyof JSX.IntrinsicElements | JSXElementConstructor<any
|
||||
/**
|
||||
* A flexbox container helper
|
||||
*/
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
export function Flex<T extends keyof JSX.IntrinsicElements | JSXElementConstructor<any> = "div">({
|
||||
as = "div",
|
||||
display = "flex",
|
||||
|
||||
@@ -21,7 +21,7 @@ export class I18nApi implements II18nApi {
|
||||
/**
|
||||
* Register translations for the module, may override app's existing translations
|
||||
*/
|
||||
public register(translations: Partial<Translations>): void {
|
||||
public register(this: void, translations: Partial<Translations>): void {
|
||||
const langs: Record<string, Record<string, string>> = {};
|
||||
|
||||
for (const key in translations) {
|
||||
@@ -42,11 +42,9 @@ export class I18nApi implements II18nApi {
|
||||
* @param key - The key to translate
|
||||
* @param variables - Optional variables to interpolate into the translation
|
||||
*/
|
||||
public translate(key: TranslationKey, variables?: Variables): string {
|
||||
public translate(this: void, key: TranslationKey, variables?: Variables): string {
|
||||
return _t(key, variables);
|
||||
}
|
||||
|
||||
public humanizeTime(timeMillis: number): string {
|
||||
return humanizeTime(timeMillis, this);
|
||||
}
|
||||
public humanizeTime = (timeMillis: number): string => humanizeTime(timeMillis, this);
|
||||
}
|
||||
|
||||
@@ -113,7 +113,7 @@ export interface IVirtualizedListProps<Item, Context> extends Omit<
|
||||
/**
|
||||
* Utility type for the prop scrollIntoViewOnChange allowing it to be memoised by a caller without repeating types
|
||||
*/
|
||||
export type ScrollIntoViewOnChange<Item, Context = any> = NonNullable<
|
||||
export type ScrollIntoViewOnChange<Item, Context> = NonNullable<
|
||||
VirtuosoProps<Item, VirtualizedListContext<Context>>["scrollIntoViewOnChange"]
|
||||
>;
|
||||
|
||||
@@ -124,7 +124,7 @@ export type ScrollIntoViewOnChange<Item, Context = any> = NonNullable<
|
||||
* @template Item - The type of data items in the list
|
||||
* @template Context - The type of additional context data passed to items
|
||||
*/
|
||||
export function VirtualizedList<Item, Context = any>(props: IVirtualizedListProps<Item, Context>): React.ReactElement {
|
||||
export function VirtualizedList<Item, Context>(props: IVirtualizedListProps<Item, Context>): React.ReactElement {
|
||||
// Extract our custom props to avoid conflicts with Virtuoso props
|
||||
const {
|
||||
items,
|
||||
|
||||
@@ -9,15 +9,22 @@ Please see LICENSE files in the repository root for full details.
|
||||
* The interface for a generic View Model passed to the shared components.
|
||||
* The snapshot is of type T which is a type specifying a snapshot for the view in question.
|
||||
*/
|
||||
export interface ViewModel<T> {
|
||||
|
||||
// Utility type to map all VM actions to unbound functions so that they do not have
|
||||
// to be called with the correct 'this' context. This prevents "cannot read X of undefined" bugs.
|
||||
type MapToVoidThis<T> = {
|
||||
[K in keyof T]: T[K] extends (...args: infer A) => infer R ? (this: void, ...args: A) => R : T[K];
|
||||
};
|
||||
|
||||
export type ViewModel<Snapshot, Actions = unknown> = {
|
||||
/**
|
||||
* The current snapshot of the view model.
|
||||
*/
|
||||
getSnapshot: () => T;
|
||||
getSnapshot: () => Snapshot;
|
||||
|
||||
/**
|
||||
* Subscribes to changes in the view model.
|
||||
* The listener will be called whenever the snapshot changes.
|
||||
*/
|
||||
subscribe: (listener: () => void) => () => void;
|
||||
}
|
||||
} & MapToVoidThis<Actions>;
|
||||
|
||||
@@ -14,7 +14,7 @@ import { type ViewModel } from "./ViewModel";
|
||||
* @param vm The view model to use
|
||||
* @returns The current snapshot
|
||||
*/
|
||||
export function useViewModel<T>(vm: ViewModel<T>): T {
|
||||
export function useViewModel<T>(vm: ViewModel<T, unknown>): T {
|
||||
// We need to pass the same getSnapshot function as getServerSnapshot as this
|
||||
// is used when making the HTML chat export.
|
||||
return useSyncExternalStore(vm.subscribe, vm.getSnapshot, vm.getSnapshot);
|
||||
|
||||
Reference in New Issue
Block a user