Apply some code quality suggestions from SonarQube (#144)

* Apply some code quality suggestions from SonarQube

* Prefer globalThis over window

https://sonarcloud.io/organizations/matrix-org/rules?open=typescript%3AS7764&rule_key=typescript%3AS7764

* De-negate delayed event conditions

* Apply prefer-readonly lint rule

* Tag MSC-related identifiers as `@experimental`

* Optimize expression

- Use optional chaining
- Don't call toString on a string

* Apply await-thenable lint rule

* Apply no-duplicate-imports lint rule

* Remove useless assignment

* Migrate from deprecated Jest methods

* Fix tested delay parent ID arguments

* Improve code coverage

* Set `noUnusedLocals` TSConfig option

* Remove overwritten assignment

* Add code coverage for requests over postMessage

* De-negate response/request check

Author: AndrewFerr

.eslintrc.js

@@ -8,6 +8,7 @@ module.exports = {
         browser: true,
     },
     rules: {
+        "no-duplicate-imports": ["error"],
         "no-var": ["warn"],
         "prefer-rest-params": ["warn"],
         "prefer-spread": ["warn"],
@@ -24,6 +25,8 @@ module.exports = {
                 asyncArrow: "always",
             },
         ],
+        "@typescript-eslint/await-thenable": ["error"],
+        "@typescript-eslint/prefer-readonly": ["error"],
         "arrow-parens": "off",
         "prefer-promise-reject-errors": "off",
         "quotes": "off",

examples/widget/utils.js

@@ -15,7 +15,7 @@
  */
 
 function parseFragment() {
-    const fragmentString = window.location.hash || "?";
+    const fragmentString = globalThis.location.hash || "?";
     return new URLSearchParams(fragmentString.substring(Math.max(fragmentString.indexOf("?"), 0)));
 }
 

src/ClientWidgetApi.ts

@@ -62,7 +62,6 @@ import {
 } from "./interfaces/SendToDeviceAction";
 import { EventDirection, EventKind, WidgetEventCapability } from "./models/WidgetEventCapability";
 import { IRoomEvent } from "./interfaces/IRoomEvent";
-import { IRoomAccountData } from "./interfaces/IRoomAccountData";
 import {
     IGetOpenIDActionRequest,
     IGetOpenIDActionResponseData,
@@ -141,15 +140,15 @@ export class ClientWidgetApi extends EventEmitter {
     private cachedWidgetVersions: ApiVersion[] | null = null;
     // contentLoadedActionSent is used to check that only one ContentLoaded request is send.
     private contentLoadedActionSent = false;
-    private allowedCapabilities = new Set<Capability>();
-    private allowedEvents: WidgetEventCapability[] = [];
+    private readonly allowedCapabilities = new Set<Capability>();
+    private readonly allowedEvents: WidgetEventCapability[] = [];
     private isStopped = false;
     private turnServers: AsyncGenerator<ITurnServer> | null = null;
     private contentLoadedWaitTimer?: ReturnType<typeof setTimeout>;
     // Stores pending requests to push a room's state to the widget
-    private pushRoomStateTasks = new Set<Promise<void>>();
+    private readonly pushRoomStateTasks = new Set<Promise<void>>();
     // Room ID → event type → state key → events to be pushed
-    private pushRoomStateResult = new Map<string, Map<string, Map<string, IRoomEvent>>>();
+    private readonly pushRoomStateResult = new Map<string, Map<string, Map<string, IRoomEvent>>>();
     private flushRoomStateTask: Promise<void> | null = null;
 
     /**
@@ -162,8 +161,8 @@ export class ClientWidgetApi extends EventEmitter {
      */
     public constructor(
         public readonly widget: Widget,
-        private iframe: HTMLIFrameElement,
-        private driver: WidgetDriver,
+        iframe: HTMLIFrameElement,
+        private readonly driver: WidgetDriver,
     ) {
         super();
         if (!iframe?.contentWindow) {
@@ -175,7 +174,12 @@ export class ClientWidgetApi extends EventEmitter {
         if (!driver) {
             throw new Error("Invalid driver");
         }
-        this.transport = new PostmessageTransport(WidgetApiDirection.ToWidget, widget.id, iframe.contentWindow, window);
+        this.transport = new PostmessageTransport(
+            WidgetApiDirection.ToWidget,
+            widget.id,
+            iframe.contentWindow,
+            globalThis,
+        );
         this.transport.targetOrigin = widget.origin;
         this.transport.on("message", this.handleMessage.bind(this));
 
@@ -230,7 +234,7 @@ export class ClientWidgetApi extends EventEmitter {
 
     public async getWidgetVersions(): Promise<ApiVersion[]> {
         if (Array.isArray(this.cachedWidgetVersions)) {
-            return Promise.resolve(this.cachedWidgetVersions);
+            return this.cachedWidgetVersions;
         }
 
         try {
@@ -381,7 +385,7 @@ export class ClientWidgetApi extends EventEmitter {
             });
         }
 
-        if (!request.data?.uri || !request.data?.uri.toString().startsWith("https://matrix.to/#")) {
+        if (!request.data?.uri.startsWith("https://matrix.to/#")) {
             return this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Invalid matrix.to URI" },
             });
@@ -467,9 +471,9 @@ export class ClientWidgetApi extends EventEmitter {
 
         this.driver.askOpenID(observer);
     }
+
     private handleReadRoomAccountData(request: IReadRoomAccountDataFromWidgetActionRequest): void | Promise<void> {
-        let events: Promise<IRoomAccountData[]> = Promise.resolve([]);
-        events = this.driver.readRoomAccountData(request.data.type);
+        const events = this.driver.readRoomAccountData(request.data.type);
 
         if (!this.canReceiveRoomAccountData(request.data.type)) {
             return this.transport.reply<IWidgetApiErrorResponseData>(request, {
@@ -609,17 +613,17 @@ export class ClientWidgetApi extends EventEmitter {
                 });
             }
 
-            if (!isDelayedEvent) {
-                sendEventPromise = this.driver.sendEvent(
+            if (isDelayedEvent) {
+                sendEventPromise = this.driver.sendDelayedEvent(
+                    request.data.delay ?? null,
+                    request.data.parent_delay_id ?? null,
                     request.data.type,
                     request.data.content || {},
                     request.data.state_key,
                     request.data.room_id,
                 );
             } else {
-                sendEventPromise = this.driver.sendDelayedEvent(
-                    request.data.delay ?? null,
-                    request.data.parent_delay_id ?? null,
+                sendEventPromise = this.driver.sendEvent(
                     request.data.type,
                     request.data.content || {},
                     request.data.state_key,
@@ -635,17 +639,17 @@ export class ClientWidgetApi extends EventEmitter {
                 });
             }
 
-            if (!isDelayedEvent) {
-                sendEventPromise = this.driver.sendEvent(
+            if (isDelayedEvent) {
+                sendEventPromise = this.driver.sendDelayedEvent(
+                    request.data.delay ?? null,
+                    request.data.parent_delay_id ?? null,
                     request.data.type,
                     content,
                     null, // not sending a state event
                     request.data.room_id,
                 );
             } else {
-                sendEventPromise = this.driver.sendDelayedEvent(
-                    request.data.delay ?? null,
-                    request.data.parent_delay_id ?? null,
+                sendEventPromise = this.driver.sendEvent(
                     request.data.type,
                     content,
                     null, // not sending a state event
@@ -715,25 +719,25 @@ export class ClientWidgetApi extends EventEmitter {
 
     private async handleSendToDevice(request: ISendToDeviceFromWidgetActionRequest): Promise<void> {
         if (!request.data.type) {
-            await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+            this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Invalid request - missing event type" },
             });
         } else if (!request.data.messages) {
-            await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+            this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Invalid request - missing event contents" },
             });
         } else if (typeof request.data.encrypted !== "boolean") {
-            await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+            this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Invalid request - missing encryption flag" },
             });
         } else if (!this.canSendToDeviceEvent(request.data.type)) {
-            await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+            this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Cannot send to-device events of this type" },
             });
         } else {
             try {
                 await this.driver.sendToDevice(request.data.type, request.data.encrypted, request.data.messages);
-                await this.transport.reply<ISendToDeviceFromWidgetResponseData>(request, {});
+                this.transport.reply<ISendToDeviceFromWidgetResponseData>(request, {});
             } catch (e) {
                 console.error("error sending to-device event", e);
                 this.handleDriverError(e, request, "Error sending event");
@@ -762,12 +766,12 @@ export class ClientWidgetApi extends EventEmitter {
 
     private async handleWatchTurnServers(request: IWatchTurnServersRequest): Promise<void> {
         if (!this.hasCapability(MatrixCapabilities.MSC3846TurnServers)) {
-            await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+            this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Missing capability" },
             });
         } else if (this.turnServers) {
             // We're already polling, so this is a no-op
-            await this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
+            this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
         } else {
             try {
                 const turnServers = this.driver.getTurnServers();
@@ -776,14 +780,14 @@ export class ClientWidgetApi extends EventEmitter {
                 // client isn't banned from getting TURN servers entirely
                 const { done, value } = await turnServers.next();
                 if (done) throw new Error("Client refuses to provide any TURN servers");
-                await this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
+                this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
 
                 // Start the poll loop, sending the widget the initial result
                 this.pollTurnServers(turnServers, value);
                 this.turnServers = turnServers;
             } catch (e) {
                 console.error("error getting first TURN server results", e);
-                await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+                this.transport.reply<IWidgetApiErrorResponseData>(request, {
                     error: { message: "TURN servers not available" },
                 });
             }
@@ -792,17 +796,17 @@ export class ClientWidgetApi extends EventEmitter {
 
     private async handleUnwatchTurnServers(request: IUnwatchTurnServersRequest): Promise<void> {
         if (!this.hasCapability(MatrixCapabilities.MSC3846TurnServers)) {
-            await this.transport.reply<IWidgetApiErrorResponseData>(request, {
+            this.transport.reply<IWidgetApiErrorResponseData>(request, {
                 error: { message: "Missing capability" },
             });
         } else if (!this.turnServers) {
             // We weren't polling anyways, so this is a no-op
-            await this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
+            this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
         } else {
             // Stop the generator, allowing it to clean up
             await this.turnServers.return(undefined);
             this.turnServers = null;
-            await this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
+            this.transport.reply<IWidgetApiAcknowledgeResponseData>(request, {});
         }
     }
 
@@ -1147,7 +1151,7 @@ export class ClientWidgetApi extends EventEmitter {
     private async flushRoomState(): Promise<void> {
         try {
             // Only send a single action once all concurrent tasks have completed
-            do await Promise.all([...this.pushRoomStateTasks]);
+            do await Promise.all(this.pushRoomStateTasks);
             while (this.pushRoomStateTasks.size > 0);
 
             const events: IRoomEvent[] = [];
@@ -1257,7 +1261,7 @@ export class ClientWidgetApi extends EventEmitter {
                     eventTypeMap.set(rawEvent.type, stateKeyMap);
                 }
                 if (!stateKeyMap.has(rawEvent.type)) stateKeyMap.set(rawEvent.state_key, rawEvent);
-                do await Promise.all([...this.pushRoomStateTasks]);
+                do await Promise.all(this.pushRoomStateTasks);
                 while (this.pushRoomStateTasks.size > 0);
                 await this.flushRoomStateTask;
             }

src/WidgetApi.ts

@@ -131,7 +131,7 @@ export class WidgetApi extends EventEmitter {
 
     private capabilitiesFinished = false;
     private supportsMSC2974Renegotiate = false;
-    private requestedCapabilities: Capability[] = [];
+    private readonly requestedCapabilities: Capability[] = [];
     private approvedCapabilities?: Capability[];
     private cachedClientVersions?: ApiVersion[];
     private turnServerWatchers = 0;
@@ -142,15 +142,17 @@ export class WidgetApi extends EventEmitter {
      * the API will use the widget ID from the first valid request it receives.
      * @param {string} clientOrigin The origin of the client, or null if not known.
      */
-    public constructor(
-        widgetId: string | null = null,
-        private clientOrigin: string | null = null,
-    ) {
+    public constructor(widgetId: string | null = null, clientOrigin: string | null = null) {
         super();
-        if (!window.parent) {
+        if (!globalThis.parent) {
             throw new Error("No parent window. This widget doesn't appear to be embedded properly.");
         }
-        this.transport = new PostmessageTransport(WidgetApiDirection.FromWidget, widgetId, window.parent, window);
+        this.transport = new PostmessageTransport(
+            WidgetApiDirection.FromWidget,
+            widgetId,
+            globalThis.parent,
+            globalThis,
+        );
         this.transport.targetOrigin = clientOrigin;
         this.transport.on("message", this.handleMessage.bind(this));
     }
@@ -191,7 +193,9 @@ export class WidgetApi extends EventEmitter {
      * @throws Throws if the capabilities negotiation has already started.
      */
     public requestCapabilities(capabilities: Capability[]): void {
-        capabilities.forEach((cap) => this.requestCapability(cap));
+        for (const cap of capabilities) {
+            this.requestCapability(cap);
+        }
     }
 
     /**
@@ -474,7 +478,7 @@ export class WidgetApi extends EventEmitter {
     }
 
     /**
-     * @deprecated This currently relies on an unstable MSC (MSC4157).
+     * @experimental This currently relies on an unstable MSC (MSC4157).
      */
     public cancelScheduledDelayedEvent(delayId: string): Promise<IUpdateDelayedEventFromWidgetResponseData> {
         return this.transport.send<IUpdateDelayedEventFromWidgetRequestData, IUpdateDelayedEventFromWidgetResponseData>(
@@ -487,7 +491,7 @@ export class WidgetApi extends EventEmitter {
     }
 
     /**
-     * @deprecated This currently relies on an unstable MSC (MSC4157).
+     * @experimental This currently relies on an unstable MSC (MSC4157).
      */
     public restartScheduledDelayedEvent(delayId: string): Promise<IUpdateDelayedEventFromWidgetResponseData> {
         return this.transport.send<IUpdateDelayedEventFromWidgetRequestData, IUpdateDelayedEventFromWidgetResponseData>(
@@ -500,7 +504,7 @@ export class WidgetApi extends EventEmitter {
     }
 
     /**
-     * @deprecated This currently relies on an unstable MSC (MSC4157).
+     * @experimental This currently relies on an unstable MSC (MSC4157).
      */
     public sendScheduledDelayedEvent(delayId: string): Promise<IUpdateDelayedEventFromWidgetResponseData> {
         return this.transport.send<IUpdateDelayedEventFromWidgetRequestData, IUpdateDelayedEventFromWidgetResponseData>(
@@ -681,7 +685,7 @@ export class WidgetApi extends EventEmitter {
      * @param {string} uri The URI to navigate to.
      * @returns {Promise<void>} Resolves when complete.
      * @throws Throws if the URI is invalid or cannot be processed.
-     * @deprecated This currently relies on an unstable MSC (MSC2931).
+     * @experimental This currently relies on an unstable MSC (MSC2931).
      */
     public navigateTo(uri: string): Promise<void> {
         if (!uri || !uri.startsWith("https://matrix.to/#")) {
@@ -704,7 +708,7 @@ export class WidgetApi extends EventEmitter {
         const onUpdateTurnServers = async (ev: CustomEvent<IUpdateTurnServersRequest>): Promise<void> => {
             ev.preventDefault();
             setTurnServer(ev.detail.data);
-            await this.transport.reply<IWidgetApiAcknowledgeResponseData>(ev.detail, {});
+            this.transport.reply<IWidgetApiAcknowledgeResponseData>(ev.detail, {});
         };
 
         // Start listening for updates before we even start watching, to catch

src/interfaces/Capabilities.ts

@@ -26,28 +26,31 @@ export enum MatrixCapabilities {
      */
     RequiresClient = "io.element.requires_client",
     /**
-     * @deprecated It is not recommended to rely on this existing - it can be removed without notice.
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
      */
     MSC2931Navigate = "org.matrix.msc2931.navigate",
+    /**
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
+     */
     MSC3846TurnServers = "town.robin.msc3846.turn_servers",
     /**
-     * @deprecated It is not recommended to rely on this existing - it can be removed without notice.
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
      */
     MSC3973UserDirectorySearch = "org.matrix.msc3973.user_directory_search",
     /**
-     * @deprecated It is not recommended to rely on this existing - it can be removed without notice.
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
      */
     MSC4039UploadFile = "org.matrix.msc4039.upload_file",
     /**
-     * @deprecated It is not recommended to rely on this existing - it can be removed without notice.
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
      */
     MSC4039DownloadFile = "org.matrix.msc4039.download_file",
     /**
-     * @deprecated It is not recommended to rely on this existing - it can be removed without notice.
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
      */
     MSC4157SendDelayedEvent = "org.matrix.msc4157.send.delayed_event",
     /**
-     * @deprecated It is not recommended to rely on this existing - it can be removed without notice.
+     * @experimental It is not recommended to rely on this existing - it can be removed without notice.
      */
     MSC4157UpdateDelayedEvent = "org.matrix.msc4157.update_delayed_event",
 }