-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Svelte: Fix source with decorators always showing the SlotDecorator
component
#24800
Svelte: Fix source with decorators always showing the SlotDecorator
component
#24800
Conversation
…r-show-slotdecorator-as-source
test('Decorators are excluded from generated source code', async ({ page }) => { | ||
const sbPage = new SbPage(page); | ||
|
||
await sbPage.navigateToStory('stories/renderers/svelte/slot-decorators', 'docs'); | ||
const root = sbPage.previewRoot(); | ||
const showCodeButton = (await root.locator('button', { hasText: 'Show Code' }).all())[0]; | ||
await showCodeButton.click(); | ||
const sourceCode = root.locator('pre.prismjs'); | ||
const expectedSource = '<ButtonJavaScript primary/>'; | ||
await expect(sourceCode.textContent()).resolves.toContain(expectedSource); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hoping to write framework-agnostic versions of this test soon, because I sense that this is an area where we're often seeing regressions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
The only reason not to do it by default is that it is a bit weird that we end up executing the story twice; and folks that aren't using decorators or don't care about source might not want us to do that for .. reasons.
Closes #21477
What I did
Defaulted the Svelte source decorator to always exclude decorators from the source generation.
We have a similar pattern in React, HTML and Web components, however for them
excludeDecorators
is configurable. I didn't think that was necessary here, since including decorators leads to a broken result, so I don't know why you would want that.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
/?path=/docs/stories-renderers-svelte-slot-decorators--docs
<ButtonJavaScript primary/>
and not<SlotDecorator primary/>
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/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>