Vue3: Use component's name as displayName if available #33428
Conversation
Sidnioulz
changed the title
Sidnioulz
left a comment
Sidnioulz
left a comment
There was a problem hiding this comment.
Thanks for the PR @huang-julien! Much appreciated.
Could you please just revert the order between __name and meta to ensure future compatibility if the docgen tool ends up applying a different logic in the future? I'd like to treat Vue internals as the default, and the docgen process as the one that has the last say.
Also, could you please expand on the manual testing section? I think I can infer what to do from the issue in this case but it really greatly speeds up reviews when we have exact steps to follow on each PR to validate changes.
Thanks!
| s.append(`\n;${name}.__docgenInfo = Object.assign(${JSON.stringify(meta)}, { | ||
| displayName: ${name}.name || ${name}.__name || ${JSON.stringify(meta.displayName)} | ||
| })`); |
There was a problem hiding this comment.
Whilst I agree with the need to read __name, I'd prefer if we read it before meta and if we honour meta if it is defined.
So you could do something like:
Object.assign({
displayName: ${name}.name || ${name}.__name,
}, JSON.stringify(meta))This will ensure whatever changes are made in the future to the layer in charge of extracting docgen information will be correctly taken into account here.
|
|
||
| const s = new MagicString(src); | ||
| s.append(`;_sfc_main.__docgenInfo = ${JSON.stringify(metaData)}`); | ||
| s.append(`;_sfc_main.__docgenInfo = Object.assign(${JSON.stringify(metaData)}, { |
📝 WalkthroughWalkthroughBoth Vue3 Vite plugin files are modified to enhance component documentation metadata. Instead of directly assigning docgen info, both now use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts (1)
137-139: Consider adding automated tests for the displayName resolution logic.The PR description indicates that no automated tests were added for this change. Given that this modifies runtime behavior for both default and named exports, and affects how component names appear in Storybook documentation, it would be valuable to add tests covering:
- Components with explicit
nameproperty- Components relying on Vue's
__name- Components falling back to metadata displayName
- Both default and named exports
This would help prevent regressions and ensure the fix for issue #30455 continues to work correctly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
code/frameworks/vue3-vite/src/plugins/vue-component-meta.tscode/frameworks/vue3-vite/src/plugins/vue-docgen.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json,md,html,css,scss}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Format code using Prettier with
yarn prettier --write <file>
Files:
code/frameworks/vue3-vite/src/plugins/vue-component-meta.tscode/frameworks/vue3-vite/src/plugins/vue-docgen.ts
**/*.{js,jsx,json,html,ts,tsx,mjs}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run ESLint checks using
yarn lint:js:cmd <file>or the full commandcross-env NODE_ENV=production eslint --cache --cache-location=../.cache/eslint --ext .js,.jsx,.json,.html,.ts,.tsx,.mjs --report-unused-disable-directivesto fix linting errors before committing
Files:
code/frameworks/vue3-vite/src/plugins/vue-component-meta.tscode/frameworks/vue3-vite/src/plugins/vue-docgen.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Enable TypeScript strict mode across all packages
Files:
code/frameworks/vue3-vite/src/plugins/vue-component-meta.tscode/frameworks/vue3-vite/src/plugins/vue-docgen.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx}: Export functions from modules if they need to be tested
Do not useconsole.log,console.warn, orconsole.errordirectly unless in isolated files where importing loggers would significantly increase bundle size
Files:
code/frameworks/vue3-vite/src/plugins/vue-component-meta.tscode/frameworks/vue3-vite/src/plugins/vue-docgen.ts
code/{core,lib,addons,builders,frameworks,presets}/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
loggerfromstorybook/internal/node-loggerfor server-side logging in Node.js code
Files:
code/frameworks/vue3-vite/src/plugins/vue-component-meta.tscode/frameworks/vue3-vite/src/plugins/vue-docgen.ts
🧬 Code graph analysis (1)
code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts (1)
code/core/src/csf-tools/CsfFile.ts (1)
meta(964-966)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: normal
| s.append(`\n;${name}.__docgenInfo = Object.assign(${JSON.stringify(meta)}, { | ||
| displayName: ${name}.name || ${name}.__name || ${JSON.stringify(meta.displayName)} | ||
| })`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use nullish coalescing (??) for consistency with vue-docgen.ts.
Line 138 uses the logical OR operator (||) to check for the displayName, while vue-docgen.ts (line 22) uses the nullish coalescing operator (??). This creates an inconsistency in how the two plugins handle falsy values.
For component name properties, ?? is more appropriate because it only checks for null or undefined, whereas || would skip over other falsy values like empty strings. Although component names should not be empty strings in practice, using ?? is more semantically correct and maintains consistency across both plugin files.
🔎 Proposed fix for consistency
- s.append(`\n;${name}.__docgenInfo = Object.assign(${JSON.stringify(meta)}, {
- displayName: ${name}.name || ${name}.__name || ${JSON.stringify(meta.displayName)}
- })`);
+ s.append(`\n;${name}.__docgenInfo = Object.assign(${JSON.stringify(meta)}, {
+ displayName: ${name}.name ?? ${name}.__name ?? ${JSON.stringify(meta.displayName)}
+ })`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| s.append(`\n;${name}.__docgenInfo = Object.assign(${JSON.stringify(meta)}, { | |
| displayName: ${name}.name || ${name}.__name || ${JSON.stringify(meta.displayName)} | |
| })`); | |
| s.append(`\n;${name}.__docgenInfo = Object.assign(${JSON.stringify(meta)}, { | |
| displayName: ${name}.name ?? ${name}.__name ?? ${JSON.stringify(meta.displayName)} | |
| })`); |
🤖 Prompt for AI Agents
In code/frameworks/vue3-vite/src/plugins/vue-component-meta.ts around lines 137
to 139, the displayName fallback uses the logical OR operator (||) which differs
from vue-docgen.ts; replace the OR with the nullish coalescing operator (??) so
the fallback becomes: displayName: {name}.name ?? {name}.__name ??
{JSON.stringify(meta.displayName)}; keep the surrounding string/serialization
exactly as-is and ensure spacing and punctuation remain valid.
| s.append(`;_sfc_main.__docgenInfo = Object.assign(${JSON.stringify(metaData)}, { | ||
| displayName: _sfc_main.name ?? _sfc_main.__name ?? ${JSON.stringify(metaData.displayName)} | ||
| });`); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Vue 3 __name property component runtime behavior automatic
💡 Result:
Short answer
- Vue's SFC compiler (esp. for <script setup>) adds an internal __name property to the compiled component object to carry the component's inferred/display name (filename or defineOptions/ name option). It's used at runtime for DevTools, warning traces, KeepAlive includes, and other tooling, but it's an internal helper, not a public API. [1][2]
Details / practical notes
- Where the name comes from: SFC filename inference or the name option / defineOptions({ name }). [1][2]
- How it appears at runtime: compiled component objects often include __name (e.g. accessible as instance.type.__name or instance._?.type?.__name). This is an implementation detail and can change across Vue/compiler versions. [2][3]
- Behavior differences: __name may be present only for components compiled with the SFC compiler (script-setup) and may be omitted/optimized away in some builds (production/optimizer), so don’t rely on it for app logic. [1][2][4]
- If you need a stable runtime name, use the public name option (name) or defineOptions and treat __name as internal. [1][2]
Sources
- Vue docs — component name / name inference. [1]
- Community/StackOverflow discussion showing runtime __name access and its internal/unstable nature. [2][3]
- Reports/threads showing __name sometimes missing/optimized and related build/tooling quirks. [4]
Be aware that __name is an internal Vue implementation detail, not a public API, and may be optimized away in production builds.
The code relies on _sfc_main.__name as part of the fallback chain for component names. While Vue's SFC compiler does add this property, it is an undocumented internal implementation detail that can vary across compiler versions and may be omitted in some builds. The fallback to metaData.displayName provides coverage, but consider documenting this dependency on internal Vue behavior or exploring more stable alternatives like the public name option.
🤖 Prompt for AI Agents
In code/frameworks/vue3-vite/src/plugins/vue-docgen.ts around lines 21 to 23,
the generated code relies on the internal _sfc_main.__name property which is not
a stable public API and may be removed in production builds; remove the
dependency on __name and use only the public name option with a fallback to
metaData.displayName (i.e. set displayName from _sfc_main.name ??
metaData.displayName), and if you must keep any mention of __name document this
internal dependency in a comment or README so consumers understand it’s not
guaranteed by Vue.
|
Thank you very much for your review ! So I took a look at
By default, Vue applies the filename to After some thought and to not break anything (also for better maintainability), maybe this could be an opt-in in both plugins ? WDYT ?
Sure ! To manually test it, you can create a component like this one that has a Filename <template>
<!---->
</template>
<script lang="ts" setup>
defineOptions({
name: 'my-button',
});
</script>When browsing its stories with this PR, storybook uses the <template>
<MyButton label="Button" primary />
</template>I can also write some unit tests if needed to prevent any future breaking updates with vue-component-meta or vue-docgen-api 👀 |
I didn't mean to imply we should revert the ordering of Rather, I'd like to treat docgen as a separate process from the framework's own provided information. Obviously, here, Vue maintainers provide the docgen tool and it fits in well with what the framework does. But we've had scenarios where that hasn't been the case. Someone could write their own distinct codegen tool for performance reasons, or because they wanna generate code for multiple frameworks from a declarative data model and there is nowhere to parse source code and extract types/JSDocs. By applying Vue/Nuxt's
Thanks! <3
Feel free to use your best judgement here. I don't want to have you waste time testing things that are guaranteed by the framework; though if |
2 participants
Closes #30455
What I did
Hey 👋 this PR makes the vue-component-meta plugin use the component's name if provided at runtime. By default, Vue will auto fill the
__nameproperty of the component, users usually define thenameproperty directlyChecklist for Contributors
Testing
It seems like there's no unit test for vue=-component-meta plugin.
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
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-storybook/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 publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.