Merge pull request #1888 from GMOD/lazyreact

More lazy loading of react components to reduce bundle size

.github/workflows/pull_request_label.yml

@@ -10,7 +10,8 @@ jobs:
       !contains(github.event.pull_request.labels.*.name, 'enhancement') &&
       !contains(github.event.pull_request.labels.*.name, 'bug') &&
       !contains(github.event.pull_request.labels.*.name, 'documentation') &&
-      !contains(github.event.pull_request.labels.*.name, 'internal')
+      !contains(github.event.pull_request.labels.*.name, 'internal') &&
+      !contains(github.event.pull_request.labels.*.name, 'performance')
     steps:
       - uses: actions/labeler@main
         with:

.github/workflows/push.yml

@@ -38,8 +38,8 @@ jobs:
         run: |
           cd website/
           yarn build
-  buildjbrowseweb:
-    name: Build jbrowse-web on node 10.x and ubuntu-latest
+  buildwholerepo:
+    name: Build whole repo on node 10.x and ubuntu-latest
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
@@ -53,6 +53,24 @@ jobs:
         run: yarn build
       - name: Test build
         run: BUILT_TESTS=1 yarn built-test-ci
+
+  buildjbrowseweb:
+    name: Build only jbrowse-web and upload to s3 on node 10.x and ubuntu-latest
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Use Node.js 10.x
+        uses: actions/setup-node@v1
+        with:
+          node-version: 10.x
+      - name: Install deps (with cache)
+        uses: bahmutov/npm-install@v1
+      - name: Build project
+        run: |
+          echo $RELEASE_VERSION
+          cd products/jbrowse-web/
+          NODE_OPTIONS='--max-old-space-size=6500' yarn build
+          cd ../../
       - name: Configure AWS credentials
         uses: aws-actions/configure-aws-credentials@v1
         with:

packages/core/BaseFeatureWidget/BaseFeatureDetail.tsx

@@ -440,21 +440,6 @@ function isEmpty(obj: Record<string, unknown>) {
   return Object.keys(obj).length === 0
 }
 
