-
-
Notifications
You must be signed in to change notification settings - Fork 182
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 an option to generate async component #126
Conversation
- AsyncComponent.react.js: The exported component (inside template dir) - {{component_name}}.react.js: The real component - LazyLoader.js: The intermediate lazy loading function
- Always deletes the cookiecutter_templates - Delete fragments and lazyloader.js when async not used
@plotly/dash-core Let me know when you have bandwidth to review this |
@xhlulu I'll can review this, just give me a few days to write the documentation in dash-docs. |
name(module, chunks, cacheGroupKey) { | ||
return `${cacheGroupKey}-${chunks[0].name}`; | ||
} | ||
} |
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.
Adding the "shared" async chunk for code that repeats across multiple async chunks here would be good as this will have already solved the problem for component developers.
https://github.com/plotly/dash-core-components/blob/dev/webpack.config.js#L124
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.
It would still require the developer to add the entry in __init__.py
but maybe we can make a comment to that effect there or in the README instead of leaving them to fend off for themselves :)
@@ -0,0 +1,49 @@ | |||
import React, {Component} from 'react'; | |||
import PropTypes from 'prop-types'; | |||
import { {{cookiecutter.component_name}} as RealComponent } from '../LazyLoader'; |
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.
We could add but not use the asyncDecorator here (comment out correct usage) like in https://github.com/plotly/dash-table/blob/dev/src/dash-table/dash/DataTable.js#L27 and just leave a comment to use it for components that have side-effects (e.g. prop values calculated from other props or that need to render in order to update/process their props)
For example: https://github.com/plotly/dash-table/blob/dev/src/dash-table/dash/DataTable.js#L27
This is an edge case but not one I'd like component developers to have to find out about. It's not terribly bad either if they do decide to turn it on even if not required.. worst case.
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.
@xhlulu Look good, just a few comments. @jdamiba For your 👀 too just so you remain aware of all aspects of async :)
Also you'll need to update the MANIFEST to include the new files. Maybe a glob similar too:
include {{cookiecutter.project_shortname}}/async-*.js
include {{cookiecutter.project_shortname}}/async-*.js.map
@Marc-Andre-Rivet Thanks for the review - very helpful comments. I've made the changes to webpack/readme/manifest.in. For
I've added "dash-component-plugins" as a dependency. However I wonder if it might be better to more thoroughly explain why |
@xhlulu Fair point. Sadly unless I'm wrong we don't have such an entry in the docs at this point. |
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.
💃
@jdamiba Hey can I merge this? |
@xhlulu This is awesome! 🥇 When will the docs be ready? It would be great to do an announcement on the forum too - this topic came up recently: https://community.plotly.com/t/dash-how-to-implement-chunked-loading-of-custom-components/55366/6. Perhaps there could be also be instructions on how to add the async loading to an existing project. Or would you recommend re-creating the project with the new version of the |
Closes #122
When
use_async
is set to True, new files will be generated:src/lib/fragments/Component.react.js
: This contains the real componentsrc/lib/LazyLoader.js
: This exports a function that lazy loads the component from the async chunkAnd the following modified:
src/lib/components/Component.react.js
: will import from lazy loader and usereact.suspense
instead of the usual pattern.dash_component/__init__.py
: Will load thejs
andjs.map
files defined in listasync_resources
The
webpack.config.js
will always use the async optimization and package.json will always have the packages required for async as dev dependencies. This avoids having to re-add those packages later on if the developer decides to add async.