-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
CLI: Install Svelte CSF v5 in Svelte5 projects #29323
Conversation
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.
3 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
const packageManager = { | ||
getInstalledVersion: async (pkg: string) => pkg === 'svelte' ? svelteVersion : undefined, | ||
getAllDependencies: async () => ({ svelte: `^${svelteVersion}` }) | ||
} as any as JsPackageManager; |
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.
style: Avoid using 'as any' type assertion. Consider creating a mock class that implements JsPackageManager interface
const packageManager = { | ||
getInstalledVersion: async (pkg: string) => undefined, | ||
getAllDependencies: async () => ({ svelte: svelteSpecifier }) | ||
} as any as JsPackageManager; |
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.
style: Same as above, avoid 'as any' type assertion
try { | ||
const coerced = coerce(svelteSpecifier); | ||
if(coerced?.version) { | ||
return versionHelper(major(coerced.version)); | ||
} | ||
} catch {} |
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.
style: Consider handling specific errors instead of using an empty catch block
☁️ Nx Cloud ReportCI is running/has finished running commands for commit c5e384e. 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 2 targetsSent with 💌 from NxCloud. |
Yes we do. I can provide the stories, if you can figure out how to correctly select them based on the version? |
Let's do that as a separate task. And after Svelte5 is released? Self-merging @JReinhold |
CLI: Install Svelte CSF v5 in Svelte5 projects (cherry picked from commit 0eb384b)
Closes N/A
What I did
Prepare
storybook init
for the Svelte 5 release, in both Svelte and SvelteKit projects:package.json
version specifieraddon-svelte-csf
@JReinhold Do we need to update the template stories for Svelte CSF v5?
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
What I did:
svelte-kit/skeleton-ts
sandbox and verify that it's using the latest v4 stable release ofaddon-svelte-csf
svelte-kit/prerelease-ts
sandbox and verify that it's using the latest prerelease ofaddon-svelte-csf
🦋 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 PR updates the Storybook CLI to dynamically install the appropriate version of addon-svelte-csf based on the installed Svelte version, preparing for Svelte 5 compatibility.
getAddonSvelteCsfVersion
function incode/lib/create-storybook/src/generators/SVELTE/index.ts
to determine correct addon versioncode/lib/create-storybook/src/generators/SVELTE/index.test.ts
for various Svelte versionscode/lib/create-storybook/src/generators/SVELTEKIT/index.ts
to use dynamic addon version