-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add async loading to reduce package size #29
Conversation
Enables the use of the import function inside react for React.lazy
This exports lazy components which are imported in the respective component scripts. This is needed in order for dash to detect the component and add it to _imports_.py
@alexcjohnson @Marc-Andre-Rivet I pushed some commits here using Suspense and lazy. Let me know if that's what I'm supposed to do; if there's no problem I'll do the same thing for the other components. |
@Marc-Andre-Rivet Thanks for the advice. I had a pair-coding session with Joe and I was succefully in wrapping one of the components ( |
I'm rerolling back to before adding async so I can explore the single-chunk async apporach (load all of vtk or none). I'm saving the state of this branch here: https://github.com/plotly/dash-vtk/tree/lazy-loading-archive-1 |
* tried stuff * ok let's go back to where it was before * tried some more stuff that seems to make it work? * set async to true * Add usage simple, simplify import structure * Generalized the async component with a async component builder function Temporarily commenting out the other components that were causing problem * Expand async to more components; usage.py now runs but still white square * Improve naming for clarity * Replace all with async react vtk * Add more components to async-react-vtk, make builder more verbose * Wrap Mesh with lazy/suspense * npm run build * Fix incorrect source map * Add tests for docs tutorials * Move the async import call inside the builder function * npm run build * Tests: Increase sleep time for demo, decrease for tutorials * fix typo in usage-vtk-cfd * update react-vtk-js to 1.4.1 * update generated files * trigger circleci * update react-vtk-js to 1.4.2 * update generated files * trigger ci * Apply black to all tests and usage files * Remove dummy app * Update code to be eslint compliant * run build Co-authored-by: Sebastien Jourdain <sebastien.jourdain@kitware.com>
@alexcjohnson @Marc-Andre-Rivet @jdamiba This PR is ready to be reviewed! I think the async should work now, and all the problem on the vtk side have been resolved by @jourdain . Feel free to take a look at the percy snapshot to validate the result. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me as well. 💃🏿
I tested this locally by running the usage.py
example both with and without dash-vtk
components in the app layout, and can confirm that the async ReactVTK.js
bundle is only served when dash-vtk
components are in the layout. When the library is simply imported into an app, only the thin shell is loaded.
The same holds true at https://dash-docs-pr-1091.herokuapp.com/ and https://dash-docs-pr-1091.herokuapp.com/vtk, which are a version of the dash-docs
which point to this branch of the dash-vtk
library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @Marc-Andre-Rivet @jdamiba @jourdain! Merging this now. |
closes #28
More information in PR #40
Screenshot of network when
dash_vtk
is imported but not used:Pasting some explanation of the
asyncComponentBuilder
function that I shared with @Marc-Andre-Rivet