Try to use suspense/lazy

[xhluca]

What was tried

Over the weekend I was able to incorporate React.lazy and React.Suspense for each of the components. To do that, I created some modules:

• src/lib/ReactVTK.js: This module simply imports and export the react-vtk-js library, which is needed in order to use the webpackChunkName functionality added in webpack and create a async-ReactVTK.js file.
• src/lib/AsyncReactVTK.js: This module contains a AsyncComponentBuilder function. It creates an async function that returns a module-like object, which we can then pass to React.lazy without causing errors. Later on (inside the components definition), it gets rendered correctly by React.Suspense (in theory only). This is the component builder function I used:
  

  dash-vtk/src/lib/AsyncReactVTK.js

  Lines 5 to 15 in 4cd17de

  	const AsyncComponentBuilder = name => async () => {
  	const ReactVTK = await AsyncReactVTK;
  	// console.log("ReactVTK", ReactVTK);
  	// window.ReactVTK = ReactVTK;
  	const reactComponent = ReactVTK.default[name];
  	// We need to trick React.lazy into thinking we are giving
  	// the output of an import("my-module.js") Promise.
  	const fakeModule = {default: reactComponent};
  	return fakeModule;
  	}

  
  (I'm happy to hop on a call to explain how I got to this).

I also brought some modifications to the files:

• webpack.config.js: Some changes in lazy-loading (see Add async loading to reduce package size #29), no change here. Mostly code for chunk splitting in order to allow async.
• package.json: Some changes in lazy-loading (see Add async loading to reduce package size #29), no change here. Added a few packages that were necessary to use the new webpack optimization.
• src/lib/components: Each component here was updated such that the import came from AsyncReactVTK.js instead of react-vtk-js, and also the component outputted is wrapped with a React.Suspense.
• dash_vtk/__init__.py: The following elements were added to _js_dist in order to load the async fragment:

{
    'relative_package_path': 'async-ReactVTK.js',
    'external_url': 'https://unpkg.com/{0}@{2}/{1}/async-ReactVTK.js'.format(package_name, __name__, __version__),
    'namespace': package_name,
    'async': True
},
{
    'relative_package_path': 'async-ReactVTK.js.map',
    'external_url': 'https://unpkg.com/{0}@{2}/{1}/async-ReactVTK.js.map'.format(package_name, __name__, __version__),
    'namespace': package_name,
    'dynamic': True
}

What worked

Rendering the simplest possible app, i.e. an empty dash_vtk.View worked with this new approach; no error was found in the console. Moreover, I tried with usage.py, which yielded this result:

So it partially worked in the sense that the cone and the starwars obj were rendered correctly, but not the colored pane in the back.
Moreover, very simple demos like the synthetic-volume-rendering and t05_reader seemed to work as well. Additionally, some apps seem to load after a few seconds, but the intiial view is completely broken (you have to zoom out and drag it around to have the correct view; this didn't happen before):

What didn't work

Most of the apps (except maybe two) have problems in some way or another. In many cases, the app is completely broken and can't be used at all. Here are two examples of issues with the apps:

• slice-rendering: Getting this error in the console:

  Uncaught TypeError: Failed to execute 'bindTexture' on 'WebGL2RenderingContext': parameter 2 is not of type 'WebGLTexture'.
  

• volume-rendering: Getting this error in the console:

  dash_renderer.v1_9_1m1618031064.dev.js:100499 TypeError: Failed to execute 'bindTexture' on 'WebGL2RenderingContext': parameter 2 is not of type 'WebGLTexture'.
   at Object.Ln.e.bind (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.Ln.e.activate (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.Io.e.renderPieceDraw (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.Io.e.renderPiece (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Io.e.volumePass (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.e.apply (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.e.traverse (async-ReactVTK.v0_0_7m1618261439.js:2)
   at e.traverseVolumePass (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.e.traverse (async-ReactVTK.v0_0_7m1618261439.js:2)
   at Object.e.traverse (async-ReactVTK.v0_0_7m1618261439.js:2)
  

Not actual async loading

Moreover, I also noticed that when I import dash_vtk inside an app that doesn't use dash_vtk at all, it will still load async-ReactVTK.js, which is the new file with 500kb:

So although we have correctly implemented async, something inside the react-vtk-js initialization code forces this file to still be loaded; unfortunately i do not have insight on that.

UPDATE: In f6b8c8a, I moved the the import('./ReactVTK') call inside the builder function and that seemed to have resolved that issue. Furthermore, it seems like the colored pane in usage.py works correctly now.

Next step

So far, @jdamiba and I successfully achieved the suggested approach from a React perspective by using React.lazy and React.Suspense; we also used the same approach as dash-core-components for loading the components (install extra packages in package.json, add custom webpack commands, use webpackChunkName in the import calls, etc), albeit with minor modifications through the AsyncComponentBuilder function, which I highlight doubt is the cause of the problems here.

As a next step, it would be amazing if @jourdain takes a look at this branch (specifically, the new and modified files in src/lib) and decide whether more development will be needed (in react-vtk-js or in dash-vtk's JS code) in order to achieve the async functionality that we added in this branch.

Furthermore, I think it'd be beneficial if @alexcjohnson @Marc-Andre-Rivet reviews our tentative approach to determine if there's a simple fix that @jdamiba and I missed, or if we are doing something wrong.

[jourdain]

Is it working? You will have to complete the missing set of classes (CellData...)
It seems reasonable.

[xhluca]

@jourdain I just updated the initial comment with a write-up of what worked, what didn't work, and what's next.

[jourdain]

Thanks @xhlulu for the detailed update. I'll look into it tomorrow to better handle out of order pipeline update. (The cause of the issues you are seeing of bad initial reset camera and no field update (color))

Regarding your following comment

So although we have correctly implemented async, something inside the react-vtk-js initialization code forces this file to still be loaded; unfortunately i do not have insight on that.

I don't think it has anything to do with react-vtk-js which is part of your async dependency. This come from the way you use the async part as dependency.

My wild guess is that we are still loading the dash_vtk.js library too eagerly which then trigger its async dependency right away. I'm not sure how to prevent such loading if we don't use it.

[xhluca]

I don't think it has anything to do with react-vtk-js which is part of your async dependency. This come from the way you use the async part as dependency.

Yeah it was because I called import("./ReactVTK") outside of the component builder function. I've moved it inside and it seems to have resolved the issue. I added an update in the initial comment.

[jourdain]

I think you can merge it like that (without publishing a new version just yet). I just need to make react-vtk-js more flexible in the binding order.

[jourdain]

LGTM

[xhluca]

@jourdain I prefer waiting for the new react-vtk-js to be released so i can try it on this branch before merging. this way if there's error in the way I implemented async we can catch it without having to do another PR

[jourdain]

I'm on it... But I'm trying to figure out what is the best way to handle the initialization order issue and how to test/validate it inside react-vtk-js context.

[jourdain]

@xhlulu can you trigger the ci?