Svelte: Add scripts for ecosystem CI #32564
Conversation
WalkthroughThis change centralizes the ecosystem CI before-test logic and adds Svelte-specific CI scripts. scripts/ecosystem-ci/before-test.js now accepts a sandbox template argument (defaulting to react-vite/default-ts), constructs a dynamic sandbox directory by replacing "/" with "-", and uses that directory for subsequent yarn and Playwright commands. package.json adds Sequence Diagram(s)sequenceDiagram
autonumber
actor CI as CI Runner
participant NPM as package.json Scripts
participant Script as scripts/ecosystem-ci/before-test.js
participant FS as Sandbox Dir (dynamic)
participant Yarn as yarn / npx
note over CI,NPM: Trigger "vite-ecosystem-ci:before-test" or "svelte-ecosystem-ci:before-test"
CI->>NPM: run <ecosystem-ci>:before-test
NPM->>Script: node before-test.js <template>\n(e.g., react-vite/default-ts or svelte-kit/skeleton-ts)
Script->>Script: Resolve sandboxDir from template\n(replace "/" with "-")
Script->>FS: Locate ../../sandbox/<template-as-dir>/package.json
Script->>Yarn: yarn add -D @playwright/test (cwd=sandboxDir)
Script->>Yarn: npx playwright install (cwd=sandboxDir)
NPM->>Yarn: yarn install (cwd=sandboxDir)
note right of Yarn: Dependencies installed in selected sandbox
sequenceDiagram
autonumber
actor CI as CI Runner
participant NPM as package.json Scripts
participant Tasks as task runner
participant Code as code workspace
participant SB as sandbox (svelte-kit-skeleton-ts)
rect rgb(240,248,255)
note over CI,NPM: Svelte build flow
CI->>NPM: run svelte-ecosystem-ci:build
NPM->>Tasks: task --task install
NPM->>Code: yarn --cwd code build svelte
NPM->>Tasks: task sandbox --template svelte-kit/skeleton-ts --start-from=compile --no-link --skip-cache
end
rect rgb(245,255,240)
note over CI,NPM: Svelte test flow
CI->>NPM: run svelte-ecosystem-ci:test
NPM->>Tasks: task --task test-runner --template svelte-kit/skeleton-ts --start-from=build --no-link --skip-cache
NPM->>Tasks: task --task vitest-integration --template svelte-kit/skeleton-ts --start-from=vitest-integration --no-link --skip-cache
end
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
JReinhold
added
svelte
build
|
View your CI Pipeline Execution ↗ for commit 60a567c
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/ecosystem-ci/before-test.js (2)
22-22: Replace all slashes when slugging the sandbox template.
replace('/')only affects the first slash; usereplaceAll('/')to prevent path surprises if templates ever contain multiple segments.- `../../sandbox/${sandbox.replace('/', '-')}/package.json` + `../../sandbox/${sandbox.replaceAll('/', '-')}/package.json`
17-17: Optionally validate the template arg to fail fast on typos.Before reading/writing, check that the resolved sandbox directory exists and throw a clear error if not. Keeps CI failures actionable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)scripts/ecosystem-ci/before-test.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Adhere to ESLint and Prettier rules across all JS/TS source files
Files:
scripts/ecosystem-ci/before-test.js
🧠 Learnings (2)
📚 Learning: 2025-09-25T09:21:27.274Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.274Z
Learning: Applies to code/sandbox/** : Do not use or modify code/sandbox; it contains internal build artifacts and is not for testing
Applied to files:
scripts/ecosystem-ci/before-test.js
📚 Learning: 2025-09-25T09:21:27.274Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.274Z
Learning: Applies to code/**/playwright.config.@(js|ts|mjs|cjs) : Use Playwright 1.52.0 for E2E tests and keep Playwright configuration aligned
Applied to files:
scripts/ecosystem-ci/before-test.js
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Danger JS
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (3)
package.json (3)
14-15: LGTM: Svelte ecosystem CI scripts integration looks correct.These hook into the centralized before-test and the sandbox flow as expected.
21-21: LGTM: Centralized Vite before-test usage.Switching to the shared before-test with a template arg is a good consolidation.
7-7: No functional change detected.Entry is unchanged in behavior.
package.json
Outdated
| "svelte-ecosystem-ci:before-test": "node ./scripts/ecosystem-ci/before-test.js svelte-kit/skeleton-ts && cd ./sandbox/svelte-kit-skeleton-ts && yarn install", | ||
| "svelte-ecosystem-ci:build": "yarn task --task install && yarn --cwd code build svelte && yarn task --task sandbox --template svelte-kit/skeleton-ts --start-from=compile --no-link", | ||
| "svelte-ecosystem-ci:test": "yarn task --task test-runner --template svelte-kit/skeleton-ts --start-from=build --no-link && yarn task --task vitest-integration --template svelte-kit/skeleton-ts --start-from vitest-integration --no-link", |
There was a problem hiding this comment.
Adding some comments here for my own sanity:
- build:
- Install dependencies
- Explicitly build
@storybook/svelte, because for some weird reason, nx does not see that package when the repo is cloned deep into the ecosystem CI repo, so it is not built as part of running thecompiletask. - Compile packages in production mode, because currently Svelte sandboxes don't work in dev mode because the Svelte CSF addon is failing to resolve due to some symlinking issues.
- Create a SvelteKit sandbox.
- before test:
- set up playwright and resolutions in the sandbox
- test:
- Run all stories via the test runner, by building the sandbox Storybook first, then serving it, and running the test runner on it
- Run all the stories again, this time via Vitest
|
@dominikg I used our SvelteKit based sandbox here to get as much coverage as possible, but I could also use a plain Svelte+Vite sandbox instead, if you think there's a reason for that. |
|
we need to make sure that ecosystem cis added overrides for svelte are honored if you run install. also keep in mind to not use nx cloud/nx caching for this to avoid stale issues. nx is likely unaware of ecosystem ci. |
good call, it appears to be working as intended. I tested this by running the ecosystem CI with
Good call, added cache skipping in 60a567c |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-25T09:21:27.274Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-25T09:21:27.274Z
Learning: Applies to test-storybooks/** : Maintain test configurations and assets under test-storybooks/ for Storybook testing
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (2)
package.json (2)
16-16: Fix CLI flag formatting: use--start-from=consistently.Second task uses the space form; align with equals form to avoid mis‑parsing by the task runner.
- "svelte-ecosystem-ci:test": "yarn task --task test-runner --template svelte-kit/skeleton-ts --start-from=build --no-link --skip-cache && yarn task --task vitest-integration --template svelte-kit/skeleton-ts --start-from vitest-integration --no-link --skip-cache", + "svelte-ecosystem-ci:test": "yarn task --task test-runner --template svelte-kit/skeleton-ts --start-from=build --no-link --skip-cache && yarn task --task vitest-integration --template svelte-kit/skeleton-ts --start-from=vitest-integration --no-link --skip-cache",
7-7: Sanity check: confirmscripts/get-templateexists and is exported.New script entry looks fine; please verify the scripts workspace exposes
get-template(analogous toget-sandbox-dir) to prevent CI failures.
Companion PR: sveltejs/svelte-ecosystem-ci#39
What I did
Added scripts to run in the Svelte ecosystem CI, similar to what we do with the Vite ecosystem CI.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake 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/coreteam 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>Summary by CodeRabbit