-
Notifications
You must be signed in to change notification settings - Fork 114
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 ESM bundle for Kedro-Viz #2268
Conversation
Signed-off-by: ravi_kumar_pilla <[email protected]>
this is awesome <3 , few questions around
I'm asking out of curiosity. We can continue improving our bundle sizes iteratively as we move forward :D |
Thanks @ravi-kumar-pilla ! I tested this HTML with the bundle and it worked 👏🏼 <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Kedro-Viz</title>
</head>
<body>
<div id="kedro-viz-1234" style="height: 600px"></div>
<script src="https://cdn.jsdelivr.net/npm/[email protected]/umd/react.production.min.js"></script>
<!-- script src="https://cdn.jsdelivr.net/npm/[email protected]/umd/react-dom.production.min.js"></script -->
<script type="module">
import { KedroViz, createRoot } from './kedro-viz.production.mjs';
const viz_container = document.getElementById('kedro-viz-1234');
const options = {
display: {
expandPipelinesBtn: false,
exportBtn: false,
globalNavigation: false,
labelBtn: false,
layerBtn: false,
metadataPanel: true,
miniMap: false,
sidebar: false,
zoomToolbar: false,
},
expandAllPipelines: false,
behaviour: {
reFocus: false,
},
theme: 'dark',
};
fetch('spaceflights.mock.json')
.then((response) => response.json())
.then((json) => {
if (createRoot && viz_container) {
const viz_root = createRoot(viz_container);
viz_root.render(
React.createElement(KedroViz, {
data: json,
options: options,
})
);
}
})
.catch((error) => console.error('Error loading data:', error));
</script>
</body>
</html>
|
Frankly I was missing 2 lines in the config which says
Yes, when I was going thru this over the internet everyone said esm bundles are in kbs but it is the same for umd with additional optimizations (which I did earlier). ESM provides optimizations by default. I do not see any difference between UMD + additional optimization and ESM.
We can put them in external to make the bundle size small. I did that and it went to 1.3MB. I discussed this with @jitu5 and after testing with other versions of React, our bundle did not work. So we decided to bundle everything together. The advantage of doing this is, if any user wants to run viz directly in HTML as @astrojuanlu did, they just need to refer to 1 script. But if we make them external, they need to install external dependencies separately which will increase the complexity of using the bundle and also it might not work (for example React19 does not work). The disadvantage is it increases the bundle size. For our current use case, I think it is better we go with a unified bundle and see if this creates any issues of users complaining of slowness.
Yes, I think UMD and ESM take the same space. Only difference I saw was ESM provides optimization like tree shaking by default with modern module syntax (import/export vs require)
For sure. This would be a starting point and we can improve on the size by examining our node_modules. Thank you |
Hi @astrojuanlu , Glad it worked :D
Yes, it is expected.
We need our bundle to be hosted somewhere which can be served via CDN. I could find 3 places where we can have this -
As mentioned in the reply above - #2268 (comment) Thank you |
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.
thanks Ravi!! :) let's merge! ⭐
Signed-off-by: ravi_kumar_pilla <[email protected]>
Thanks for the clarifications! My only last comment is that maybe we can indeed publish the bundle to GitHub Releases without having to commit the bundle to the Other than that, I'll let one more engineer review this 👍🏼 |
I tried that @astrojuanlu uploading the bundle as an artifact in the github release. But I was unable to refer it via CDN like - https://cdn.jsdelivr.net/gh/kedro-org/[email protected]/esm/kedro-viz.production.mjs. I do see some articles suggesting that. Let me try one more time. If anyone did this before, please let me know @rashidakanchwala @ankatiyar @SajidAlamQB . Thanks |
I tried publishing individual files in the release assets but these are not served on jsdelivr or other CDN. Only files that are committed can be served. I used GitHub native url but it is blocked due to CORS and not suitable for modules. I think we can go with the current approach and modify in future if there is a better alternative @astrojuanlu Thank you |
Signed-off-by: ravi_kumar_pilla <[email protected]>
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.
Thanks @ravi-kumar-pilla!
Description
Add an ESM Kedro-Viz bundle to be used directly in a html document.
Development notes
make version
, i.e., add a step to update this folder when we do a new releaseQA notes
make version VERSION=10.3.0
ornpm run build:esm
. Discard all the changes after testingChecklist
RELEASE.md
file