Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Allow projects to use dcc source directly #723

Merged
merged 6 commits into from
Dec 19, 2019
Merged

Allow projects to use dcc source directly #723

merged 6 commits into from
Dec 19, 2019

Conversation

Marc-Andre-Rivet
Copy link
Contributor

  • add /lib with babel transpiled js code to published package
  • fix deps vs devDeps for source usage
  • Rework LazyLoader to allow tree shaking of async resources

…rojects (automate on publish)

- update LazyLoader to allow for tree-shaking
- declare the project side-effect-free
@@ -12,6 +12,7 @@
"homepage": "https://github.com/plotly/dash-core-components",
"main": "dash_core_components/dash_core_components.min.js",
"scripts": {
"prepublishOnly": "rm -rf lib && babel src --out-dir lib --copy-files",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On prepublish, transpile all source to /lib

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include the css files too! :)

@@ -48,9 +49,11 @@
"react-markdown": "^4.0.6",
"react-select-fast-filter-options": "^0.2.3",
"react-virtualized-select": "^3.1.3",
"react-resize-detector": "^4.2.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Required for 3rd party build inclusion

"uniqid": "^5.0.3"
},
"devDependencies": {
"@babel/cli": "^7.4.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allows using babel from the command line

@@ -88,8 +90,10 @@
"webpack-serve": "^2.0.3"
},
"files": [
"/dash_core_components/*{.js,.map}"
"/dash_core_components/*{.js,.map}",
"/lib/**"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include in the published package

@@ -112,7 +112,7 @@ module.exports = (env, argv) => {
plugins: [
new WebpackDashDynamicImport(),
new webpack.SourceMapDevToolPlugin({
filename: '[name].js.map',
filename: '[file].map',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

caused the entrypoint sourcemap to be main.js.map instead of dash_core_components.min.js.map

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review December 18, 2019 22:02
import(/* webpackChunkName: "plotlyjs" */ 'plotly.js').then(({ default: Plotly }) => {
window.Plotly = Plotly;
return Plotly;
}));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting it all up for tree shaking reasons..

@@ -0,0 +1,6 @@
export default () => Promise.resolve(window.Plotly ||
Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Dec 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're here, might be interesting to check if window.Plotly.version matches our expected minimal version?, and if not, force loading the one in our bundle.. this will also be useful to protect against old versions of dash-bio dumping an old version of Plotly.js for DCC to use.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 91cd3a9 into dev Dec 19, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the tree-shake branch December 19, 2019 14:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants