Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Vue: Fix out of memory error when using vue-component-meta #28589

Merged
merged 5 commits into from
Jul 17, 2024
Merged

Vue: Fix out of memory error when using vue-component-meta #28589

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
33b487f
fix(vue-component-meta): fix out of memory issue
larsrickert Jul 13, 2024
70a6a41
update comment
larsrickert Jul 13, 2024
ebe47ff
fix test
larsrickert Jul 13, 2024
e75ac8b
update docs
larsrickert Jul 16, 2024
9601e82
Merge branch 'next' into larsrickert/26647-out-of-memory
kasperpeulen Jul 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,26 @@ export async function vueComponentMeta(tsconfigPath = 'tsconfig.json'): Promise<

const exportName = exportNames[index];

// we remove nested object schemas here since they are not used inside Storybook (we don't generate controls for object properties)
// and they can cause "out of memory" issues for large/complex schemas (e.g. HTMLElement)
// it also reduced the bundle size when running "Storybook build" when such schemas are used
(['props', 'exposed'] as const).forEach((key) => {
meta[key].forEach((value) => {
if (typeof value.schema !== 'object') return;

// we need to use Object.defineProperty here since schema is a getter so we can not set it directly
Object.defineProperty(value, 'schema', {
configurable: true,
enumerable: true,
value: {
kind: value.schema.kind,
type: value.schema.type,
// note that value.schema.schema is not included here (see comment above)
},
});
});
});

const exposed =
// the meta also includes duplicated entries in the "exposed" array with "on"
// prefix (e.g. onClick instead of click), so we need to filter them out here
Expand Down
61 changes: 8 additions & 53 deletions code/renderers/vue3/src/docs/__snapshots__/extractArgTypes.test.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,7 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
"value": {
"name": "object",
"required": false,
"value": {
"nestedProp": {
"name": "string",
"required": true,
},
},
"value": {},
},
},
},
Expand All @@ -183,12 +178,7 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
"value": {
"name": "object",
"required": false,
"value": {
"nestedProp": {
"name": "string",
"required": true,
},
},
"value": {},
},
},
},
Expand Down Expand Up @@ -279,12 +269,7 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
"type": {
"name": "object",
"required": true,
"value": {
"foo": {
"name": "string",
"required": true,
},
},
"value": {},
},
},
"literalFromContext": {
Expand Down Expand Up @@ -325,12 +310,7 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
"type": {
"name": "object",
"required": true,
"value": {
"nestedProp": {
"name": "string",
"required": true,
},
},
"value": {},
},
},
"nestedIntersection": {
Expand All @@ -347,16 +327,7 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
"type": {
"name": "object",
"required": true,
"value": {
"additionalProp": {
"name": "string",
"required": true,
},
"nestedProp": {
"name": "string",
"required": true,
},
},
"value": {},
},
},
"nestedOptional": {
Expand All @@ -377,22 +348,12 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
{
"name": "object",
"required": false,
"value": {
"nestedProp": {
"name": "string",
"required": true,
},
},
"value": {},
},
{
"name": "object",
"required": false,
"value": {
"nestedProp": {
"name": "string",
"required": true,
},
},
"value": {},
},
],
},
Expand All @@ -411,13 +372,7 @@ exports[`extractArgTypes (vue-docgen-api) > should extract props for component 1
"type": {
"name": "object",
"required": false,
"value": {
"recursive": {
"name": "other",
"required": true,
"value": "MyNestedRecursiveProps",
},
},
"value": {},
},
},
"stringArray": {
Expand Down
12 changes: 3 additions & 9 deletions code/renderers/vue3/src/docs/extractArgTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { VueDocgenInfo, VueDocgenInfoEntry, VueDocgenPlugin } from '@storybook/vue3-vite';
import type { ExtractedProp } from 'storybook/internal/docs-tools';
import {
convert,
Expand All @@ -6,7 +7,6 @@ import {
type ArgTypesExtractor,
} from 'storybook/internal/docs-tools';
import type { SBType, StrictArgTypes, StrictInputType } from 'storybook/internal/types';
import type { VueDocgenInfo, VueDocgenInfoEntry, VueDocgenPlugin } from '@storybook/vue3-vite';

type PropertyMetaSchema = VueDocgenInfoEntry<'vue-component-meta', 'props'>['schema'];

Expand Down Expand Up @@ -283,17 +283,11 @@ export const convertVueComponentMetaProp = (
};
}

// recursively/deeply convert all properties of the object
case 'object':
return {
name: 'object',
value: Object.entries(schema.schema ?? {}).reduce<Record<string, SBType>>(
(obj, [propName, propSchema]) => {
obj[propName] = convertVueComponentMetaProp(propSchema);
return obj;
},
{}
),
// Storybook does not generate controls for object properties so we don't need to recursively map the object schema here
Copy link
Contributor

@chakAs3 chakAs3 Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes exactly, I updated the comment so I hope its clearer now :)

value: {},
required,
};

Expand Down
Loading