Merge pull request #1994 from GMOD/fix_loading_session_plugin_in_worker

Make session plugins get transferred to the worker properly

packages/core/Plugin.ts

@@ -7,6 +7,10 @@ import { AnyConfigurationSchemaType } from './configuration/configurationSchema'
 export default abstract class Plugin {
   abstract name: string
 
+  url?: string
+
+  version?: string
+
   install(_pluginManager: PluginManager): void {}
 
   configure(_pluginManager: PluginManager): void {}

packages/core/PluginLoader.ts

@@ -26,6 +26,11 @@ export interface PluginDefinition {
   url: string
 }
 
+export interface PluginRecord {
+  plugin: PluginConstructor
+  definition: PluginDefinition
+}
+
 export default class PluginLoader {
   definitions: PluginDefinition[] = []
 
@@ -95,9 +100,12 @@ export default class PluginLoader {
     })
   }
 
-  async load(): Promise<PluginConstructor[]> {
+  async load(): Promise<PluginRecord[]> {
     return Promise.all(
-      this.definitions.map(definition => this.loadPlugin(definition)),
+      this.definitions.map(async definition => ({
+        plugin: await this.loadPlugin(definition),
+        definition,
+      })),
     )
   }
 }

packages/core/PluginManager.ts

@@ -32,6 +32,7 @@ import { AnyConfigurationSchemaType } from './configuration/configurationSchema'
 import { AbstractRootModel } from './util'
 import CorePlugin from './CorePlugin'
 import createJexlInstance from './util/jexl'
+import { PluginDefinition } from './PluginLoader'
 
 /** little helper class that keeps groups of callbacks that are
 then run in a specified order by group */
@@ -134,20 +135,25 @@ type AnyFunction = (...args: any) => any
  * Can also use this metadata to stash other things about why the plugin is
  * loaded, such as where it came from, what plugin depends on it, etc.
  */
-export type PluginMetaData = Record<string, unknown>
+export type PluginMetadata = Record<string, unknown>
 
