Modernise callbacks + various simplifications

- swap useCallback and various helpers with useEvent
- remove memoisation of some leaf components where it is not helpful
- disable expensive graphical effects when running E2E tests to improve performance
- remove explicit void returns
- remove crypto patch for node 16 (unsupported now)

Author: davidje13

backend/src/index.ts

@@ -12,7 +12,7 @@ import { logError, logInfo } from './log';
 let activeApp: App | null = null;
 const server = createServer();
 
-function startServer(): void {
+function startServer() {
   server.listen(config.port, config.serverBindAddress, () => {
     logInfo(`Available at http://localhost:${config.port}/`);
     logInfo('Press Ctrl+C to stop');

backend/src/routers/ForwardingRouter.ts

@@ -1,5 +1,5 @@
 import { Router } from 'websocket-express';
-import type { RequestHandler } from 'http-proxy-middleware';
+import { type RequestHandler } from 'http-proxy-middleware';
 
 const filteredLogger = {
   info: () => null,

backend/src/routers/StaticRouter.ts

@@ -26,7 +26,7 @@ export class StaticRouter extends Router {
       serveStatic: {
         cacheControl: false,
         redirect: false,
-        setHeaders: (res, filePath): void => {
+        setHeaders: (res, filePath) => {
           if (VERSIONED_FILE.test(filePath)) {
             res.setHeader('cache-control', VERSIONED_CACHE_CONTROL);
           } else {

backend/src/services/RetroService.test.ts

@@ -19,7 +19,7 @@ class ChangeListener {
   ) => void;
 
   public constructor() {
-    this.onChange = (message: ChangeInfo<Spec<Retro>>, meta: unknown): void => {
+    this.onChange = (message: ChangeInfo<Spec<Retro>>, meta: unknown) => {
       this.messages.push({ message, meta });
     };
   }

backend/src/services/RetroService.ts

@@ -13,7 +13,7 @@ import { extractRetro } from '../helpers/jsonParsers';
 const VALID_SLUG = /^[a-z0-9][a-z0-9_\-]*$/;
 const MAX_SLUG_LENGTH = 64;
 
-function validateSlug(slug: string): void {
+function validateSlug(slug: string) {
   if (slug.length > MAX_SLUG_LENGTH) {
     throw new Error('URL is too long');
   }

backend/src/tokens/TokenManager.ts

@@ -15,7 +15,7 @@ const asyncGenerateKeyPair = promisify(
     type: string,
     options: Record<string, unknown>,
     callback: GenerateKeyPairCallback,
-  ): void =>
+  ) =>
     generateKeyPair(
       type as any,
       options as any,

e2e/helpers/downloads.ts

@@ -22,9 +22,7 @@ export async function waitForFile(
     try {
       return await readFile(fileName, { encoding: 'utf-8' });
     } catch (e) {
-      await new Promise((res): void => {
-        setTimeout(res, 100);
-      });
+      await new Promise((res) => setTimeout(res, 100));
     }
   } while (Date.now() < exp);
 

e2e/helpers/selenium.ts

@@ -11,6 +11,7 @@ const headless = process.env['HEADLESS'] !== 'false';
 
 const width = 900; // ensure non-mobile display
 const height = 500;
+const headlessUserAgent = 'HeadlessEndToEndTest';
 
 const chromeOptions = new ChromeOptions();
 chromeOptions.windowSize({ width, height });
@@ -34,8 +35,13 @@ if (process.env['DOCKER'] === 'true') {
 }
 
 if (headless) {
-  chromeOptions.addArguments('--headless=new');
-  firefoxOptions.addArguments('--headless');
+  chromeOptions.addArguments(
+    '--headless=new',
+    '--user-agent=' + headlessUserAgent,
+  );
+  firefoxOptions
+    .addArguments('--headless')
+    .setPreference('general.useragent.override', headlessUserAgent);
 }
 
 const logPrefs = new logging.Preferences();

frontend/src/api/PasswordService.ts

@@ -5,8 +5,8 @@ export class PasswordService {
 
   public async countPasswordBreaches(password: string): Promise<number> {
     const passwordHash = (await sha1(password)).toUpperCase();
-    const key = passwordHash.substr(0, 5);
-    const rest = passwordHash.substr(5);
+    const key = passwordHash.substring(0, 5);
+    const rest = passwordHash.substring(5);
 
     const response = await fetch(`${this.apiBase}/password-check/${key}`);
     const lines = (await response.text()).split('\n');

frontend/src/api/RetroTracker.ts

@@ -43,33 +43,33 @@ class RetroWrapper {
   ) {
     this.retroStateCallbacks = new Set();
 
-    const setState = (state: RetroState): void => {
+    const setState = (state: RetroState) => {
       this.latestState = state;
       this.retroStateCallbacks.forEach((fn) => fn(state));
     };
 
     this.reducer = SharedReducer.for<Retro>(
       `${wsBase}/retros/${retroId}`,
-      (data): void => setState({ retro: data, error: null }),
+      (data) => setState({ retro: data, error: null }),
     )
       .withReducer(context)
       .withToken(retroToken)
-      .withErrorHandler((err): void => setState({ retro: null, error: err }))
+      .withErrorHandler((err) => setState({ retro: null, error: err }))
       .build();
   }
 
-  public addStateCallback(stateCallback: RetroStateCallback): void {
+  public addStateCallback(stateCallback: RetroStateCallback) {
     this.retroStateCallbacks.add(stateCallback);
     if (this.latestState) {
       stateCallback(this.latestState);
     }
   }
 
-  public removeStateCallback(stateCallback: RetroStateCallback): void {
+  public removeStateCallback(stateCallback: RetroStateCallback) {
     this.retroStateCallbacks.delete(stateCallback);
   }
 
-  public close(): void {
+  public close() {
     this.reducer.close();
   }
 }
@@ -84,7 +84,7 @@ export class RetroTracker {
     this.subscriptionTracker = new SubscriptionTracker(
       ({ retroId, retroToken }): RetroWrapper =>
         new RetroWrapper(apiBase, wsBase, retroId, retroToken),
-      (service): void => service.close(),
+      (service) => service.close(),
     );
   }
 
@@ -100,7 +100,7 @@ export class RetroTracker {
     sub.service.addStateCallback(retroStateCallback);
 
     return {
-      unsubscribe: (): void => {
+      unsubscribe: () => {
         sub.service.removeStateCallback(retroStateCallback);
         sub.unsubscribe().catch((e) => {
           console.warn(`Failed to unsubscribe from retro ${retroId}`, e);

frontend/src/api/SlugTracker.ts

@@ -47,11 +47,11 @@ export class SlugTracker {
     return this.storage.get(slug).observable;
   }
 
-  public set(slug: string, id: string): void {
+  public set(slug: string, id: string) {
     this.storage.get(slug).subject.next(id);
   }
 
-  public remove(slug: string): void {
+  public remove(slug: string) {
     this.storage.remove(slug);
   }
 

frontend/src/api/__mocks__/api.ts

@@ -20,13 +20,13 @@ class FakeRetroTracker {
 
   private expectedRetroToken: string | null = null;
 
-  public dispatch = (): void => undefined;
+  public dispatch = () => undefined;
 
-  public setExpectedToken(retroToken: string): void {
+  public setExpectedToken(retroToken: string) {
     this.expectedRetroToken = retroToken;
   }
 
-  public setServerData(retroId: string, serverData: Partial<RetroState>): void {
+  public setServerData(retroId: string, serverData: Partial<RetroState>) {
     this.data.set(retroId, {
       retro: null,
       error: null,
@@ -55,7 +55,7 @@ class FakeRetroTracker {
     retroStateCallback({ ...serverData });
 
     return {
-      unsubscribe: (): void => {
+      unsubscribe: () => {
         this.subscribed -= 1;
       },
     };
@@ -67,15 +67,15 @@ class FakeArchiveTracker {
 
   private expectedRetroToken: string | null = null;
 
-  public setExpectedToken(retroToken: string): void {
+  public setExpectedToken(retroToken: string) {
     this.expectedRetroToken = retroToken;
   }
 
   public setServerData(
     retroId: string,
     archiveId: string,
     archive: RetroArchive,
-  ): void {
+  ) {
     if (!this.data.has(retroId)) {
       this.data.set(retroId, new Map());
     }
@@ -131,7 +131,7 @@ class FakeUserTokenService {
 
   private userToken: string | null = null;
 
-  public setServerData(userToken: string): void {
+  public setServerData(userToken: string) {
     this.userToken = userToken;
   }
 
@@ -153,7 +153,7 @@ class FakeRetroTokenService {
 
   private data = new Map<string, string>();
 
-  public setServerData(retroId: string, retroToken: string): void {
+  public setServerData(retroId: string, retroToken: string) {
     this.data.set(retroId, retroToken);
   }
 
@@ -196,7 +196,7 @@ class FakeRetroService {
 
   private id: string | null = null;
 
-  public setServerData(retroId: string): void {
+  public setServerData(retroId: string) {
     this.id = retroId;
   }
 

frontend/src/api/reducer.ts

@@ -1,10 +1,6 @@
-import React from 'react';
 import listCommands from 'json-immutability-helper/commands/list';
 import { context as defaultContext, type Spec } from 'json-immutability-helper';
-import { makeHooks } from 'json-immutability-helper/helpers/hooks';
 
 export type { Spec };
 
 export const context = defaultContext.with(listCommands);
-
-export const { useEvent, useScopedReducer } = makeHooks(context, React);

frontend/src/components/RetroRouter.tsx

@@ -37,7 +37,7 @@ function replaceSlug(
   setLocation: SetLocation,
   newSlug: string,
   retroId: string,
-): void {
+) {
   const oldSlug = RETRO_SLUG_PATH.exec(oldPath)?.[1];
   if (!oldSlug || oldSlug === newSlug) {
     return;
@@ -46,7 +46,7 @@ function replaceSlug(
   slugTracker.set(newSlug, retroId);
   const oldPrefix = `/retros/${oldSlug}`;
   const newPrefix = `/retros/${newSlug}`;
-  const newPath = newPrefix + oldPath.substr(oldPrefix.length);
+  const newPath = newPrefix + oldPath.substring(oldPrefix.length);
   setLocation(newPath, { replace: true });
 }
 
@@ -83,7 +83,7 @@ function useRetroReducer(
       (data: RetroState) => nonce.check(myNonce) && setRetroState(data),
       (err: RetroError) => nonce.check(myNonce) && setError(err),
     );
-    return (): void => subscription.unsubscribe();
+    return () => subscription.unsubscribe();
   }, [
     retroTracker,
     setRetroState,

frontend/src/components/archive-list/ArchiveListPage.tsx

@@ -22,14 +22,7 @@ export const ArchiveListPage = memo(({ retroToken, retro }: PropsT) => {
       <Loader
         error={archivesError}
         Component={ArchiveList}
-        componentProps={
-          archives
-            ? {
-                slug: retro.slug,
-                archives,
-              }
-            : null
-        }
+        componentProps={archives ? { slug: retro.slug, archives } : null}
       />
       <div className="extra-links">
         <ApiDownload

frontend/src/components/archive/ArchivePage.tsx

@@ -44,7 +44,7 @@ export const ArchivePage = memo(
                   group,
                   archive: true,
                   archiveTime: archive.created,
-                  onComplete: (): void => undefined,
+                  onComplete: () => undefined,
                 }
               : null
           }

frontend/src/components/attachments/giphy/GiphyButton.tsx

@@ -1,8 +1,8 @@
-import { useState, useCallback, memo } from 'react';
+import { useState, memo } from 'react';
 import { type RetroItemAttachment } from '../../../shared/api-entities';
+import { useEvent } from '../../../hooks/useEvent';
 import { Popup } from '../../common/Popup';
 import { WrappedButton } from '../../common/WrappedButton';
-import { useBoundCallback } from '../../../hooks/useBoundCallback';
 import { GiphyPopup } from './GiphyPopup';
 
 interface PropsT {
@@ -13,15 +13,12 @@ interface PropsT {
 export const GiphyButton = memo(
   ({ defaultAttachment = null, onChange }: PropsT) => {
     const [visible, setVisible] = useState(false);
-    const show = useBoundCallback(setVisible, true);
-    const hide = useBoundCallback(setVisible, false);
-    const handleSave = useCallback(
-      (newAttachment: RetroItemAttachment | null) => {
-        hide();
-        onChange(newAttachment);
-      },
-      [hide, onChange],
-    );
+    const show = useEvent(() => setVisible(true));
+    const hide = useEvent(() => setVisible(false));
+    const handleSave = useEvent((newAttachment: RetroItemAttachment | null) => {
+      hide();
+      onChange(newAttachment);
+    });
 
     let popup = null;
     if (visible) {

frontend/src/components/attachments/giphy/GiphyPopup.tsx

@@ -1,8 +1,7 @@
-import { useState, useCallback, memo, SyntheticEvent } from 'react';
+import { useState, memo, SyntheticEvent } from 'react';
 import { type RetroItemAttachment } from '../../../shared/api-entities';
 import { WrappedButton } from '../../common/WrappedButton';
 import { Input } from '../../common/Input';
-import { useBoundCallback } from '../../../hooks/useBoundCallback';
 import { useNonce } from '../../../hooks/useNonce';
 import { giphyService } from '../../../api/api';
 import { type GifInfo } from '../../../api/GiphyService';
@@ -19,33 +18,29 @@ export const GiphyPopup = memo(
     const [query, setQuery] = useState('');
     const [options, setOptions] = useState<GifInfo[]>([]);
 
-    const handleDelete = useBoundCallback(onConfirm, null);
     const deleteButton = defaultAttachment ? (
-      <WrappedButton onClick={handleDelete}>Remove</WrappedButton>
+      <WrappedButton onClick={() => onConfirm(null)}>Remove</WrappedButton>
     ) : null;
 
     const loadNonce = useNonce();
-    const loadOptions = useCallback(
-      async (e: SyntheticEvent) => {
-        e.preventDefault();
-        e.stopPropagation();
-        const nonce = loadNonce.next();
-        setOptions([]);
+    const loadOptions = async (e: SyntheticEvent) => {
+      e.preventDefault();
+      e.stopPropagation();
+      const nonce = loadNonce.next();
+      setOptions([]);
 
-        const gifs = await giphyService.search(query);
-        if (!loadNonce.check(nonce)) {
-          return;
-        }
+      const gifs = await giphyService.search(query);
+      if (!loadNonce.check(nonce)) {
+        return;
+      }
 
-        setOptions(gifs);
-      },
-      [loadNonce, query],
-    );
+      setOptions(gifs);
+    };
 
     const optionElements = options.map(({ small, medium }) => (
       <WrappedButton
         key={medium}
-        onClick={(): void => onConfirm({ type: 'giphy', url: medium })}
+        onClick={() => onConfirm({ type: 'giphy', url: medium })}
       >
         <img
           src={small}