Fix circular dependency issue

Author: MidhunSureshR

src/modules/BuiltinsApi.tsx

@@ -8,27 +8,39 @@ Please see LICENSE files in the repository root for full details.
 import React from "react";
 import { type RoomViewProps, type BuiltinsApi } from "@element-hq/element-web-module-api";
 
-import RoomAvatar from "../components/views/avatars/RoomAvatar";
 import { MatrixClientPeg } from "../MatrixClientPeg";
+import type { Room } from "matrix-js-sdk/src/matrix";
 
 interface RoomViewPropsWithRoomId extends RoomViewProps {
     roomId: string;
 }
 
+interface RoomAvatarProps {
+    room: Room;
+    size?: string;
+}
+
+interface Components {
+    roomView: React.ComponentType<RoomViewPropsWithRoomId>;
+    roomAvatar: React.ComponentType<RoomAvatarProps>;
+}
+
 export class ElementWebBuiltinsApi implements BuiltinsApi {
     private _roomView?: React.ComponentType<RoomViewPropsWithRoomId>;
+    private _roomAvatar?: React.ComponentType<RoomAvatarProps>;
 
     /**
-     * Sets the components used to render a RoomView
+     * Sets the components used by the API.
      *
-     * This only really exists here because referencing RoomView directly causes a nightmare of
+     * This only really exists here because referencing these components directly causes a nightmare of
      * circular dependencies that break the whole app, so instead we avoid referencing it here
      * and pass it in from somewhere it's already referenced (see related comment in app.tsx).
      *
      * @param component The RoomView component
      */
-    public setRoomViewComponent(component: React.ComponentType<RoomViewPropsWithRoomId>): void {
-        this._roomView = component;
+    public setComponents(components: Components): void {
+        this._roomView = components.roomView;
+        this._roomAvatar = components.roomAvatar;
     }
 
     public getRoomViewComponent(): React.ComponentType<RoomViewPropsWithRoomId> {
@@ -39,6 +51,14 @@ export class ElementWebBuiltinsApi implements BuiltinsApi {
         return this._roomView;
     }
 
+    public getRoomAvatarComponent(): React.ComponentType<RoomAvatarProps> {
+        if (!this._roomAvatar) {
+            throw new Error("No RoomAvatar component has been set");
+        }
+
+        return this._roomAvatar;
+    }
+
     public renderRoomView(roomId: string): React.ReactNode {
         const Component = this.getRoomViewComponent();
         return <Component roomId={roomId} />;
@@ -49,6 +69,7 @@ export class ElementWebBuiltinsApi implements BuiltinsApi {
         if (!room) {
             throw new Error(`No room such room: ${roomId}`);
         }
-        return <RoomAvatar room={room} size={size} />;
+        const Component = this.getRoomAvatarComponent();
+        return <Component room={room} size={size} />;
     }
 }

src/vector/app.tsx

@@ -32,6 +32,7 @@ import { getInitialScreenAfterLogin, getScreenFromLocation, init as initRouting,
 import { UserFriendlyError } from "../languageHandler";
 import { ModuleApi } from "../modules/Api";
 import { RoomView } from "../components/structures/RoomView";
+import RoomAvatar from "../components/views/avatars/RoomAvatar";
 
 logger.log(`Application is running in ${process.env.NODE_ENV} mode`);
 
@@ -55,10 +56,9 @@ function onTokenLoginCompleted(): void {
 }
 
 export async function loadApp(fragParams: QueryDict, matrixChatRef: React.Ref<MatrixChat>): Promise<ReactElement> {
-    // XXX: This lives here because RoomVew import so many things that importing it in a sensible place (eg.
-    // the builtins module or init.tsx) causes a circular dependency. Instead, we reference RoomView here where we
-    // already reference it indirectly via MatrixChat.
-    ModuleApi.instance.builtins.setRoomViewComponent(RoomView);
+    // XXX: This lives here because certain components import so many things that importing it in a sensible place (eg.
+    // the builtins module or init.tsx) causes a circular dependency.
+    ModuleApi.instance.builtins.setComponents({ roomView: RoomView, roomAvatar: RoomAvatar });
 
     initRouting();
     const platform = PlatformPeg.get();

test/unit-tests/modules/BuiltinsApi-test.tsx

@@ -10,39 +10,34 @@ import { render } from "jest-matrix-react";
 import { ElementWebBuiltinsApi } from "../../../src/modules/BuiltinsApi";
 import { stubClient } from "../../test-utils/test-utils";
 
-jest.mock("../../../src/components/views/avatars/RoomAvatar", () => {
-    const Avatar: React.FC<{ room: { roomId: string }; size: string }> = ({ room, size }) => {
-        return (
-            <div>
-                Avatar, {room.roomId}, {size}
-            </div>
-        );
-    };
-    return {
-        __esModule: true,
-        default: Avatar,
-    };
-});
+const Avatar: React.FC<{ room: { roomId: string }; size: string }> = ({ room, size }) => {
+    return (
+        <div>
+            Avatar, {room.roomId}, {size}
+        </div>
+    );
+};
 
 describe("ElementWebBuiltinsApi", () => {
     it("returns the RoomView component thats been set", () => {
         const builtinsApi = new ElementWebBuiltinsApi();
         const sentinel = {};
-        builtinsApi.setRoomViewComponent(sentinel as any);
+        builtinsApi.setComponents({ roomView: sentinel, roomAvatar: Avatar } as any);
         expect(builtinsApi.getRoomViewComponent()).toBe(sentinel);
     });
 
     it("returns rendered RoomView component", () => {
         const builtinsApi = new ElementWebBuiltinsApi();
         const RoomView = () => <div>hello world</div>;
-        builtinsApi.setRoomViewComponent(RoomView as any);
+        builtinsApi.setComponents({ roomView: RoomView, roomAvatar: Avatar } as any);
         const { container } = render(<> {builtinsApi.renderRoomView("!foo:m.org")}</>);
         expect(container).toHaveTextContent("hello world");
     });
 
     it("returns rendered RoomAvatar component", () => {
         stubClient();
         const builtinsApi = new ElementWebBuiltinsApi();
+        builtinsApi.setComponents({ roomView: {}, roomAvatar: Avatar } as any);
         const { container } = render(<> {builtinsApi.renderRoomAvatar("!foo:m.org", "50")}</>);
         expect(container).toHaveTextContent("Avatar");
         expect(container).toHaveTextContent("!foo:m.org");