-export type PluginLoadRecord = {
-  metadata: PluginMetaData
+export interface PluginLoadRecord {
+  metadata?: PluginMetadata
   plugin: Plugin
 }
+export interface RuntimePluginLoadRecord extends PluginLoadRecord {
+  definition: PluginDefinition
+}
 
 export default class PluginManager {
   plugins: Plugin[] = []
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   jexl: any = createJexlInstance()
 
-  pluginMetaData: Record<string, PluginMetaData> = {}
+  pluginMetadata: Record<string, PluginMetadata> = {}
+
+  runtimePluginDefinitions: PluginDefinition[] = []
 
   elementCreationSchedule = new PhasedScheduler<PluggableElementTypeGroup>(
     'renderer',
@@ -200,17 +206,21 @@ export default class PluginManager {
     return configurationSchemas
   }
 
-  addPlugin(load: Plugin | PluginLoadRecord) {
+  addPlugin(load: Plugin | PluginLoadRecord | RuntimePluginLoadRecord) {
     if (this.configured) {
       throw new Error('JBrowse already configured, cannot add plugins')
     }
-    const [plugin, metadata] =
+    const [plugin, metadata = {}] =
       load instanceof Plugin ? [load, {}] : [load.plugin, load.metadata]
 
     if (this.plugins.includes(plugin)) {
       throw new Error('plugin already installed')
     }
-    this.pluginMetaData[plugin.name] = metadata
+
+    this.pluginMetadata[plugin.name] = metadata
+    if ('definition' in load) {
+      this.runtimePluginDefinitions.push(load.definition)
+    }
     plugin.install(this)
     this.plugins.push(plugin)
     return this

packages/core/rpc/RpcManager.ts

@@ -40,19 +40,15 @@ export default class RpcManager {
 
   backendConfigurations: BackendConfigurations
 
-  runtimePluginDefinitions: PluginDefinition[]
-
   constructor(
     pluginManager: PluginManager,
-    runtimePluginDefinitions: PluginDefinition[] = [],
     mainConfiguration: AnyConfigurationModel,
     backendConfigurations: BackendConfigurations,
   ) {
     if (!mainConfiguration) {
       throw new Error('RpcManager requires at least a main configuration')
     }
     this.pluginManager = pluginManager
-    this.runtimePluginDefinitions = runtimePluginDefinitions
     this.mainConfiguration = mainConfiguration
     this.backendConfigurations = backendConfigurations
     this.driverObjects = new Map()
@@ -74,7 +70,7 @@ export default class RpcManager {
     }
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     const newDriver = new DriverClassImpl(backendConfiguration as any, {
-      plugins: this.runtimePluginDefinitions,
+      plugins: this.pluginManager.runtimePluginDefinitions,
     })
     this.driverObjects.set(backendName, newDriver)
     return newDriver

packages/core/util/types/index.ts

@@ -89,6 +89,7 @@ export interface AbstractSessionModel extends AbstractViewContainer {
   showWidget?: Function
   addWidget?: Function
 
+  addTrackConf?: Function
   DialogComponent?: DialogComponentType
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   DialogProps: any

plugins/data-management/src/PluginStoreWidget/components/InstalledPluginsList.tsx

@@ -17,30 +17,26 @@ function InstalledPluginsList({
   pluginManager: PluginManager
   model: PluginStoreModel
 }) {
-  const { plugins } = pluginManager
+  const { plugins } = pluginManager as PluginManager
 
   const corePlugins = plugins
-    .filter((p: BasePlugin) =>
-      Boolean(pluginManager.pluginMetaData[p.name]?.isCore),
-    )
-    .map((p: BasePlugin) => p.name)
-
-  const externalPlugins = plugins.filter((plugin: BasePlugin) => {
-    return !corePlugins.includes(plugin.name)
-  })
-
-  const externalPluginsRender = externalPlugins
-    .filter((plugin: BasePlugin) => {
-      return plugin.name.toLowerCase().includes(model.filterText.toLowerCase())
-    })
-    .map((plugin: BasePlugin) => {
-      return <InstalledPlugin key={plugin.name} plugin={plugin} model={model} />
-    })
+    .filter(p => pluginManager.pluginMetadata[p.name]?.isCore)
+    .map(p => p.name)
+
+  const externalPlugins = plugins.filter(
+    plugin => !corePlugins.includes(plugin.name),
+  )
 
   return (
     <List>
       {externalPlugins.length ? (
-        externalPluginsRender
+        externalPlugins
+          .filter(plugin =>
+            plugin.name.toLowerCase().includes(model.filterText.toLowerCase()),
+          )
+          .map(plugin => (
+            <InstalledPlugin key={plugin.name} plugin={plugin} model={model} />
+          ))
       ) : (
         <Typography>No plugins currently installed</Typography>
       )}

plugins/menus/src/AboutWidget/components/AboutWidget.test.js

@@ -1,10 +1,32 @@
 import React from 'react'
 import { render } from '@testing-library/react'
 import AboutWidget from './AboutWidget'
+import { types } from 'mobx-state-tree'
+import { ConfigurationSchema } from '@jbrowse/core/configuration'
 
 describe('<AboutWidget />', () => {
   it('renders', () => {
-    const { container } = render(<AboutWidget />)
+    const session = types
+      .model({
+        configuration: ConfigurationSchema('Null', {}),
+        rpcManager: types.frozen({}),
+      })
+      .create(
+        {},
+        {
+          pluginManager: {
+            pluginMetadata: {},
+            plugins: [
+              {
+                name: 'HelloPlugin',
+                version: '1.0.0',
+                url: 'http://google.com',
+              },
+            ],
+          },
+        },
+      )
+    const { container } = render(<AboutWidget model={session} />)
     expect(container.firstChild).toMatchSnapshot()
   })
 })

plugins/menus/src/AboutWidget/components/AboutWidget.tsx

@@ -3,7 +3,7 @@ import { observer } from 'mobx-react'
 import { IAnyStateTreeNode, getEnv } from 'mobx-state-tree'
 import { getSession } from '@jbrowse/core/util'
 import { makeStyles, Typography, Link } from '@material-ui/core'
-import { PluginMetaData } from '@jbrowse/core/PluginManager'
+import PluginManager from '@jbrowse/core/PluginManager'
 
 const useStyles = makeStyles(theme => ({
   root: {
@@ -19,46 +19,28 @@ const useStyles = makeStyles(theme => ({
   },
 }))
 
-interface BasePlugin {
-  version?: string
-  name: string
-  url?: string
-}
-
 function About({ model }: { model: IAnyStateTreeNode }) {
   const classes = useStyles()
-  const slotDefinition = {
-    version: '',
-    pluginManager: {
-      plugins: [],
-      pluginMetaData: {} as Record<string, PluginMetaData>,
-    },
-  }
-  const { pluginManager } = model ? getEnv(model) : slotDefinition
-  const { plugins } = pluginManager
+  const { version } = getSession(model)
+  const { pluginManager } = getEnv(model)
+  const { plugins } = pluginManager as PluginManager
   const corePlugins = plugins
-    .filter((p: BasePlugin) =>
-      Boolean(pluginManager.pluginMetaData[p.name]?.isCore),
-    )
-    .map((p: BasePlugin) => p.name)
+    .filter(p => pluginManager.pluginMetadata[p.name]?.isCore)
+    .map(p => p.name)
 
   const corePluginsRender = plugins
-    .filter((plugin: BasePlugin) => {
+    .filter(plugin => {
       return corePlugins.includes(plugin.name)
     })
-    .map((plugin: BasePlugin) => {
-      return (
-        <li key={plugin.name}>
-          {plugin.name} {plugin.version || ''}
-        </li>
-      )
-    })
+    .map(plugin => (
+      <li key={plugin.name}>
+        {plugin.name} {plugin.version || ''}
+      </li>
+    ))
 
   const externalPluginsRender = plugins
-    .filter((plugin: BasePlugin) => {
-      return !corePlugins.includes(plugin.name)
-    })
-    .map((plugin: BasePlugin) => {
+    .filter(plugin => !corePlugins.includes(plugin.name))
+    .map(plugin => {
       const text = `${plugin.name} ${plugin.version || ''}`
       return (
         <li key={plugin.name}>
@@ -79,7 +61,7 @@ function About({ model }: { model: IAnyStateTreeNode }) {
         JBrowse 2
       </Typography>
       <Typography variant="h6" align="center" className={classes.subtitle}>
-        {model ? getSession(model).version : slotDefinition.version}
+        {version}
       </Typography>
       <Typography align="center" variant="body2">
         JBrowse is a{' '}

plugins/menus/src/AboutWidget/components/__snapshots__/AboutWidget.test.js.snap

@@ -36,6 +36,24 @@ exports[`<AboutWidget /> renders 1`] = `
   </p>
   <div
     class="makeStyles-pluginList"
-  />
+  >
+    <h6
+      class="MuiTypography-root MuiTypography-h6"
+    >
+      External plugins loaded
+    </h6>
+    <ul>
+      <li>
+        <a
+          class="MuiTypography-root MuiLink-root MuiLink-underlineHover MuiTypography-colorPrimary"
+          href="http://google.com"
+          rel="noopener noreferrer"
+          target="_blank"
+        >
+          HelloPlugin 1.0.0
+        </a>
+      </li>
+    </ul>
+  </div>
 </div>
 `;

products/jbrowse-desktop/src/Loader.tsx

@@ -80,7 +80,10 @@ export default function Loader({
               plugin: new P(),
               metadata: { isCore: true },
             })),
-            ...runtimePlugins.map(P => new P()),
+            ...runtimePlugins.map(({ plugin: P, definition }) => ({
+              plugin: new P(),
+              definition,
+            })),
           ])
         } catch (e) {
           // used to launch an error dialog for whatever caused plugin loading

products/jbrowse-desktop/src/rootModel.ts

@@ -140,7 +140,6 @@ export default function RootModel(pluginManager: PluginManager) {
       ] as Menu[],
       rpcManager: new RpcManager(
         pluginManager,
-        getSnapshot(self.jbrowse.plugins),
         self.jbrowse.configuration.rpc,
         {
           ElectronRpcDriver: { workerCreationChannel: 'createWindowWorker' },

products/jbrowse-desktop/src/worker.ts

@@ -87,8 +87,10 @@ async function getPluginManager() {
   const pluginLoader = new PluginLoader(config.plugins)
   pluginLoader.installGlobalReExports(window)
   const runtimePlugins = await pluginLoader.load()
-  const plugins = [...corePlugins, ...runtimePlugins]
-  const pluginManager = new PluginManager(plugins.map(P => new P()))
+  const plugins = [...corePlugins.map(p => ({ plugin: p })), ...runtimePlugins]
+  const pluginManager = new PluginManager(
+    plugins.map(({ plugin: P }) => new P()),
+  )
 
   pluginManager.createPluggableElements()
   pluginManager.configure()

products/jbrowse-react-linear-genome-view/src/createModel/createModel.ts

@@ -54,14 +54,9 @@ export default function createModel(runtimePlugins: PluginConstructor[]) {
       },
     }))
     .volatile(self => ({
-      rpcManager: new RpcManager(
-        pluginManager,
-        // don't need runtimePluginDefinitions since MainThreadRpcDriver doesn't
-        // use them
-        [],
-        self.config.configuration.rpc,
-        { MainThreadRpcDriver: {} },
-      ),
+      rpcManager: new RpcManager(pluginManager, self.config.configuration.rpc, {
+        MainThreadRpcDriver: {},
+      }),
     }))
   return { model: rootModel, pluginManager }
 }

products/jbrowse-react-linear-genome-view/stories/JBrowseLinearGenomeView.stories.tsx

@@ -1,4 +1,4 @@
-import { PluginConstructor } from '@jbrowse/core/Plugin'
+import { PluginRecord } from '@jbrowse/core/PluginLoader'
 import React, { useEffect, useState } from 'react'
 import {
   createViewState,
@@ -164,7 +164,7 @@ export const WithPlugins = () => {
   // const plugins = [UCSCPlugin]
 
   // alternative usage with runtime plugins
-  const [plugins, setPlugins] = useState<PluginConstructor[]>()
+  const [plugins, setPlugins] = useState<PluginRecord[]>()
   useEffect(() => {
     async function getPlugins() {
       const loadedPlugins = await loadPlugins([
@@ -213,7 +213,7 @@ export const WithPlugins = () => {
         },
       },
     },
-    plugins,
+    plugins: plugins.map(p => p.plugin),
     tracks: [
       {
         type: 'FeatureTrack',