-
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Sveltekit: Add support for $app/state #32437
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
Sveltekit: Add support for $app/state #32437
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.
Reviewing changes made in this pull request
code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/state.stories.js
Outdated
Show resolved
Hide resolved
justindomingue
left a comment
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.
🥳
tayyabmh
left a comment
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.
👍
|
@shilman @ndelangen if have a chance to review! |
|
@alexandfox kudos, we don't see this level of quality often in first-time PRs! Just so you know, we have a dedicated weekly meeting where we go through new PRs and assign them to maintainers. This happens on Mondays at 3PM CET, and it's a public meeting. You can join on Discord if you feel like providing additional context on the PR can be helpful. |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 49 KB | 58 KB | 🚨 +9 KB 🚨 |
| Dependency size | 26.84 MB | 26.84 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
awesome, thanks for the info! will definitely try to join, would be awesome to get this merged to support testing of svelte 5 components using $app/state 🙌🏼 |
|
View your CI Pipeline Execution ↗ for commit dd1f9cf
☁️ Nx Cloud last updated this comment at |
… and consistency. Updated state handling in preview.ts and state.ts to ensure correct type assertions and maintain code clarity.
…on for improved organization and clarity.
…sting capabilities.
…proved readability by adding missing commas in object literals.
|
@alexandfox I tried fixing some problems encountered on CI, but the remaining ones are a bit out of my depth, do you want to take a look? 🙏 |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds support for mocking SvelteKit’s $app/state: introduces a new mock module, wires it into build/exports, aliases it in the Vite plugin, initializes it from preview parameters, extends types, and adds template examples and stories demonstrating page, navigating, and updated state. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Storybook as Storybook Preview
participant Decorator as Preview Decorator
participant Mocks as $app/state (mock)
participant Component as State.svelte
User->>Storybook: Select story
Storybook->>Decorator: Initialize with parameters
Decorator->>Mocks: setPageState(page)
Decorator->>Mocks: setNavigatingState(navigating)
Decorator->>Mocks: setUpdatedState(updated)
Storybook->>Component: Render
Component->>Mocks: Import page, navigating, updated
Component->>Mocks: updated.check()
Mocks-->>Component: current values (page/navigating/updated)
Note right of Mocks: Dispatches "storybook:updated-check" event
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
code/frameworks/sveltekit/src/mocks/app/state.ts (1)
1-69: Blocker: remove Svelte runes/context usage from TS mocks — convert to plain exported values + settersrg confirms $state/getContext/setContext appear in:
- code/frameworks/sveltekit/src/mocks/app/state.ts
- code/frameworks/sveltekit/src/mocks/app/stores.ts
- code/frameworks/sveltekit/src/mocks/app/navigation.ts
These use Svelte runes/contexts in plain .ts files (won’t compile / will throw at runtime) and the setter functions only call setContext without updating exported values. Action: apply the framework-agnostic rewrite for state.ts from the original comment (replace with plain exported objects + setters invoked by the preview decorator) and refactor stores.ts and navigation.ts the same way — remove $state, getContext, setContext usage and expose mutable exports + setters.
🧹 Nitpick comments (1)
code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/State.svelte (1)
19-21: Consider moving updated.check() to onMountAvoids side effects during module evaluation/SSR; call it in onMount instead.
Apply this diff:
- // Trigger check for demonstration - updated.check(); + // Trigger check for demonstration + import { onMount } from 'svelte'; + onMount(() => { + updated.check(); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
code/frameworks/sveltekit/build-config.ts(1 hunks)code/frameworks/sveltekit/package.json(1 hunks)code/frameworks/sveltekit/src/mocks/app/state.ts(1 hunks)code/frameworks/sveltekit/src/plugins/mock-sveltekit-stores.ts(1 hunks)code/frameworks/sveltekit/src/preview.ts(2 hunks)code/frameworks/sveltekit/src/types.ts(1 hunks)code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/State.svelte(1 hunks)code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/state.stories.js(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Implement mock behaviors in beforeEach blocks
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should be placed in beforeEach blocks
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Group related mocks together
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.tscode/frameworks/sveltekit/src/mocks/app/state.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required dependencies that the test subject uses
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid mocking only a subset of required dependencies
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Document complex mock behaviors
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.tscode/frameworks/sveltekit/src/mocks/app/state.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock at the highest level of abstraction needed
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock all required properties and methods that the test subject uses
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.ts
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Keep mock implementations simple and focused
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Avoid inline mock implementations within test cases
Applied to files:
code/frameworks/sveltekit/package.jsoncode/frameworks/sveltekit/build-config.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to type and access mocked functions
Applied to files:
code/frameworks/sveltekit/package.json
📚 Learning: 2025-09-17T08:11:47.197Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.197Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use type-safe mocking with vi.mocked()
Applied to files:
code/frameworks/sveltekit/package.json
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vi.mocked() to access and implement mock behaviors
Applied to files:
code/frameworks/sveltekit/build-config.tscode/frameworks/sveltekit/src/mocks/app/state.ts
📚 Learning: 2025-09-17T08:11:47.196Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-09-17T08:11:47.196Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Mock implementations should match the expected return type of the original function
Applied to files:
code/frameworks/sveltekit/build-config.ts
🧬 Code graph analysis (1)
code/frameworks/sveltekit/src/preview.ts (1)
code/frameworks/sveltekit/src/mocks/app/state.ts (3)
setPageState(58-60)setNavigatingState(62-64)setUpdatedState(66-68)
⏰ 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: normal
- GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (7)
code/frameworks/sveltekit/src/plugins/mock-sveltekit-stores.ts (1)
11-11: Alias for $app/state looks correctGood addition; path matches the new internal export.
Please confirm the vite plugin that includes mockSveltekitStores() is active in SvelteKit stories so this alias takes effect at runtime.
code/frameworks/sveltekit/package.json (1)
37-38: Public export for internal mocks/app/state is correctExport path and ordering are consistent with forms/navigation/stores.
code/frameworks/sveltekit/build-config.ts (1)
25-29: Build entry added for mocks/app/stateMatches other mock entries; dts disabled consistently.
code/frameworks/sveltekit/src/preview.ts (2)
7-9: Preview wires state setters similarly to stores — OK pending state mock fixThe calls align with parameter shape. However, current state.ts implementation uses Svelte context + Svelte 5 runes in a .ts file, which will not compile/run (see comments in state.ts). Once state.ts is reworked, these calls should work.
24-27: Boolean coercion for updatedDouble‑bang coercion is fine here to normalize truthy values.
code/frameworks/sveltekit/src/types.ts (1)
64-68: State parameters shape LGTMTypes mirror SvelteKit semantics (page object, navigating nullable, updated boolean).
code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/state.stories.js (1)
157-157: Style: Add newline at end of fileMatches repo style and prior bot comment.
|
I really appreciate the work you did here, and everyone's input! We had an earlier PR working on this that got close before going stale, but your work here prompted me to finish up the last missing pieces there, and support for mocking Thank you ❤️ |
Closes #30209
What I did
Add support for mocking $app/state in Svelte 5.
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>Greptile Summary
Updated On: 2025-09-10 17:34:55 UTC
This PR adds support for mocking SvelteKit's new
$app/statemodule in Storybook, which was introduced in SvelteKit 2.12.0 as the Svelte 5 rune-based replacement for the existing$app/storesmodule. The implementation extends the existing mocking infrastructure to support the new state management paradigm while maintaining backward compatibility.The changes include:
state.tsfile that creates reactive state objects using Svelte 5's$state.raw()and context API, mirroring the structure of the existing stores implementation'$app/state': '@storybook/sveltekit/internal/mocks/app/state'to redirect imports to the mock implementationsvelteKitMocksDecoratorto handle both stores and state parameters through parallel setter functionsstateparameter structure toSvelteKitParametersinterface withpage,navigating, andupdatedpropertiesState.svelte) and stories (state.stories.js) that demonstrate mocking various state scenarios including page data, navigation states, error conditions, and form dataThe implementation follows the established pattern of the existing
$app/storesmocking system but adapts it for Svelte 5's runes-based reactivity. Users can now mock state using thesveltekit_experimental.stateparameter structure, allowing them to test components that use the modern$app/statemodule in isolation. The change maintains the same API consistency as the existing stores system, making it easy for developers to transition between the two approaches.Confidence score: 4/5
state.tsto ensure Svelte 5 runes integration works correctlySummary by CodeRabbit
New Features
Documentation