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.

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: Add v5 stories to CLI templates #29382

Merged
merged 9 commits into from
Oct 17, 2024
Merged

Svelte: Add v5 stories to CLI templates #29382

merged 9 commits into from
Oct 17, 2024

Conversation

JReinhold
Copy link
Contributor

@JReinhold JReinhold commented Oct 16, 2024

Follow-up to #29323

What I did

Added Svelte 5 templates for the usual Button, Header and Page templates, for JS and TS.

Note 1: The ts-3-8 and ts-4-9 directories are identical, in Svelte CSF there's no difference
Note 2: Only the components differ between the JS and TS templates, the story files are identical.

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

What I did:

  • Initialize the sveltekit-prerelease-ts sandbox and verify that new stories are added & it runs
  • Initialize the sveltekit-skeleton-ts sandbox and verify that old stories are added & it runs

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 pull request has been released as version 0.0.0-pr-29382-sha-f608e224. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information
Published version 0.0.0-pr-29382-sha-f608e224
Triggered by @JReinhold
Repository storybookjs/storybook
Branch svelte-5-stories
Commit f608e224
Datetime Thu Oct 17 12:16:59 UTC 2024 (1729167419)
Workflow run 11384739899

To request a new release of this pull request, mention the @storybookjs/core team.

core team members can create a new canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=29382

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 78.7 MB 78.7 MB 10.6 kB 3.99 0%
initSize 147 MB 147 MB 13.2 kB -0.33 0%
diffSize 68.3 MB 68.3 MB 2.55 kB -0.35 0%
buildSize 6.79 MB 6.79 MB 0 B 0.34 0%
buildSbAddonsSize 1.5 MB 1.5 MB 0 B -0.6 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 1.83 MB 1.83 MB 0 B 1.11 0%
buildSbPreviewSize 270 kB 270 kB 0 B -1.53 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 3.8 MB 3.8 MB 0 B 0.54 0%
buildPreviewSize 2.99 MB 2.99 MB 0 B 0.32 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 8.4s 7.7s -717ms -0.59 -9.3%
generateTime 27.6s 19.7s -7s -956ms -0.57 -40.3%
initTime 18.2s 12.8s -5s -420ms -0.63 -42.2%
buildTime 8.2s 8.7s 518ms -0.18 5.9%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 7.3s 5.4s -1s -859ms -1.25 🔰-34%
devManagerResponsive 4.4s 3.5s -825ms -1.3 🔰-23%
devManagerHeaderVisible 574ms 534ms -40ms -0.78 -7.5%
devManagerIndexVisible 603ms 564ms -39ms -0.78 -6.9%
devStoryVisibleUncached 1.1s 856ms -331ms -0.73 -38.7%
devStoryVisible 601ms 563ms -38ms -0.8 -6.7%
devAutodocsVisible 404ms 449ms 45ms -0.97 10%
devMDXVisible 542ms 423ms -119ms -1.49 🔰-28.1%
buildManagerHeaderVisible 593ms 508ms -85ms -0.5 -16.7%
buildManagerIndexVisible 622ms 520ms -102ms -0.49 -19.6%
buildStoryVisible 623ms 577ms -46ms -0.41 -8%
buildAutodocsVisible 505ms 438ms -67ms -0.71 -15.3%
buildMDXVisible 511ms 402ms -109ms -1.01 -27.1%

Greptile Summary

This PR adds Svelte 5 story templates to the Storybook CLI, including new Button, Header, and Page components for both JavaScript and TypeScript.

  • Introduced new Svelte 5 templates in code/renderers/svelte/template/cli for JS and TS
  • Updated code/core/src/cli/helpers.ts to support Svelte 5 template selection
  • Modified code/lib/create-storybook/src/generators/SVELTE/index.ts for simplified Svelte version detection
  • Updated Svelte dependency to version ^5.0.0-next.268 in code/package.json
  • Added new $props() and $state syntax usage in Svelte 5 components

@JReinhold JReinhold added cli patch:yes Bugfix & documentation PR that need to be picked to main branch svelte maintenance User-facing maintenance tasks ci:daily Run the CI jobs that normally run in the daily job. labels Oct 16, 2024
@JReinhold
Copy link
Contributor Author

@shilman could you help me get this over the finish line?

I've added the necessary templates here, but they aren't actually being used CLI during init when Svelte 5 is detected. During #29323 you said that it would be a breeze to do this?

Copy link

nx-cloud bot commented Oct 16, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit d22b8e9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@JReinhold
Copy link
Contributor Author

@benmccann could you sanity check my Svelte 5 syntax skills here?

@benmccann
Copy link
Contributor

looks good to me!

@shilman shilman changed the title Svelte: Add v5 stories to templates Svelte: Add v5 stories to CLI templates Oct 17, 2024
@shilman shilman marked this pull request as ready for review October 17, 2024 04:23
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.

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


<button
type="button"
class={['storybook-button', `storybook-button--${size}`].join(' ')}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Use template literal instead of array join for better readability

Comment on lines +36 to +44
{#if user}
<span class="welcome">
Welcome, <b>{user.name}</b>!
</span>
<Button size="small" onClick={onLogout} label="Log out" />
{:else}
<Button size="small" onClick={onLogin} label="Log in" />
<Button primary size="small" onClick={onCreateAccount} label="Sign up" />
{/if}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding an aria-label to the Button components for better accessibility


<button
type="button"
class={['storybook-button', `storybook-button--${size}`].join(' ')}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Use template literal for class instead of array join for better performance

Copy link
Contributor Author

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

Thanks for the help @shilman. I tested the canary in Svelte 4 + 5, with TS/JSDoc/JS and everything worked as expected.

I've had to silence some TS errors in our own stories, there's a big risk that our CSF typings for Svelte are slightly incompatible with Svelte 5, but that's not for this PR to solve.

@JReinhold
Copy link
Contributor Author

Merging, CI failures are unrelated to this work.

@JReinhold JReinhold merged commit b370f6a into next Oct 17, 2024
103 of 108 checks passed
@JReinhold JReinhold deleted the svelte-5-stories branch October 17, 2024 20:12
JReinhold added a commit that referenced this pull request Oct 18, 2024
Svelte: Add v5 stories to CLI templates
(cherry picked from commit b370f6a)
@github-actions github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:daily Run the CI jobs that normally run in the daily job. cli maintenance User-facing maintenance tasks 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.

3 participants