MM-67137 Fix references to window in client package (#35195)

* MM-67137 Fix references to window in client package

* Fix Client tests running on compiled code

* Mostly revert changes to limit the chance of accidental changes

webapp/platform/client/.eslintrc.json

@@ -2,5 +2,21 @@
   "root": true,
   "extends": [
     "plugin:@mattermost/base"
+  ],
+  "rules": {
+
+    // window isn't defined when using this package in a Node.js app, so prevent people from using it by default
+    "no-restricted-globals": [
+      "error",
+      "window"
+    ]
+  },
+  "overrides": [
+    {
+      "files": "*.test.*",
+      "rules": {
+        "no-restricted-globals": ["off"]
+      }
+    }
   ]
 }

webapp/platform/client/jest.config.js

@@ -7,6 +7,7 @@ module.exports = {
     moduleNameMapper: {
         '^@mattermost/types/(.*)$': '<rootDir>/../types/src/$1',
     },
+    testPathIgnorePatterns: ['/node_modules/', '/lib/'],
     setupFiles: ['<rootDir>/setup_jest.ts'],
     collectCoverageFrom: [
         'src/**/*.{js,jsx,ts,tsx}',

webapp/platform/client/src/websocket.test.ts

@@ -10,34 +10,6 @@ if (typeof WebSocket === 'undefined') {
     };
 }
 
-// Mock window and navigator if they're not defined
-if (typeof window === 'undefined') {
-    const eventHandlers: {[key: string]: Array<(event: Event) => void>} = {};
-
-    // Create a mock window object with working event handlers
-    (global as any).window = {
-        addEventListener: jest.fn((event: string, handler: (event: Event) => void) => {
-            if (!eventHandlers[event]) {
-                eventHandlers[event] = [];
-            }
-            eventHandlers[event].push(handler);
-        }),
-        removeEventListener: jest.fn((event: string, handler: (event: Event) => void) => {
-            if (eventHandlers[event]) {
-                const index = eventHandlers[event].indexOf(handler);
-                if (index !== -1) {
-                    eventHandlers[event].splice(index, 1);
-                }
-            }
-        }),
-        dispatchEvent: jest.fn((event: Event) => {
-            const handlers = eventHandlers[event.type] || [];
-            handlers.forEach((handler) => handler(event));
-            return true;
-        }),
-    };
-}
-
 // Mock Event class if it's not defined
 if (typeof Event === 'undefined') {
     (global as any).Event = class MockEvent {
@@ -476,159 +448,217 @@ describe('websocketclient', () => {
         jest.useRealTimers();
     });
 
-    test('should add network event listener on initialize', () => {
-        // Mock window.addEventListener
-        const originalAddEventListener = window.addEventListener;
-        const originalRemoveEventListener = window.removeEventListener;
+    describe('online/offline', () => {
+        const originalWindow = globalThis.window;
 
-        const addEventListenerMock = jest.fn();
-        const removeEventListenerMock = jest.fn();
+        // Mock window and navigator if they're not defined
+        beforeAll(() => {
+            if (typeof window === 'undefined') {
+                const eventHandlers: {[key: string]: Array<(event: Event) => void>} = {};
 
-        window.addEventListener = addEventListenerMock;
-        window.removeEventListener = removeEventListenerMock;
+                // Create a mock window object with working event handlers
+                (globalThis as any).window = {
+                    addEventListener: jest.fn((event: string, handler: (event: Event) => void) => {
+                        if (!eventHandlers[event]) {
+                            eventHandlers[event] = [];
+                        }
+                        eventHandlers[event].push(handler);
+                    }),
+                    removeEventListener: jest.fn((event: string, handler: (event: Event) => void) => {
+                        if (eventHandlers[event]) {
+                            const index = eventHandlers[event].indexOf(handler);
+                            if (index !== -1) {
+                                eventHandlers[event].splice(index, 1);
+                            }
+                        }
+                    }),
+                    dispatchEvent: jest.fn((event: Event) => {
+                        const handlers = eventHandlers[event.type] || [];
+                        handlers.forEach((handler) => handler(event));
+                        return true;
+                    }),
+                };
+            }
+        });
 
-        // Create client
-        const mockWebSocket = new MockWebSocket();
-        const client = new WebSocketClient({
-            newWebSocketFn: (url: string) => {
+        afterAll(() => {
+            globalThis.window = originalWindow;
+        });
+
+        test('should add network event listener on initialize', () => {
+            // Mock window.addEventListener
+            const originalAddEventListener = window.addEventListener;
+            const originalRemoveEventListener = window.removeEventListener;
+
+            const addEventListenerMock = jest.fn();
+            const removeEventListenerMock = jest.fn();
+
+            window.addEventListener = addEventListenerMock;
+            window.removeEventListener = removeEventListenerMock;
+
+            // Create client
+            const mockWebSocket = new MockWebSocket();
+            const client = new WebSocketClient({
+                newWebSocketFn: (url: string) => {
+                    mockWebSocket.url = url;
+                    return mockWebSocket;
+                },
+            });
+
+            // No listeners should be added on construction
+            expect(addEventListenerMock).not.toHaveBeenCalled();
+
+            // Initialize should add listeners
+            client.initialize('mock.url');
+
+            // Verify event listeners are added on initialize
+            expect(addEventListenerMock).toHaveBeenCalledWith('online', expect.any(Function));
+
+            // Clean up
+            client.close();
+
+            // Verify event listeners are removed
+            expect(removeEventListenerMock).toHaveBeenCalledWith('online', expect.any(Function));
+
+            // Restore mocks
+            window.addEventListener = originalAddEventListener;
+            window.removeEventListener = originalRemoveEventListener;
+        });
+
+        test('should reconnect when network comes online', () => {
+            jest.useFakeTimers();
+
+            var connected = true;
+            const mockWebSocket = new MockWebSocket();
+            const newWebSocketFn = jest.fn((url: string) => {
                 mockWebSocket.url = url;
-                return mockWebSocket;
-            },
-        });
 
-        // No listeners should be added on construction
-        expect(addEventListenerMock).not.toHaveBeenCalled();
-
-        // Initialize should add listeners
-        client.initialize('mock.url');
-
-        // Verify event listeners are added on initialize
-        expect(addEventListenerMock).toHaveBeenCalledWith('online', expect.any(Function));
-
-        // Clean up
-        client.close();
-
-        // Verify event listeners are removed
-        expect(removeEventListenerMock).toHaveBeenCalledWith('online', expect.any(Function));
-
-        // Restore mocks
-        window.addEventListener = originalAddEventListener;
-        window.removeEventListener = originalRemoveEventListener;
-    });
-
-    test('should reconnect when network comes online', () => {
-        jest.useFakeTimers();
-
-        var connected = true;
-        const mockWebSocket = new MockWebSocket();
-        const newWebSocketFn = jest.fn((url: string) => {
-            mockWebSocket.url = url;
-
-            // selectively simulate the network being down
-            setTimeout(() => {
-                if (!connected && mockWebSocket.onclose) {
-                    mockWebSocket.close();
-                }
-            }, 1);
-
-            return mockWebSocket;
-        });
-
-        // Use a small minWebSocketRetryTime to speed up the test
-        const client = new WebSocketClient({
-            newWebSocketFn,
-            minWebSocketRetryTime: 100,
-            maxWebSocketRetryTime: 1000,
-        });
-
-        // Initialize the client
-        client.initialize('mock.url');
-        mockWebSocket.open();
-        expect(newWebSocketFn).toHaveBeenCalledTimes(1);
-
-        // Simulate network going offline
-        mockWebSocket.close();
-        connected = false;
-
-        // Connection should be closed
-        expect(mockWebSocket.readyState).toBe(WebSocket.CLOSED);
-
-        // Wait a very long time to max out retry timeout
-        jest.advanceTimersByTime(10000);
-
-        // Reset the mock to track the next retry
-        // which should be quicker than the max
-        newWebSocketFn.mockClear();
-
-        // Simulate network coming back online
-        connected = true;
-        const onlineEvent = new Event('online');
-        window.dispatchEvent(onlineEvent);
-
-        // Should not reconnect immediately (should wait for the timeout)
-        expect(newWebSocketFn).not.toHaveBeenCalled();
-
-        // Advance timers to trigger the reconnect
-        jest.advanceTimersByTime(110);
-
-        // Should have reconnected after the timeout
-        expect(newWebSocketFn).toHaveBeenCalledTimes(1);
-
-        // Clean up
-        client.close();
-
-        // Reset timers
-        jest.useRealTimers();
-    });
-
-    test('should send ping when network goes offline', () => {
-        jest.useFakeTimers();
-
-        const mockWebSocket = new MockWebSocket();
-        const client = new WebSocketClient({
-            newWebSocketFn: (url: string) => {
-                mockWebSocket.url = url;
+                // selectively simulate the network being down
                 setTimeout(() => {
-                    if (mockWebSocket.onopen) {
-                        mockWebSocket.open();
+                    if (!connected && mockWebSocket.onclose) {
+                        mockWebSocket.close();
                     }
                 }, 1);
+
                 return mockWebSocket;
-            },
-            clientPingInterval: 300,
+            });
+
+            // Use a small minWebSocketRetryTime to speed up the test
+            const client = new WebSocketClient({
+                newWebSocketFn,
+                minWebSocketRetryTime: 100,
+                maxWebSocketRetryTime: 1000,
+            });
+
+            // Initialize the client
+            client.initialize('mock.url');
+            mockWebSocket.open();
+            expect(newWebSocketFn).toHaveBeenCalledTimes(1);
+
+            // Simulate network going offline
+            mockWebSocket.close();
+            connected = false;
+
+            // Connection should be closed
+            expect(mockWebSocket.readyState).toBe(WebSocket.CLOSED);
+
+            // Wait a very long time to max out retry timeout
+            jest.advanceTimersByTime(10000);
+
+            // Reset the mock to track the next retry
+            // which should be quicker than the max
+            newWebSocketFn.mockClear();
+
+            // Simulate network coming back online
+            connected = true;
+            const onlineEvent = new Event('online');
+            window.dispatchEvent(onlineEvent);
+
+            // Should not reconnect immediately (should wait for the timeout)
+            expect(newWebSocketFn).not.toHaveBeenCalled();
+
+            // Advance timers to trigger the reconnect
+            jest.advanceTimersByTime(110);
+
+            // Should have reconnected after the timeout
+            expect(newWebSocketFn).toHaveBeenCalledTimes(1);
+
+            // Clean up
+            client.close();
+
+            // Reset timers
+            jest.useRealTimers();
         });
 
-        let numPings = 0;
-        mockWebSocket.send = (evt) => {
-            const msg = JSON.parse(evt);
-            if (msg.action !== 'ping') {
-                return;
-            }
-            numPings++;
-        };
+        test('should send ping when network goes offline', () => {
+            jest.useFakeTimers();
 
-        const openSpy = jest.spyOn(mockWebSocket, 'open');
-        const closeSpy = jest.spyOn(mockWebSocket, 'close');
+            const mockWebSocket = new MockWebSocket();
+            const client = new WebSocketClient({
+                newWebSocketFn: (url: string) => {
+                    mockWebSocket.url = url;
+                    setTimeout(() => {
+                        if (mockWebSocket.onopen) {
+                            mockWebSocket.open();
+                        }
+                    }, 1);
+                    return mockWebSocket;
+                },
+                clientPingInterval: 300,
+            });
+
+            let numPings = 0;
+            mockWebSocket.send = (evt) => {
+                const msg = JSON.parse(evt);
+                if (msg.action !== 'ping') {
+                    return;
+                }
+                numPings++;
+            };
+
+            const openSpy = jest.spyOn(mockWebSocket, 'open');
+            const closeSpy = jest.spyOn(mockWebSocket, 'close');
+
+            client.initialize('mock.url');
+            jest.advanceTimersByTime(10);
+
+            expect(mockWebSocket.readyState).toBe(WebSocket.OPEN);
+            expect(openSpy).toHaveBeenCalledTimes(1);
+            expect(closeSpy).toHaveBeenCalledTimes(0);
+            expect(numPings).toBe(1);
+
+            // Simulate network going offline
+            const offlineEvent = new Event('offline');
+            window.dispatchEvent(offlineEvent);
+            jest.advanceTimersByTime(10);
+
+            client.close();
+
+            expect(openSpy).toHaveBeenCalledTimes(1);
+            expect(closeSpy).toHaveBeenCalledTimes(1);
+            expect(numPings).toBe(2);
+
+            jest.useRealTimers();
+        });
+    });
+
+    test('should be able to use WebSocketClient in a non-browser environment', () => {
+        const mockWebSocket = new MockWebSocket();
+
+        const client = new WebSocketClient({
+            newWebSocketFn: (url: string) => {
+                mockWebSocket.url = url;
+                return mockWebSocket;
+            },
+        });
+
+        expect(window).not.toBeDefined();
 
         client.initialize('mock.url');
-        jest.advanceTimersByTime(10);
 
-        expect(mockWebSocket.readyState).toBe(WebSocket.OPEN);
-        expect(openSpy).toHaveBeenCalledTimes(1);
-        expect(closeSpy).toHaveBeenCalledTimes(0);
-        expect(numPings).toBe(1);
-
-        // Simulate network going offline
-        const offlineEvent = new Event('offline');
-        window.dispatchEvent(offlineEvent);
-        jest.advanceTimersByTime(10);
+        expect(mockWebSocket.onopen).toBeTruthy();
+        expect(mockWebSocket.onclose).toBeTruthy();
 
         client.close();
-
-        expect(openSpy).toHaveBeenCalledTimes(1);
-        expect(closeSpy).toHaveBeenCalledTimes(1);
-        expect(numPings).toBe(2);
-
-        jest.useRealTimers();
     });
 });

webapp/platform/client/src/websocket.ts

@@ -150,10 +150,10 @@ export default class WebSocketClient {
         // Setup network event listener
         // Remove existing listeners if any
         if (this.onlineHandler) {
-            window.removeEventListener('online', this.onlineHandler);
+            globalThis.window?.removeEventListener('online', this.onlineHandler);
         }
         if (this.offlineHandler) {
-            window.removeEventListener('offline', this.offlineHandler);
+            globalThis.window?.removeEventListener('offline', this.offlineHandler);
         }
 
         this.onlineHandler = () => {
@@ -199,8 +199,8 @@ export default class WebSocketClient {
             });
         };
 
-        window.addEventListener('online', this.onlineHandler);
-        window.addEventListener('offline', this.offlineHandler);
+        globalThis.window?.addEventListener('online', this.onlineHandler);
+        globalThis.window?.addEventListener('offline', this.offlineHandler);
 
         // Add connection id, and last_sequence_number to the query param.
         // We cannot use a cookie because it will bleed across tabs.
@@ -559,11 +559,11 @@ export default class WebSocketClient {
         }
 
         if (this.onlineHandler) {
-            window.removeEventListener('online', this.onlineHandler);
+            globalThis.window?.removeEventListener('online', this.onlineHandler);
             this.onlineHandler = null;
         }
         if (this.offlineHandler) {
-            window.removeEventListener('offline', this.offlineHandler);
+            globalThis.window?.removeEventListener('offline', this.offlineHandler);
             this.offlineHandler = null;
         }
     }
@@ -656,7 +656,7 @@ export default class WebSocketClient {
     acknowledgePostedNotification(postId: string, status: string, reason?: string, postedData?: string) {
         const data = {
             post_id: postId,
-            user_agent: window.navigator.userAgent,
+            user_agent: globalThis.window?.navigator?.userAgent ?? '',
             status,
             reason,
             data: postedData,