From bb2ad7c07a6017cbe64ceb1d695c66e0d661897b Mon Sep 17 00:00:00 2001 From: Colin Diesh Date: Fri, 20 Oct 2023 16:52:10 -0400 Subject: [PATCH] Refactors and typescript improvements for jbrowse-web loader (#4005) --- packages/core/PluginLoader.ts | 2 +- products/jbrowse-web/src/SessionLoader.ts | 34 +++---- .../src/components/ConfigWarningDialog.tsx | 25 +++-- .../jbrowse-web/src/components/Loader.tsx | 98 +++++++++++-------- .../src/components/SessionWarningDialog.tsx | 24 ++--- .../jbrowse-web/src/createPluginManager.ts | 2 + .../tests/AssemblySelectorImportForm.test.tsx | 4 +- .../jbrowse-web/src/tests/Loader.test.tsx | 4 +- products/jbrowse-web/src/util.ts | 2 +- 9 files changed, 106 insertions(+), 89 deletions(-) diff --git a/packages/core/PluginLoader.ts b/packages/core/PluginLoader.ts index 346baa4ae1..6b90b0149e 100644 --- a/packages/core/PluginLoader.ts +++ b/packages/core/PluginLoader.ts @@ -111,7 +111,7 @@ export interface LoadedPlugin { default: PluginConstructor } -function pluginDescriptionString(pluginDefinition: PluginDefinition) { +export function pluginDescriptionString(pluginDefinition: PluginDefinition) { if (isUMDPluginDefinition(pluginDefinition)) { return `UMD plugin ${pluginDefinition.name}` } diff --git a/products/jbrowse-web/src/SessionLoader.ts b/products/jbrowse-web/src/SessionLoader.ts index 4b02469657..5a2bb6be1a 100644 --- a/products/jbrowse-web/src/SessionLoader.ts +++ b/products/jbrowse-web/src/SessionLoader.ts @@ -12,6 +12,12 @@ import { nanoid } from '@jbrowse/core/util/nanoid' import { readSessionFromDynamo } from './sessionSharing' import { addRelativeUris, checkPlugins, fromUrlSafeB64, readConf } from './util' +export interface SessionTriagedInfo { + snap: unknown + origin: string + reason: PluginDefinition[] +} + const SessionLoader = types .model({ configPath: types.maybe(types.string), @@ -25,18 +31,11 @@ const SessionLoader = types initialTimestamp: types.number, }) .volatile(() => ({ - // eslint-disable-next-line @typescript-eslint/no-explicit-any, - blankSession: false as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any, - sessionTriaged: undefined as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any, - shareWarningOpen: false as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any, - configSnapshot: undefined as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any, - sessionSnapshot: undefined as any, - // eslint-disable-next-line @typescript-eslint/no-explicit-any, - sessionSpec: undefined as any, + sessionTriaged: undefined as SessionTriagedInfo | undefined, + configSnapshot: undefined as Record | undefined, + sessionSnapshot: undefined as Record | undefined, + sessionSpec: undefined as Record | undefined, + blankSession: false, runtimePlugins: [] as PluginRecord[], sessionPlugins: [] as PluginRecord[], sessionError: undefined as unknown, @@ -114,21 +113,17 @@ const SessionLoader = types setSessionPlugins(plugins: PluginRecord[]) { self.sessionPlugins = plugins }, - setConfigSnapshot(snap: unknown) { + setConfigSnapshot(snap: Record) { self.configSnapshot = snap }, setBlankSession(flag: boolean) { self.blankSession = flag }, - setSessionTriaged(args?: { - snap: unknown - origin: string - reason: { url?: string }[] - }) { + setSessionTriaged(args?: SessionTriagedInfo) { self.sessionTriaged = args }, - setSessionSnapshotSuccess(snap: unknown) { + setSessionSnapshotSuccess(snap: Record) { self.sessionSnapshot = snap }, })) @@ -254,6 +249,7 @@ const SessionLoader = types async fetchSharedSession() { const defaultURL = 'https://share.jbrowse.org/api/v1/' const decryptedSession = await readSessionFromDynamo( + // @ts-expect-error `${readConf(self.configSnapshot, 'shareURL', defaultURL)}load`, self.sessionQuery || '', self.password || '', diff --git a/products/jbrowse-web/src/components/ConfigWarningDialog.tsx b/products/jbrowse-web/src/components/ConfigWarningDialog.tsx index e6f33b6597..53799c0ecb 100644 --- a/products/jbrowse-web/src/components/ConfigWarningDialog.tsx +++ b/products/jbrowse-web/src/components/ConfigWarningDialog.tsx @@ -11,6 +11,10 @@ import factoryReset from '../factoryReset' import { SessionLoaderModel } from '../SessionLoader' import WarningIcon from '@mui/icons-material/Warning' +import { + PluginDefinition, + pluginDescriptionString, +} from '@jbrowse/core/PluginLoader' function ConfigWarningDialog({ onConfirm, @@ -19,16 +23,10 @@ function ConfigWarningDialog({ }: { onConfirm: () => void onCancel: () => void - reason: { url: string }[] + reason: PluginDefinition[] }) { return ( - + @@ -36,7 +34,7 @@ function ConfigWarningDialog({ unknown plugins:
    {reason.map(r => ( -
  • URL: {r.url}
    • +
  • {pluginDescriptionString(r)}
    • ))}
Please ensure you trust the source of this link. @@ -65,10 +63,11 @@ export default function ConfigTriaged({ loader: SessionLoaderModel handleClose: () => void }) { - return ( + const { sessionTriaged } = loader + return sessionTriaged ? ( { - const session = JSON.parse(JSON.stringify(loader.sessionTriaged.snap)) + const session = JSON.parse(JSON.stringify(sessionTriaged.snap)) await loader.fetchPlugins(session) loader.setConfigSnapshot({ ...session, id: nanoid() }) handleClose() @@ -77,7 +76,7 @@ export default function ConfigTriaged({ await factoryReset() handleClose() }} - reason={loader.sessionTriaged.reason} + reason={sessionTriaged.reason} /> - ) + ) : null } diff --git a/products/jbrowse-web/src/components/Loader.tsx b/products/jbrowse-web/src/components/Loader.tsx index f0eb423ea4..f1a3b4274d 100644 --- a/products/jbrowse-web/src/components/Loader.tsx +++ b/products/jbrowse-web/src/components/Loader.tsx @@ -14,13 +14,16 @@ import '@fontsource/roboto' import Loading from './Loading' import JBrowse from './JBrowse' import factoryReset from '../factoryReset' -import SessionLoader, { SessionLoaderModel } from '../SessionLoader' +import SessionLoader, { + SessionLoaderModel, + SessionTriagedInfo, +} from '../SessionLoader' import StartScreenErrorMessage from './StartScreenErrorMessage' import PluginManager from '@jbrowse/core/PluginManager' import { createPluginManager } from '../createPluginManager' -const ConfigTriaged = lazy(() => import('./ConfigWarningDialog')) -const SessionTriaged = lazy(() => import('./SessionWarningDialog')) +const ConfigWarningDialog = lazy(() => import('./ConfigWarningDialog')) +const SessionWarningDialog = lazy(() => import('./SessionWarningDialog')) const StartScreen = lazy(() => import('./StartScreen')) export function Loader({ @@ -67,61 +70,78 @@ export function Loader({ return } +const SessionTriaged = observer(function ({ + sessionTriaged, + loader, +}: { + loader: SessionLoaderModel + sessionTriaged: SessionTriagedInfo +}) { + return ( + }> + {sessionTriaged?.origin === 'session' ? ( + loader.setSessionTriaged(undefined)} + /> + ) : ( + loader.setSessionTriaged(undefined)} + /> + )} + + ) +}) + +const PluginManagerLoaded = observer(function ({ + pluginManager, +}: { + pluginManager: PluginManager +}) { + const { rootModel } = pluginManager + return !rootModel?.session ? ( + }> + + + ) : ( + + ) +}) + const Renderer = observer(function ({ loader, }: { loader: SessionLoaderModel }) { - const { configError, ready, shareWarningOpen } = loader + const { configError, ready, sessionTriaged } = loader const [pluginManager, setPluginManager] = useState() const [error, setError] = useState() useEffect(() => { + let pm: PluginManager | undefined try { - if (!ready || shareWarningOpen) { + if (!ready) { return } - const pm = createPluginManager(loader) + pm = createPluginManager(loader) setPluginManager(pm) } catch (e) { console.error(e) setError(e) } - }, [loader, ready, shareWarningOpen]) - if (configError || error) { - return - } + }, [loader, ready]) - if (loader.sessionTriaged) { - return ( - }> - {loader.sessionTriaged.origin === 'session' ? ( - loader.setSessionTriaged(undefined)} - /> - ) : ( - loader.setSessionTriaged(undefined)} - /> - )} - - ) - } - if (pluginManager) { - return !pluginManager.rootModel?.session ? ( - }> - - - ) : ( - - ) + const err = configError || error + if (err) { + return + } else if (sessionTriaged) { + return + } else if (pluginManager) { + return + } else { + return } - return }) const LoaderWrapper = ({ initialTimestamp }: { initialTimestamp: number }) => { diff --git a/products/jbrowse-web/src/components/SessionWarningDialog.tsx b/products/jbrowse-web/src/components/SessionWarningDialog.tsx index 3e1dead989..3dd326dea2 100644 --- a/products/jbrowse-web/src/components/SessionWarningDialog.tsx +++ b/products/jbrowse-web/src/components/SessionWarningDialog.tsx @@ -10,6 +10,10 @@ import { nanoid } from '@jbrowse/core/util/nanoid' import { SessionLoaderModel } from '../SessionLoader' import WarningIcon from '@mui/icons-material/Warning' +import { + PluginDefinition, + pluginDescriptionString, +} from '@jbrowse/core/PluginLoader' function SessionWarningDialog({ onConfirm, @@ -18,22 +22,17 @@ function SessionWarningDialog({ }: { onConfirm: () => void onCancel: () => void - reason: { url: string }[] + reason: PluginDefinition[] }) { return ( - + This link contains a session that has the following unknown plugins:
    {reason.map(r => ( -
  • URL: {r.url}
    • +
  • {pluginDescriptionString(r)}
    • ))}
Please ensure you trust the source of this session. @@ -62,10 +61,11 @@ export default function SessionTriaged({ loader: SessionLoaderModel handleClose: () => void }) { - return ( + const { sessionTriaged } = loader + return sessionTriaged ? ( { - const session = JSON.parse(JSON.stringify(loader.sessionTriaged.snap)) + const session = JSON.parse(JSON.stringify(sessionTriaged.snap)) // second param true says we passed user confirmation await loader.setSessionSnapshot({ ...session, id: nanoid() }, true) @@ -75,7 +75,7 @@ export default function SessionTriaged({ loader.setBlankSession(true) handleClose() }} - reason={loader.sessionTriaged.reason} + reason={sessionTriaged.reason} /> - ) + ) : null } diff --git a/products/jbrowse-web/src/createPluginManager.ts b/products/jbrowse-web/src/createPluginManager.ts index ba10b844c1..7edf087af1 100644 --- a/products/jbrowse-web/src/createPluginManager.ts +++ b/products/jbrowse-web/src/createPluginManager.ts @@ -45,6 +45,7 @@ export function createPluginManager(self: SessionLoaderModel) { { pluginManager }, ) + // @ts-expect-error if (!self.configSnapshot?.configuration?.rpc?.defaultDriver) { const { rpc } = rootModel.jbrowse.configuration rpc.defaultDriver.set('WebWorkerRpcDriver') @@ -67,6 +68,7 @@ export function createPluginManager(self: SessionLoaderModel) { } else if (self.sessionSnapshot) { rootModel.setSession(self.sessionSnapshot) } else if (self.sessionSpec) { + // @ts-expect-error afterInitializedCb = loadSessionSpec(self.sessionSpec, pluginManager) } else if (rootModel.jbrowse.defaultSession?.views?.length) { rootModel.setDefaultSession() diff --git a/products/jbrowse-web/src/tests/AssemblySelectorImportForm.test.tsx b/products/jbrowse-web/src/tests/AssemblySelectorImportForm.test.tsx index 9ec57fcf67..9e5b09abce 100644 --- a/products/jbrowse-web/src/tests/AssemblySelectorImportForm.test.tsx +++ b/products/jbrowse-web/src/tests/AssemblySelectorImportForm.test.tsx @@ -9,7 +9,7 @@ beforeEach(() => { doBeforeEach() }) -const delay = { timeout: 10000 } +const delay = { timeout: 20000 } test('nav to volvox2', async () => { const { getInputValue, findByText } = await doSetupForImportForm() @@ -31,5 +31,5 @@ test('select misc', async () => { const { getInputValue, findByText } = await doSetupForImportForm() fireEvent.mouseDown(await findByText('volvox')) fireEvent.click(await findByText('misc')) - await waitFor(() => expect(getInputValue()).toBe('t1')) + await waitFor(() => expect(getInputValue()).toBe('t1'), delay) }, 30000) diff --git a/products/jbrowse-web/src/tests/Loader.test.tsx b/products/jbrowse-web/src/tests/Loader.test.tsx index 205e6d6ae1..e909158ec9 100644 --- a/products/jbrowse-web/src/tests/Loader.test.tsx +++ b/products/jbrowse-web/src/tests/Loader.test.tsx @@ -124,10 +124,10 @@ test('approves sessionPlugins from plugin list', async () => { // minimal session, // {"session":{"id":"xSHu7qGJN","name":"test","sessionPlugins":[{"url":"https://unpkg.com/jbrowse-plugin-msaview/dist/jbrowse-plugin-msaview.umd.production.min.js"}]}} test('pops up a warning for evil plugin in sessionPlugins', async () => { - const { findByTestId } = render( + const { findByText } = render( , ) - await findByTestId('session-warning-modal') + await findByText(/Warning/, {}, delay) }, 20000) test('can use config from a url with nonexistent share param ', async () => { diff --git a/products/jbrowse-web/src/util.ts b/products/jbrowse-web/src/util.ts index ff7b0489c0..8390471627 100644 --- a/products/jbrowse-web/src/util.ts +++ b/products/jbrowse-web/src/util.ts @@ -142,7 +142,7 @@ export function addRelativeUris(config: Config, base: URL) { } } -interface Root { +export interface Root { configuration?: Config }