[Bug] Rendering does not work on React 18

[garrettjstevens]

JBrowse recently updated to React 18, which has caused the display rendering to not work. We need to see if it's something we can fix on our side easily, and if not, then discuss with JBrowse team possibly reverting that update for now (see this comment).

Here is the text of the error:

ERROR
Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
    at checkForNestedUpdates (http://localhost:3000/static/js/bundle.js:38494:15)
    at scheduleUpdateOnFiber (http://localhost:3000/static/js/bundle.js:36930:7)
    at dispatchSetState (http://localhost:3000/static/js/bundle.js:30085:11)
    at onEmpty (http://localhost:3000/static/js/vendors-node_modules_gmod_trix_esm_index_js-node_modules_gmod_vcf_esm_index_js-node_modules_m-832e20.chunk.js:12232:9)
    at http://localhost:3000/static/js/vendors-node_modules_gmod_trix_esm_index_js-node_modules_gmod_vcf_esm_index_js-node_modules_m-832e20.chunk.js:15573:7
    at http://localhost:3000/static/js/vendors-node_modules_gmod_trix_esm_index_js-node_modules_gmod_vcf_esm_index_js-node_modules_m-832e20.chunk.js:15578:7
    at commitHookEffectListMount (http://localhost:3000/static/js/bundle.js:34953:30)
    at commitLayoutEffectOnFiber (http://localhost:3000/static/js/bundle.js:35053:21)
    at commitLayoutMountEffects_complete (http://localhost:3000/static/js/bundle.js:36252:13)
    at commitLayoutEffects_begin (http://localhost:3000/static/js/bundle.js:36241:11)

I've tracked it down to LinearApolloDisplay where we have three calls like this:

Apollo3/packages/jbrowse-plugin-apollo/src/LinearApolloDisplay/components/LinearApolloDisplay.tsx

Lines 89 to 91 in cb7d2ab

	ref={(node) => {
	setCollaboratorCanvas(node)
	}}

where setCollaboratorCanvas is an MST action that sets the canvas in a volatile node on the display's state tree. Even if the state tree doesn't do anything with the canvas node, just setting it is enough to cause this error, and all three instances of storing a ref in the MST model cause this error.

The only other thing I've been able to figure out is that it likely has something to do with the usage of React 18's useSyncExternalStore in mobx-react. React's DevTools tells me that the component keeps re-rendering because of hook 3 here:

[cmdcolin]

in my e.g. LinearReadArcsDisplay I have code like this where i wrap the ref callback with a useCallback:

const cb = useCallback(
    (ref: HTMLCanvasElement) => {
      model.setRef(ref)
    },
    // eslint-disable-next-line react-hooks/exhaustive-deps
    [model, width, height],
  )

  // note: the position absolute below avoids scrollbar from appearing on track
  return (
    <canvas
      data-testid="arc-canvas"
      ref={cb}
      style={{ width, height, position: 'absolute' }}
      width={width * 2}
      height={height * 2}
    />
  )

this has, in my experience, reduced the amount of times the ref gets destroyed and recreated.

however, I am wondering if that is the true cause of the error. the maximum depth exceeded is often due to Creating an observable e.g. during rendering. In the react 18 PR, we had an example of this with the connection code (see https://github.com/GMOD/jbrowse-components/pull/3502/files#diff-faaefea17f30fdf77887b47ad504682c377319f693394238f7e615bcc48b6bd8R43)

mobx discussion thread mobxjs/mobx#3728 (comment)

also, not sure if related, but with this code:

  // eslint-disable-next-line unicorn/consistent-destructuring
  model.tabularEditor.showPane()

instead of calling that during rendering, I would maybe move that to a useEffect?

would be happy to pair on this if interested

[garrettjstevens]

I tried useCallback and also tried passing the context to the model instead of the ref, but both resulted in a lot of flickering when scrolling the display. I ended up being able to resolve it like this using one of the suggestions in that mobx thread:

return (
  <canvas
    ref={async (node: HTMLCanvasElement) => {
      await Promise.resolve()
      setCollaboratorCanvas(node)
    }}
    width={width}
    height={height}
  />
)

[cmdcolin]

nice! I don't think there's anything wrong with that. I'd have to think about why this is happening in your case but that seems like a good workaround