-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: add minify configuration to BundleOptions #141
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonmmease
added a commit
to vega/vl-convert
that referenced
this pull request
Oct 5, 2023
Closes #33, cc @joelostblom This PR adds support for converting Vega and Vega-Lite charts to live HTML documents. There is a "bundle" option that controls whether the JavaScript dependencies should be loaded from a CDN, or whether they should be inlined into the resulting HTML file. ### bundle=False When bundle is False, this follows the [Vega Embed directions](https://vega.github.io/vega-lite/usage/embed.html#start-using-vega-lite-with-vega-embed) to load vega, vega-lite, and vega-embed from jsdelivr ### bundle=True When bundle is True, things are a bit more involved. We already inline Vega and several versions of Vega-Lite into the VlConvert executables, so I wanted to avoid including additional copies for the purpose of HTML export. But in order to use the JS deps inlined into VlConvert, they need to be bundled. I found that the [deno_emit](https://github.com/denoland/deno_emit) project provides a Rust crate that uses SWC to bundle JavaScript / TypeScript dependencies, and this ended up working well. One note, the bundled code isn't fully minimized yet, but I have an open PR that will expose SWC's minify option. See denoland/deno_emit#141. Currently, the resulting HTML files start at ~1.6MB, but this will drop to ~1MB when minification is enabled. The bundling process takes about 1.5s on my machine. Because this is pretty slow, I decided to cache the bundle results, so that subsequent HTML exports that use the same Vega-Lite version will be fast (10-20ms). ### Custom JavaScript bundles Additionally, a `javascipt_bundle` Python function is added that can be used to create bundles with custom JavaScript logic that references Vega Embed, Vega, and Vega-Lite. The idea is that Vega-based systems like Altair can use this to build additional integrations. ### Integrations The most immediate application of the HTML export is to remove the altair_viewer dependency in Altair's html export when `inline=True`. We could also use this to add an "html-offline" Altair renderer, though this could result in large notebooks as every individual Chart would be over 1MB. Another use-case I have in mind is to use the `javascript_bundle` function to create offline bundles for Altair's JupyterChart. This is why I added support for the lodash debounce function as well, since this is the only import, in addition to vegaEmbed, that JupyterChart's JavaScript logic uses. The cool thing about this approach is that we can build the offline bundle on the fly (in under 2s) without an internet connection required.
Friendly ping. Does this looks reasonable @dsherret? |
dsherret
changed the title
Add minify configuration to BundleOptions
feat: add minify configuration to BundleOptions
Oct 23, 2023
dsherret
approved these changes
Oct 23, 2023
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.
LGTM. Thanks!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a
minify
option toBundleOptions
and passes the option through toswc::codegen::Config
.Partially based on #63
Thanks!