storybook icon indicating copy to clipboard operation
storybook copied to clipboard

fix: conflict when argtype slots share names with other argtypes

Open NielsOtten opened this issue 6 months ago • 2 comments

Closes #12850

What I did

I fixed an issue specific to Vue where, if a prop and a slot have the same name, the ArgsTable would only display the prop, omitting the slot. The fix ensures both are correctly displayed by uniquely identifying slots with a section-based key.

Code Changes:

  • Updated the key used for argTypes to differentiate between slots and props by adding a prefix (slots.) to slot names.
  • Attempted to generalize this approach for other categories (e.g., exposed, events). However, this also surfaced categories like "exposed." Since I didn’t use defineExposed, this behavior needs to be reviewed to determine if it's correct. The related issue #13410 might be relevant, and if so, further generalization could be necessary.

Potential Breaking Change

  • Behavior Change: This update alters the existing behavior of the ArgsTable. Previously, if a prop and slot shared the same name, only the prop was displayed. With this fix, both the prop and slot will now be displayed. This change may affect users relying on the previous behavior, so it might be considered a breaking change.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • [ ] stories
  • [ ] unit tests
  • [ ] integration tests
  • [ ] end-to-end tests

Manual testing

  1. Run a sandbox (e.g., yarn task --task sandbox --start-from auto --template react-vite/default-ts).
  2. Open Storybook in your browser.
  3. Access a story with conflicting prop and slot names.
  4. Verify that both the slot and prop are displayed correctly in the ArgsTable.

Documentation

  • [ ] Add or update documentation reflecting your changes
  • [ ] If you are deprecating/removing a feature, make sure to update MIGRATION.MD

Checklist for Maintainers

  • [ ] When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • [ ] Make 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/core team 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>

Greptile Summary

This pull request addresses an issue specific to Vue components where props and slots with the same name were not both displayed in the ArgsTable. The fix ensures that both are correctly displayed by uniquely identifying slots with a section-based key.

Key changes:

  • Updated extractArgTypes function in code/renderers/vue3/src/docs/extractArgTypes.ts
  • Added prefix 'slots.' to slot names in the argTypes object
  • Improved handling of undefined argTypes
  • Generalized approach for other categories (e.g., exposed, events)

The fix alters existing behavior, potentially affecting users relying on the previous display logic. This change may be considered a breaking change and should be carefully reviewed.

NielsOtten avatar Aug 20 '24 20:08 NielsOtten