Make plugin manager initialization and cleanup use react ref (#5146)

Author: garrettjstevens

products/jbrowse-web/src/components/Loader.tsx

@@ -1,4 +1,4 @@
-import { Suspense, lazy, useEffect, useState } from 'react'
+import { Suspense, lazy, useCallback, useEffect, useRef, useState } from 'react'
 
 import { FatalErrorDialog } from '@jbrowse/core/ui'
 import { ErrorBoundary } from '@jbrowse/core/ui/ErrorBoundary'
@@ -98,12 +98,10 @@ const Renderer = observer(function ({
   loader: SessionLoaderModel
 }) {
   const [loader, setLoader] = useState(firstLoader)
-  const { configError, ready, sessionTriaged } = loader
-  const [pluginManager, setPluginManager] = useState<PluginManager>()
-  const [error, setError] = useState<unknown>()
-
-  useEffect(() => {
-    const reloadPluginManager = (
+  const pluginManager = useRef<PluginManager | undefined>(undefined)
+  const [pluginManagerCreated, setPluginManagerCreated] = useState(false)
+  const reloadPluginManager = useCallback(
+    (
       configSnapshot: Record<string, unknown>,
       sessionSnapshot: Record<string, unknown>,
     ) => {
@@ -125,22 +123,32 @@ const Renderer = observer(function ({
         sessionSnapshot,
       })
       setLoader(newLoader)
-    }
+      setPluginManagerCreated(false)
+    },
+    [loader],
+  )
+  const { configError, ready, sessionTriaged } = loader
+  const [error, setError] = useState<unknown>()
 
-    try {
-      if (ready) {
-        setPluginManager(previousPluginManager => {
-          if (previousPluginManager?.rootModel) {
-            destroy(previousPluginManager.rootModel)
-          }
-          return createPluginManager(loader, reloadPluginManager)
-        })
+  useEffect(() => {
+    if (ready) {
+      try {
+        if (pluginManager.current?.rootModel) {
+          destroy(pluginManager.current.rootModel)
+        }
+        pluginManager.current = createPluginManager(loader, reloadPluginManager)
+        setPluginManagerCreated(true)
+      } catch (e) {
+        console.error(e)
+        setError(e)
+      }
+    }
+    return () => {
+      if (pluginManager.current?.rootModel) {
+        destroy(pluginManager.current.rootModel)
       }
-    } catch (e) {
-      console.error(e)
-      setError(e)
     }
-  }, [loader, ready])
+  }, [ready, loader, reloadPluginManager])
 
   const err = configError || error
   if (err) {
@@ -151,8 +159,8 @@ const Renderer = observer(function ({
     )
   } else if (sessionTriaged) {
     return <SessionTriaged loader={loader} sessionTriaged={sessionTriaged} />
-  } else if (pluginManager) {
-    return <JBrowse pluginManager={pluginManager} />
+  } else if (pluginManagerCreated && pluginManager.current) {
+    return <JBrowse pluginManager={pluginManager.current} />
   } else {
     return <Loading />
   }

products/jbrowse-web/src/tests/Loader.test.tsx

@@ -1,5 +1,3 @@
-import { FatalErrorDialog } from '@jbrowse/core/ui'
-import { ErrorBoundary } from '@jbrowse/core/ui/ErrorBoundary'
 import { render, waitFor } from '@testing-library/react'
 import { Image, createCanvas } from 'canvas'
 import { LocalFile } from 'generic-filehandle2'
@@ -118,17 +116,7 @@ test('can use config from a url with nonexistent share param ', async () => {
 
 test('can catch error from loading a bad config', async () => {
   const { findAllByText } = render(
-    <ErrorBoundary
-      FallbackComponent={props => (
-        <FatalErrorDialog
-          {...props}
-          resetButtonText="Reset Session"
-          onFactoryReset={() => {}}
-        />
-      )}
-    >
-      <App search="?config=test_data/bad_config_test/config.json" />
-    </ErrorBoundary>,
+    <App search="?config=test_data/bad_config_test/config.json" />,
   )
   await findAllByText(/Error while converting/)
 }, 20000)