8000 Svelte: Fix source view always using `<wrapper ...>` by JReinhold · Pull Request #31639 · storybookjs/storybook · GitHub
[go: up one dir, main page]

Skip to content

Svelte: Fix source view always using <wrapper ...> #31639

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

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

JReinhold
Copy link
Contributor
@JReinhold JReinhold commented Jun 2, 2025

Closes https://bsky.app/profile/hmans.dev/post/3lqfb4z3pf224

What I did

In the core Svelte source generator, we now skip sorce generation if it's a Svelte CSF story, as the addon will do its own source generation in https://github.com/storybookjs/addon-svelte-csf/blob/a475611195a9fe94a0410af6c8bcae9d3c2437b1/src/runtime/emit-code.ts#L23-L44

Previously this was not an issue, as the addon would emit on the following tick, so always after the core had emitted its source code, but potentially #31488 flipped the timing of this.

Svelte CSF has a similar (opposite) skip condition at https://github.com/storybookjs/addon-svelte-csf/blob/a475611195a9fe94a0410af6c8bcae9d3c2437b1/src/runtime/emit-code.ts#L50-L54

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. Open the Svelte-Vite Storybook from next, expand the source view, and see the wrong component name: https://630873996e4e3557791c069c-pagxgwozgc.chromatic.com/?path=/docs/example-button--docs
  2. Open the same Storybook from this branch, expand the source view, and see the correct component name: https://630873996e4e3557791c069c-biyuvgoutj.chromatic.com/?path=/docs/example-button--docs

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

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-storybook/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

Fixes source code generation for Svelte CSF stories by preventing duplicate source rendering between core and addon implementations. The change ensures proper source view display without wrapper elements.

  • Modified skipSourceRender in code/renderers/svelte/src/docs/sourceDecorator.ts to skip core source generation for Svelte CSF stories
  • Added test coverage in sourceDecorator.test.ts to validate source generation behavior
  • Prevents timing conflicts between core and addon source generation implementations
  • Aligns with existing skip condition in addon-svelte-csf for consistent source handling
  • Resolves incorrect component name display in source view documentation

Copy link
Contributor
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR resolves an issue where the source view was always rendered using a <wrapper ...> element by adding a condition to bypass rendering for Svelte CSF stories.

  • Added a condition to check if any tag in the context starts with "svelte-csf".
  • Skips core source generation when the story is a Svelte CSF story so that the addon can handle source generation.

8000

Copy link
nx-cloud bot commented Jun 2, 2025

View your CI Pipeline Execution ↗ for commit 22a5378.

Command Status Duration Result
nx run-many -t build -c production --parallel=3 ✅ Succeeded 3s View ↗
nx run-many -t check -c production --parallel=7 ✅ Succeeded 1s View ↗

☁️ Nx Cloud last updated this comment at 2025-06-02 09:27:36 UTC

@JReinhold JReinhold marked this pull request as ready for review June 2, 2025 09:10
Copy link
Contributor
@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@JReinhold JReinhold added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Jun 2, 2025
@JReinhold JReinhold merged commit f2e1865 into next Jun 2, 2025
69 of 76 checks passed
@JReinhold JReinhold deleted the jeppe/fix-svelte-csf-source-code branch June 2, 2025 11:12
valentinpalkovic pushed a commit that referenced this pull request Jun 5, 2025
…e-code

Svelte: Fix source view always using `<wrapper ...>`
(cherry picked from commit f2e1865)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 5, 2025
@yannbf yannbf added needs qa Indicates that this needs manual QA during the upcoming minor/major release and removed needs qa Indicates that this needs manual QA during the upcoming minor/major release labels Jul 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
block: source bug ci:normal patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch svelte
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0