-
Notifications
You must be signed in to change notification settings - Fork 1k
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 job metrics per invocation #19048
Conversation
e41e3cb
to
42e56ff
Compare
42e56ff
to
28975da
Compare
28975da
to
fad5ae5
Compare
fad5ae5
to
6b94267
Compare
This is very cool! |
9916da2
to
b1691c5
Compare
"vega": "^5.30.0", | ||
"vega-embed": "^6.26.0", | ||
"vega-lite": "^5.21.0", |
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.
Since this is only needed in a few places, and quite a large package, could we load this on demand?
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.
I suppose if you ask then it is possible 😃 ? I don't know how to do that though.
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.
I can push a commit if you like
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!
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.
To explain what's happening here, to anyone curious:
- the vega libs were added to "modulesExcludedFromLibs" so it's not bundled into the libs bundle. This adds these libs to their own bundles.
- type imports were changes from
import { type ...}
toimport type { ... }
. These are not equivalent. The second one let's typescript completely remove the import line, while the first keeps an empty import, in case of import side effects. This would break on demand loading - Finally the component is imported as
const VegaWrapper = () => import("./VegaWrapper.vue");
. This is a way to lazy load vue components and all it's related code (the vega library in this case). The component can be used like before
Thanks @mvdbeek and @ElectronicBlueberry ... great feature! |
This PR was merged without a "kind/" label, please correct. |
Allows quickly summarizing runtime, memory usage, cores etc. per invocation:
How to test the changes?
(Select all options that apply)
License