fixes #188: make SSE connection deferred to first useTock call (#189)

* fixes #188: make SSE connection deferred to first useTock call

* update jsdoc

* fix bad hook execution order

* clean up useEffect

* rename UseTock internal interface

* add missing cleanup of SSE retry

Author: Fabilin

src/network/TockEventSource.ts

@@ -42,6 +42,7 @@ export class TockEventSource {
   private initialized: boolean;
   private eventSource: EventSource | null;
   private retryDelay: number;
+  private retryTimeoutId: number;
   onResponse: (botResponse: BotConnectorResponse) => void;
   onStateChange: (state: number) => void;
 
@@ -93,7 +94,7 @@ export class TockEventSource {
       MAX_RETRY_DELAY,
       retryDelay + RETRY_DELAY_INCREMENT,
     );
-    setTimeout(async () => {
+    this.retryTimeoutId = window.setTimeout(async () => {
       switch (await getSseStatus(url)) {
         case SseStatus.UNSUPPORTED:
           reject();
@@ -110,6 +111,7 @@ export class TockEventSource {
   }
 
   close() {
+    window.clearTimeout(this.retryTimeoutId);
     this.eventSource?.close();
     this.eventSource = null;
     this.initialized = false;

src/useTock.ts

@@ -76,6 +76,20 @@ export interface UseTock {
   sseInitializing: boolean;
 }
 
+/**
+ * Internal extensions for {@link UseTock}
+ */
+interface UseTockInternal extends UseTock {
+  /**
+   * Hook that initializes the SSE connection
+   *
+   * If SSE is disabled by the settings, the hook simply resolves the sseInitPromise.
+   *
+   * This method is idempotent.
+   */
+  useSseInit: () => void;
+}
+
 function mapButton(button: BotConnectorButton): Button {
   if (button.type === 'postback') {
     return new PostBackButton(button.title, button.payload, button.imageUrl);
@@ -126,7 +140,7 @@ export const useTock0: (
   extraHeadersProvider?: () => Promise<Record<string, string>>,
   disableSse?: boolean,
   localStorageHistory?: TockLocalStorage,
-) => UseTock = (
+) => UseTockInternal = (
   tockEndPoint: string,
   { locale, localStorage: localStorageSettings, network: networkSettings },
   extraHeadersProvider?: () => Promise<Record<string, string>>,
@@ -594,24 +608,26 @@ export const useTock0: (
     [],
   );
 
-  useEffect(() => {
-    sseSource.current.onStateChange = onSseStateChange;
-    sseSource.current.onResponse = handleSseBotResponse;
-  }, [handleSseBotResponse, onSseStateChange]);
+  const useSseInit = () => {
+    useEffect(() => {
+      sseSource.current.onStateChange = onSseStateChange;
+      sseSource.current.onResponse = handleSseBotResponse;
+    }, [handleSseBotResponse, onSseStateChange]);
 
-  useEffect(() => {
-    if (disableSse || !tockEndPoint.length) {
-      afterInit.current();
-    } else {
-      // Trigger afterInit regardless of whether the SSE call succeeded or failed
-      // (it is valid for the backend to refuse SSE connections, but we still attempt to connect by default)
-      sseSource.current
-        .open(tockEndPoint, userId)
-        .catch((e) => console.error(e))
-        .finally(afterInit.current);
-    }
-    return () => sseSource.current.close();
-  }, [disableSse, tockEndPoint]);
+    useEffect(() => {
+      if (disableSse || !tockEndPoint.length) {
+        afterInit.current();
+      } else {
+        // Trigger afterInit regardless of whether the SSE call succeeded or failed
+        // (it is valid for the backend to refuse SSE connections, but we still attempt to connect by default)
+        sseSource.current
+          .open(tockEndPoint, userId)
+          .catch((e) => console.error(e))
+          .finally(afterInit.current);
+      }
+      return () => sseSource.current.close();
+    }, [disableSse, tockEndPoint]);
+  };
 
   const addHistory: (
     messageHistory: Array<Message>,
@@ -698,36 +714,44 @@ export const useTock0: (
     loadHistory,
     sseInitPromise: afterInitPromise.current,
     sseInitializing,
+    useSseInit,
   };
 };
 
-export const UseTockContext = createContext<UseTock | undefined>(undefined);
+export const UseTockContext = createContext<UseTockInternal | undefined>(
+  undefined,
+);
 
 export default (
   tockEndPoint?: string,
   extraHeadersProvider?: () => Promise<Record<string, string>>,
   disableSse?: boolean,
   localStorageHistory?: TockLocalStorage,
-) => {
+): UseTock => {
   const contextTock = useContext(UseTockContext);
   const settings = useTockSettings();
-  if (contextTock != null) {
-    return contextTock;
-  }
+
   if (settings.endpoint == null && tockEndPoint == null) {
     throw new Error('TOCK endpoint must be provided in TockContext');
   } else if (settings.endpoint == null) {
     console.warn(
       'Passing TOCK endpoint as argument to TockChat or useTock is deprecated; please set it in TockContext instead.',
     );
   }
-  return contextTock
-    ? contextTock
-    : useTock0(
-        (tockEndPoint ?? settings.endpoint)!,
-        settings,
-        extraHeadersProvider,
-        disableSse,
-        localStorageHistory,
-      );
+
+  // the following conditional does not follow the rules of hooks,
+  // but in practice the endpoint setting should not appear or disappear in a live app
+  const ret =
+    contextTock ??
+    useTock0(
+      (tockEndPoint ?? settings.endpoint)!,
+      settings,
+      extraHeadersProvider,
+      disableSse,
+      localStorageHistory,
+    );
+
+  ret.useSseInit();
+
+  return ret;
 };