-export const BaseFeatureDetails = observer((props: BaseInputProps) => {
-  const { model } = props
-  const { featureData } = model
-
-  if (!featureData) {
-    return null
-  }
-  const feature = JSON.parse(JSON.stringify(featureData))
-
-  if (isEmpty(feature)) {
-    return null
-  }
-  return <FeatureDetails model={model} feature={feature} />
-})
-
 export const FeatureDetails = (props: {
   model: IAnyStateTreeNode
   feature: SimpleFeatureSerialized & { name?: string; id?: string }
@@ -463,23 +448,17 @@ export const FeatureDetails = (props: {
   formatter?: (val: unknown, key: string) => JSX.Element
 }) => {
   const { omit = [], model, feature, depth = 0 } = props
-  const { name, id, type, subfeatures } = feature
-  const displayName = (name || id) as string | undefined
-  const ellipsedDisplayName =
-    displayName && displayName.length > 20 ? '' : displayName
+  const { name, id, type = '', subfeatures } = feature
+  const slug = name || id || ''
+  const shortName = slug.length > 20 ? `${slug}...` : slug
+  const title = `${shortName}${type ? ` - ${type}` : ''}`
   const session = getSession(model)
   const defSeqTypes = ['mRNA', 'transcript']
   const sequenceTypes =
     getConf(session, ['featureDetails', 'sequenceTypes']) || defSeqTypes
 
   return (
-    <BaseCard
-      title={
-        ellipsedDisplayName
-          ? `${ellipsedDisplayName} - ${type}`
-          : `${type || ''}`
-      }
-    >
+    <BaseCard title={title}>
       <div>Core details</div>
       <CoreDetails {...props} />
       <Divider />
@@ -518,3 +497,20 @@ export const FeatureDetails = (props: {
     </BaseCard>
   )
 }
+
+const BaseFeatureDetails = observer((props: BaseInputProps) => {
+  const { model } = props
+  const { featureData } = model
+
+  if (!featureData) {
+    return null
+  }
+  const feature = JSON.parse(JSON.stringify(featureData))
+
+  if (isEmpty(feature)) {
+    return null
+  }
+  return <FeatureDetails model={model} feature={feature} />
+})
+
+export default BaseFeatureDetails

packages/core/BaseFeatureWidget/index.test.js

@@ -1,10 +1,10 @@
-import { render } from '@testing-library/react'
 import React from 'react'
+import { render } from '@testing-library/react'
 import { types } from 'mobx-state-tree'
 import { ConfigurationSchema } from '@jbrowse/core/configuration'
 import PluginManager from '../PluginManager'
 import { stateModelFactory } from '.'
-import { BaseFeatureDetails as ReactComponent } from './BaseFeatureDetail'
+import BaseFeatureDetails from './BaseFeatureDetail'
 
 test('open up a widget', () => {
   console.warn = jest.fn()
@@ -20,7 +20,7 @@ test('open up a widget', () => {
     widget: { type: 'BaseFeatureWidget' },
   })
   const { container, getByText } = render(
-    <ReactComponent model={model.widget} />,
+    <BaseFeatureDetails model={model.widget} />,
   )
   model.widget.setFeatureData({
     start: 2,

packages/core/BaseFeatureWidget/index.ts

@@ -26,4 +26,3 @@ export default function stateModelFactory(pluginManager: PluginManager) {
 }
 
 export { configSchema, stateModelFactory }
-export { BaseFeatureDetails as ReactComponent } from './BaseFeatureDetail'

packages/core/CorePlugin.ts

@@ -1,6 +1,9 @@
+import { lazy } from 'react'
+import { configSchema, stateModelFactory } from './BaseFeatureWidget'
 import Plugin from './Plugin'
 import PluginManager from './PluginManager'
 import * as coreRpcMethods from './rpc/coreRpcMethods'
+import WidgetType from './pluggableElementTypes/WidgetType'
 
 /** the core plugin, which registers types that ALL JBrowse applications are expected to need. */
 export default class CorePlugin extends Plugin {
@@ -11,5 +14,17 @@ export default class CorePlugin extends Plugin {
     Object.values(coreRpcMethods).forEach(RpcMethod => {
       pluginManager.addRpcMethod(() => new RpcMethod(pluginManager))
     })
+
+    pluginManager.addWidgetType(() => {
+      return new WidgetType({
+        name: 'BaseFeatureWidget',
+        heading: 'Feature details',
+        configSchema,
+        stateModel: stateModelFactory(pluginManager),
+        ReactComponent: lazy(
+          () => import('./BaseFeatureWidget/BaseFeatureDetail'),
+        ),
+      })
+    })
   }
 }

packages/core/PluginManager.ts

@@ -325,7 +325,9 @@ export default class PluginManager {
           pluggableTypes.push(thing)
         }
       })
-    // try to smooth over the case when no types are registered, mostly encountered in tests
+
+    // try to smooth over the case when no types are registered, mostly
+    // encountered in tests
     if (pluggableTypes.length === 0) {
       console.warn(
         `No JBrowse pluggable types found matching ('${typeGroup}','${fieldName}')`,

packages/core/pluggableElementTypes/ViewType.ts

@@ -2,12 +2,13 @@ import { IAnyModelType, IAnyStateTreeNode } from 'mobx-state-tree'
 import PluggableElementBase from './PluggableElementBase'
 import DisplayType from './DisplayType'
 
-type ViewReactComponent = React.ComponentType<{
+type BasicView = React.ComponentType<{
   // TODO: can we use AbstractViewModel here?
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   model: any
   session?: IAnyStateTreeNode
 }>
+type ViewReactComponent = React.LazyExoticComponent<BasicView> | BasicView
 
 export default class ViewType extends PluggableElementBase {
   ReactComponent: ViewReactComponent

packages/core/pluggableElementTypes/WidgetType.ts

@@ -1,4 +1,4 @@
-import { ComponentType } from 'react'
+import { ComponentType, LazyExoticComponent } from 'react'
 import { IAnyModelType, IAnyStateTreeNode } from 'mobx-state-tree'
 import PluggableElementBase from './PluggableElementBase'
 import { AnyConfigurationSchemaType } from '../configuration/configurationSchema'
@@ -11,7 +11,7 @@ export default class WidgetType extends PluggableElementBase {
   HeadingComponent?: ComponentType<{ model: IAnyStateTreeNode }>
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  ReactComponent: React.FC<any>
+  ReactComponent: LazyExoticComponent<React.FC<any>> | React.FC<any>
 
   stateModel: IAnyModelType
 
@@ -22,7 +22,7 @@ export default class WidgetType extends PluggableElementBase {
     configSchema: AnyConfigurationSchemaType
     stateModel: IAnyModelType
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    ReactComponent: React.FC<any>
+    ReactComponent: LazyExoticComponent<React.FC<any>> | React.FC<any>
   }) {
     super(stuff)
     this.heading = stuff.heading

packages/core/pluggableElementTypes/models/BaseTrackModel.ts

@@ -38,16 +38,6 @@ export function createBaseTrackModel(
         pluginManager.pluggableMstType('display', 'stateModel'),
       ),
     })
-    .volatile(() => ({
-      DialogComponent: undefined as React.FC | undefined,
-      DialogDisplay: undefined as any,
-    }))
-    .actions(self => ({
-      setDialogComponent(dlg?: React.FC, context?: any) {
-        self.DialogComponent = dlg
-        self.DialogDisplay = context
-      },
-    }))
     .views(self => ({
       get rpcSessionId() {
         return self.configuration.trackId

packages/core/ui/App.js

@@ -1,11 +1,6 @@
-/* eslint-disable react/prop-types */
-import React from 'react'
-import AppBar from '@material-ui/core/AppBar'
-import { makeStyles } from '@material-ui/core/styles'
-import Fab from '@material-ui/core/Fab'
+import React, { Suspense } from 'react'
+import { AppBar, Fab, Toolbar, Tooltip, makeStyles } from '@material-ui/core'
 import LaunchIcon from '@material-ui/icons/Launch'
-import Toolbar from '@material-ui/core/Toolbar'
-import Tooltip from '@material-ui/core/Tooltip'
 import { observer } from 'mobx-react'
 import { getEnv } from 'mobx-state-tree'
 import DrawerWidget from './DrawerWidget'
@@ -82,16 +77,22 @@ const useStyles = makeStyles(theme => ({
   },
 }))
 
-function App({ session, HeaderButtons }) {
+const App = observer(({ session, HeaderButtons }) => {
   const classes = useStyles()
   const { pluginManager } = getEnv(session)
-  const { visibleWidget, drawerWidth, minimized, activeWidgets } = session
+  const {
+    visibleWidget,
+    drawerWidth,
+    minimized,
+    activeWidgets,
+    savedSessionNames,
+    name,
+    menus,
+    views,
+  } = session
 
   function handleNameChange(newName) {
-    if (
-      session.savedSessionNames &&
-      session.savedSessionNames.includes(newName)
-    ) {
+    if (savedSessionNames && savedSessionNames.includes(newName)) {
       session.notify(
         `Cannot rename session to "${newName}", a saved session with that name already exists`,
         'warning',
@@ -109,11 +110,19 @@ function App({ session, HeaderButtons }) {
         }`,
       }}
     >
+      {session.DialogComponent ? (
+        <Suspense fallback={<div />}>
+          <session.DialogComponent
+            handleClose={() => session.setDialogComponent(undefined, undefined)}
+            {...session.DialogProps}
+          />
+        </Suspense>
+      ) : null}
       <div className={classes.menuBarAndComponents}>
         <div className={classes.menuBar}>
           <AppBar className={classes.appBar} position="static">
             <Toolbar>
-              {session.menus.map(menu => (
+              {menus.map(menu => (
                 <DropDownMenu
                   key={menu.label}
                   menuTitle={menu.label}
@@ -124,7 +133,7 @@ function App({ session, HeaderButtons }) {
               <div className={classes.grow} />
               <Tooltip title="Rename Session" arrow>
                 <EditableTypography
-                  value={session.name}
+                  value={name}
                   setValue={handleNameChange}
                   variant="body1"
                   classes={{
@@ -143,7 +152,7 @@ function App({ session, HeaderButtons }) {
           </AppBar>
         </div>
         <div className={classes.components}>
-          {session.views.map(view => {
+          {views.map(view => {
             const viewType = pluginManager.getViewType(view.type)
             if (!viewType) {
               throw new Error(`unknown view type ${view.type}`)
@@ -155,14 +164,18 @@ function App({ session, HeaderButtons }) {
                 view={view}
                 onClose={() => session.removeView(view)}
               >
-                <ReactComponent
-                  model={view}
-                  session={session}
-                  getTrackType={pluginManager.getTrackType}
-                />
+                <Suspense fallback={<div>Loading...</div>}>
+                  <ReactComponent
+                    model={view}
+                    session={session}
+                    getTrackType={pluginManager.getTrackType}
+                  />
+                </Suspense>
               </ViewContainer>
             )
           })}
+
+          {/* blank space at the bottom of screen allows scroll */}
           <div style={{ height: 300 }} />
         </div>
       </div>
@@ -189,6 +202,6 @@ function App({ session, HeaderButtons }) {
       <Snackbar session={session} />
     </div>
   )
-}
+})
 
-export default observer(App)
+export default App

packages/core/ui/Drawer.js

@@ -1,8 +1,8 @@
+import React from 'react'
 import Paper from '@material-ui/core/Paper'
 import { makeStyles } from '@material-ui/core/styles'
 import { observer, PropTypes as MobxPropTypes } from 'mobx-react'
 import PropTypes from 'prop-types'
-import React from 'react'
 import ResizeHandle from './ResizeHandle'
 
 const useStyles = makeStyles(theme => ({