Vue: Fix out of memory error when using vue-component-meta #28589
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
/code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts: Removed nested object schemas not used in Storybook to reduce memory usage./code/renderers/vue3/src/docs/extractArgTypes.ts: Avoided recursive mapping of object schemas to prevent out-of-memory errors.
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 9601e82. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
|
@larsrickert, I want to express my sincere gratitude for your thorough investigation and solution to this issue. Your approach of addressing the problem within Storybook's processing, rather than limiting vue-component-meta's capabilities, is indeed a much better solution than what I initially proposed. Your work here not only solves the immediate problem but also maintains the full functionality of vue-component-meta, which is crucial for many projects. This fix will be incredibly helpful for teams like mine who are looking to adopt or continue using Storybook for component documentation. Thank you for your dedication to improving Storybook and for fostering such a collaborative environment for problem-solving. Your efforts are truly appreciated! |
| }, | ||
| {} | ||
| ), | ||
| // Storybook does not generate controls for object properties so we don't need to recursively map the object schema here |
There was a problem hiding this comment.
I'm not entirely sure what you mean here. it's true that complex objects won't have proper controls, Storybook does generate controls for simple objects (JSON object controls). However, it doesn't require or use nested schemas. I think you meant controls for the properties of objects rather than props of the object type.
There was a problem hiding this comment.
Yes exactly, I updated the comment so I hope its clearer now :)
|
Could you check the CI failures @larsrickert |
|
Thank you so much @larsrickert! We are waiting eagerly for this release. It will make our static bundle go frow 84 to 4 MB, greatly reducing build time 🥳 |
|
I have tried the new alpha.2 release that included this PR but I am still experiencing the memory issues. Has this solved it for others? I'm wondering if I have something else going on. |
|
@Adamlb Sadly still experiencing the same issue 😢 |
|
@larsrickert It does in fact seem to have fixed the issue in places where I use the The only type still causing memory issues for me now is In any case, thank you for the quick alpha release! 🙏 Let me know if you need more details. |
|
@larsrickert We don't specifically pull in the types in our components yet but we are using shadcn and radix-vue which does. If I give the build 8gb of ram, it does complete, and the most egregious file is full of repeated documentation around event things, so its likely the same thing as ericvel. I will try and determine something more specific, but may not be able to get to it until this evening. |
|
@ericvel @Adamlb Thanks for your feedback! I fixed the out of memory issue only for props and exposed but forgot that you can also have complex types for event and slots (e.g. I created a follow up fix to apply the same fix also for events and slots: #28674 |
Update Storybook to version `8.3.0-alpha.3` which includes a bug fix which reduced the bundle size of our build Storybok from 26MB to 6MB and reduced build time from 37s to 10s. If you are interested, the bug was caused by the `vue-component-meta` package which included ALL schema definitions for props, events, slots and exposed so if e.g. `HTMLElement` was used, it included all nested props, JSDocs etc. although they are not needed/used by Storybook, see [Storybook PR](storybookjs/storybook#28589)
|
Is there a working workaround for this problem? Since our upgrade to storybook I tried using storybook UPDATE: build ist working now with 8.3.0-alpha.8 after setting the following in package.json |
Update Storybook to version 8.3.0-alpha.5 which includes a bug fix which reduced the bundle size of our build Storybook from 26MB to 6MB and reduced build time from 37s to 10s. This change also significantly improves the loading performance of the onyx Storybook. If you are interested, the bug was caused by the vue-component-meta package which included ALL schema definitions for props, events, slots and exposed so if e.g. HTMLElement was used, it included all nested props, JSDocs etc. although they are not needed/used by Storybook, see [Storybook PR](storybookjs/storybook#28589)
|
|
@larsrickert I just upgraded to the final release yesterday (8.3.0) and everything worked like a charm. We also had a bug in 8.3.0-alpha.8 that the controls for argTypes weren't created automatically, but this was also fixed with 8.3.0. So far, really good work. Thank you very much! |
Closes #26647
Closes #26935
Based on discussions on #28462
What I did
This PR fixes the "out of memory" issue when running
storybook buildwhen using complex property types likeHTMLElementwith thevue-component-metadocgen plugin.The root cause is that
vue-component-metaincludes all recursive schemas for object types which can be very complex and large (e.g. HTMLElement). Since the nested schema is not needed inside Storybok because we don't generate UI controls for object properties, we should be able to safely omitting them without any side effects.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task --task sandbox --start-from auto --template vue3-vite/default-tsframework.options.docgenoption tovue-component-metatest?: HTMLElementto any Vue componentbuild-storybookDocumentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>