Skip to content

Conversation

@xeho91
Copy link
Contributor

@xeho91 xeho91 commented May 3, 2025

Resolves #31257

What I did

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

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

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-storybook/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-31369-sha-038fb8e0. 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-31369-sha-038fb8e0
Triggered by @JReinhold
Repository xeho91/storybook
Branch feat/support-mocking-sveltekit-app-state
Commit 038fb8e0
Datetime Mon May 19 12:21:20 UTC 2025 (1747657280)
Workflow run 15112829944

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=31369

Greptile Summary

This PR adds support for mocking SvelteKit's $app/state module in Storybook, which is a significant enhancement for testing SvelteKit components. The changes enable developers to simulate various application states during component development and testing.

The implementation includes:

  • A new mock implementation for the $app/state module with support for page, navigating, and updated states
  • Updated type definitions and parameter structures
  • Migration from filesystem paths to package imports for better maintainability
  • Comprehensive example stories demonstrating the mocking functionality
  • Updated documentation reflecting the new features

The changes also maintain backward compatibility by supporting both the new state-based approach and the legacy stores approach.

Confidence score: 4/5

  1. This PR is generally safe to merge as it adds new functionality without breaking existing features
  2. The score of 4 reflects strong implementation with proper types, documentation, and example stories, though lacking unit tests
  3. Key files needing attention:
    • code/frameworks/sveltekit/src/mocks/app/state.svelte.js for potential edge cases
    • code/frameworks/sveltekit/src/preview.ts for state initialization logic
    • code/frameworks/sveltekit/src/types.ts for type safety

10 files reviewed, 1 comment
Edit PR Review Bot Settings | Greptile

Summary by CodeRabbit

  • New Features

    • Experimental $app/state mocking for SvelteKit (page, navigating, updated) with module alias so $app/state resolves to the mock.
    • Automatic state sync per story via a new preview hook.
    • Example State component and stories demonstrating Page, Navigating, and Updated scenarios.
    • Public types/parameters added to support sveltekit_experimental.state and richer state shapes.
  • Documentation

    • SvelteKit guide updated with $app/state API, examples, and options; stores-based docs retained.
  • Refactor

    • Renamed store setter helpers to setAppStoresPage / setAppStoresNavigating / setAppStoresUpdated.
  • Chores

    • Added packaging/build export and dependency optimization entries to surface the static state mock.

@xeho91 xeho91 marked this pull request as draft May 3, 2025 04:01
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.

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

@nx-cloud
Copy link

nx-cloud bot commented May 3, 2025

View your CI Pipeline Execution ↗ for commit e873a2f

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 47s View ↗

☁️ Nx Cloud last updated this comment at 2025-09-24 12:31:46 UTC

@JReinhold JReinhold self-assigned this May 5, 2025
@JReinhold JReinhold added feature request sveltekit ci:daily Run the CI jobs that normally run in the daily job. labels May 5, 2025
Copy link
Contributor

@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.

@paoloricciuti
Copy link
Contributor

The suggestion I added should be enough to fix the reactivity problem.

@JReinhold
Copy link
Contributor

@paoloricciuti always coming in clutch. 😍

@github-actions github-actions bot added the Stale label Jul 3, 2025
@github-actions github-actions bot closed this Jul 11, 2025
@JReinhold JReinhold reopened this Jul 15, 2025
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.

Reviewing changes made in this pull request

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 23, 2025

Note

Other AI code review bot(s) detected

CodeRabbit 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.

Walkthrough

Adds a new $app/state mock and exposes it via package.json exports and build extraOutputs. Introduces a static app-state-mock module exporting page, navigating, updated and setter functions (setAppState*). Adds a preview.beforeEach that reads parameters.sveltekit_experimental.state and applies it to the mock. Adds types for Page/Navigation and extends SvelteKitParameters with a state property. Renames internal store setters to setAppStores* and updates MockProvider to call them. Adds a State.svelte example component, stories, and docs updates.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Story author
  participant SB as Storybook runtime
  participant Preview as preview.beforeEach
  participant MockModule as static/app-state-mock.svelte.js
  participant UI as State.svelte
  participant Resolver as Module resolver / alias

  Dev->>SB: define story with parameters.sveltekit_experimental.state
  SB->>Preview: invoke beforeEach(ctx)
  Preview->>MockModule: call setAppStatePage / setAppStateNavigating / setAppStateUpdated
  SB->>UI: render component
  UI->>Resolver: import '$app/state'
  Resolver->>MockModule: resolve to static/app-state-mock.svelte.js
  MockModule-->>UI: provide page, navigating, updated (and setters)
  UI-->>SB: rendered output
Loading
sequenceDiagram
  autonumber
  participant Component as App component
  participant AliasPlugin as mock-sveltekit-stores alias
  participant PkgExports as package.json exports
  participant File as ./static/app-state-mock.svelte.js

  Component->>AliasPlugin: import '$app/state'
  AliasPlugin-->>PkgExports: map to internal export path
  PkgExports-->>File: resolve to ./static/app-state-mock.svelte.js
  File-->>Component: export page/navigating/updated + setters
Loading

Possibly related PRs

Suggested labels

ci:normal

Suggested reviewers

  • jonniebigodes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "SvelteKit: Add support for mocking $app/state" is a short, single sentence that clearly summarizes the primary change—adding support for mocking SvelteKit's $app/state module—without unnecessary detail or noise.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
code/frameworks/sveltekit/package.json (1)

30-46: Missing export for $app/state mock subpath (breaks alias resolution).

Alias points to '@storybook/sveltekit/internal/mocks/app/state.svelte.js' but this subpath is not exported. Consumers will get “Package subpath is not defined by "exports"”.

Add the export mapping:

   "exports": {
     ".": {
       "types": "./dist/index.d.ts",
       "default": "./dist/index.js"
     },
     "./internal/mocks/app/forms": "./dist/mocks/app/forms.js",
     "./internal/mocks/app/navigation": "./dist/mocks/app/navigation.js",
     "./internal/mocks/app/stores": "./dist/mocks/app/stores.js",
+    "./internal/mocks/app/state.svelte.js": "./dist/mocks/app/state.svelte.js",
     "./node": {
       "types": "./dist/node/index.d.ts",
       "default": "./dist/node/index.js"
     },
docs/get-started/frameworks/sveltekit.mdx (1)

101-116: Fix navigating example shape: use to (target) instead of top-level route.

$app/state.navigating expects { from?: NavigationTarget, to?: NavigationTarget, ... }. The current example uses route at the top level, which will be ignored by the mock and break consumers reading navigating.to.

Apply this diff to correct the example:

       state: {
         page: {
           data: {
             test: 'passed',
           },
         },
-        navigating: {
-          route: {
-            id: '/storybook',
-          },
-        },
+        navigating: {
+          to: {
+            route: { id: '/storybook' },
+            params: {},
+            url: new URL('http://localhost/storybook'),
+          },
+        },
         updated: true,
       },
🧹 Nitpick comments (3)
code/frameworks/sveltekit/scripts/copy-unbundled-to-dist.ts (1)

1-24: Harden ESM path handling and copy robustness (no __dirname in ESM; ensure parents exist; skip missing).

Use import.meta.url to derive __dirname, ensure parent dirs exist, and skip gracefully if source is absent.

-import { cp } from 'node:fs/promises';
-
-import { join } from 'path';
+import { cp, mkdir, access } from 'node:fs/promises';
+import { join, dirname } from 'node:path';
+import { fileURLToPath } from 'node:url';
 
-const src = join(__dirname, '..', 'src');
-const dist = join(__dirname, '..', 'dist');
+const __dirname = dirname(fileURLToPath(import.meta.url));
+const src = join(__dirname, '..', 'src');
+const dist = join(__dirname, '..', 'dist');
 
 // relative to src directory
 const PATHS_TO_COPY = ['mocks/app/state.svelte.js'];
 
 const run = async () => {
   console.log('Copying unbundled files to dist...');
   await Promise.all(
-    PATHS_TO_COPY.map((pathToCopy) =>
-      cp(join(src, pathToCopy), join(dist, pathToCopy), { recursive: true, force: true })
-    )
+    PATHS_TO_COPY.map(async (pathToCopy) => {
+      const from = join(src, pathToCopy);
+      const to = join(dist, pathToCopy);
+      try {
+        await access(from);
+      } catch {
+        console.warn(`[copy-unbundled-to-dist] Skipping missing ${pathToCopy}`);
+        return;
+      }
+      await mkdir(dirname(to), { recursive: true });
+      await cp(from, to, { force: true });
+    })
   );
   console.log('Done!');
 };
 
 run().catch((e) => {
   console.error(e);
   process.exitCode = 1;
 });
code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/State.svelte (1)

1-5: Call updated.check() in onMount to avoid SSR/initialization timing issues.

Prevents invoking browser-only logic at module init.

 <script>
-	import { page, navigating, updated } from '$app/state';
-
-	updated.check();
+	import { page, navigating, updated } from '$app/state';
+  import { onMount } from 'svelte';
+  onMount(() => {
+    updated.check();
+  });
 </script>
code/frameworks/sveltekit/src/types.ts (1)

108-115: Minor typing polish.

Consider unknown over any for data/state to improve safety.

-  data: Record<string, any>;
-  state: Record<string, any>;
-  form: any;
+  data: Record<string, unknown>;
+  state: Record<string, unknown>;
+  form: unknown;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ff83d3d and 25c6d70.

📒 Files selected for processing (11)
  • code/frameworks/sveltekit/build-config.ts (1 hunks)
  • code/frameworks/sveltekit/package.json (1 hunks)
  • code/frameworks/sveltekit/scripts/copy-unbundled-to-dist.ts (1 hunks)
  • code/frameworks/sveltekit/src/mocks/app/stores.ts (1 hunks)
  • code/frameworks/sveltekit/src/plugins/mock-sveltekit-stores.ts (1 hunks)
  • code/frameworks/sveltekit/src/preview.ts (3 hunks)
  • code/frameworks/sveltekit/src/types.ts (1 hunks)
  • code/frameworks/sveltekit/static/app-state-mock.svelte.js (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)
  • docs/get-started/frameworks/sveltekit.mdx (3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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} : Implement mock behaviors in beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preview.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 mock implementations outside of beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preview.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 be placed in beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preview.ts
📚 Learning: 2025-09-17T08:11:04.287Z
Learnt from: CR
PR: storybookjs/storybook#0
File: .cursorrules:0-0
Timestamp: 2025-09-17T08:11:04.287Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx} : Name test files as *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx

Applied to files:

  • code/frameworks/sveltekit/src/plugins/mock-sveltekit-stores.ts
⏰ 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: daily
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (11)
code/frameworks/sveltekit/package.json (1)

49-54: LGTM: publish surface update looks right.

Including static/**/* and *.d.ts aligns with the new static artifact and d.ts emission.

code/frameworks/sveltekit/scripts/copy-unbundled-to-dist.ts (1)

14-16: Echoing prior: check source exists to avoid silent failure.

Handled in the diff above with access() + warning.

code/frameworks/sveltekit/src/preview.ts (1)

152-158: beforeEach init looks good; double-check interaction with decorator.

Both decorator (stores) and beforeEach (state) set similar concepts. Verify there’s no unintended overwrite ordering.

If needed, gate one behind a parameter flag or unify into a single initializer.

code/frameworks/sveltekit/src/types.ts (1)

57-107: LGTM: localizing SvelteKit types avoids cross-package type coupling.

Shapes align with SvelteKit semantics.

code/frameworks/sveltekit/src/mocks/app/stores.ts (1)

18-25: LGTM: clearer setter names while preserving exports.

Renames improve clarity and match usage in preview.ts.

code/frameworks/sveltekit/build-config.ts (1)

48-50: Confirm build order: extraOutputs requires the unbundled state file to exist

scripts/build/utils/generate-package-json.ts spreads data.extraOutputs into package.json exports (so exported static files must already exist); no 'copy-unbundled' / 'copy-unbundled-to-dist' or other 'unbundled' copy step was found in the repo. Ensure the build runs the copy/unbundle step that creates static/app-state-mock.svelte.js before packaging.

Locations: code/frameworks/sveltekit/build-config.ts (extraOutputs), scripts/build/utils/generate-package-json.ts (uses data.extraOutputs).

docs/get-started/frameworks/sveltekit.mdx (1)

355-377: Good addition of state.* API docs.

Headings and brief descriptions for state.navigating, state.page, and state.updated look consistent.

code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/state.stories.js (4)

1-149: Verify other occurrences of navigating.route across the repo
Automated search returned no matches; manually inspect docs/templates and example files for the incorrect shape and update any occurrences.


116-118: Align NavigatingAndUpdated with correct navigating shape.

Apply this diff:

         navigating: {
-          route: {
-            id: '/storybook',
-          },
+          to: {
+            route: { id: '/storybook' },
+            params: {},
+            url: new URL('https://storybook.js.org'),
+          },
         },

84-90: Align PageAndNavigating with correct navigating shape.

Apply this diff:

         navigating: {
-          route: {
-            id: '/storybook',
-          },
+          to: {
+            route: { id: '/storybook' },
+            params: {},
+            url: new URL('https://storybook.js.org'),
+          },
         },

138-141: Align AllThree with correct navigating shape.

Apply this diff:

         navigating: {
-          route: {
-            id: '/storybook',
-          },
+          to: {
+            route: { id: '/storybook' },
+            params: {},
+            url: new URL('https://storybook.js.org'),
+          },
         },

@storybook-app-bot
Copy link

storybook-app-bot bot commented Sep 23, 2025

Package Benchmarks

Commit: e873a2f, ran on 24 September 2025 at 12:16:54 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 43 43 0
Self size 30.16 MB 30.13 MB 🎉 -24 KB 🎉
Dependency size 17.30 MB 17.30 MB 0 B
Bundle Size Analyzer Link Link

@storybook/sveltekit

Before After Difference
Dependency count 20 20 0
Self size 48 KB 58 KB 🚨 +11 KB 🚨
Dependency size 26.84 MB 26.84 MB 🎉 -500 B 🎉
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 187 187 0
Self size 890 KB 886 KB 🎉 -4 KB 🎉
Dependency size 79.75 MB 79.73 MB 🎉 -24 KB 🎉
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 169 169 0
Self size 35 KB 35 KB 0 B
Dependency size 76.18 MB 76.16 MB 🎉 -24 KB 🎉
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 44 44 0
Self size 1.55 MB 1.55 MB 0 B
Dependency size 47.46 MB 47.43 MB 🎉 -24 KB 🎉
Bundle Size Analyzer node node

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25c6d70 and 210036e.

📒 Files selected for processing (4)
  • code/frameworks/sveltekit/build-config.ts (1 hunks)
  • code/frameworks/sveltekit/package.json (1 hunks)
  • code/frameworks/sveltekit/src/preview.ts (2 hunks)
  • code/frameworks/sveltekit/static/MockProvider.svelte (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • code/frameworks/sveltekit/src/preview.ts
  • code/frameworks/sveltekit/build-config.ts
  • code/frameworks/sveltekit/package.json
🧰 Additional context used
🧠 Learnings (1)
📓 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} : Implement mock behaviors in beforeEach blocks
⏰ 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: daily
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/frameworks/sveltekit/static/MockProvider.svelte (1)

6-6: Import rename verified — exports & usages OK

setAppStoresPage, setAppStoresNavigating, and setAppStoresUpdated are exported from code/frameworks/sveltekit/src/mocks/app/stores.ts and are only consumed in code/frameworks/sveltekit/static/MockProvider.svelte; no other call sites found.

@JReinhold JReinhold self-requested a review September 24, 2025 07:00
Copy link
Contributor

@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.

🎉

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/get-started/frameworks/sveltekit.mdx (1)

101-116: Fix example: state.updated shape should be an object with current, not a boolean

This contradicts the types and the template stories. Using a boolean here will mislead users.

       state: {
         page: {
           data: {
             test: 'passed',
           },
         },
         navigating: {
           to: {
             route: { id: '/storybook' },
             params: {},
             url: new URL('http://localhost/storybook'),
           },
         },
-        updated: true,
+        updated: { current: true },
       },
🧹 Nitpick comments (1)
code/frameworks/sveltekit/src/preview.ts (1)

24-30: Guard setter calls to avoid passing undefined (safer no-op semantics)

Prevents unintentionally overwriting internal state with undefined if a story doesn’t specify a given field.

 export const beforeEach: Preview['beforeEach'] = async (ctx) => {
-  const svelteKitParameters: SvelteKitParameters = ctx.parameters?.sveltekit_experimental ?? {};
-
-  setAppStatePage(svelteKitParameters?.state?.page);
-  setAppStateNavigating(svelteKitParameters?.state?.navigating);
-  setAppStateUpdated(svelteKitParameters?.state?.updated);
+  const svelteKitParameters: SvelteKitParameters = ctx.parameters?.sveltekit_experimental ?? {};
+  const state = svelteKitParameters?.state;
+
+  if (state?.page) setAppStatePage(state.page);
+  if (state?.navigating) setAppStateNavigating(state.navigating);
+  if (state?.updated) setAppStateUpdated(state.updated);
 };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 210036e and e043fa8.

📒 Files selected for processing (3)
  • code/frameworks/sveltekit/src/preview.ts (2 hunks)
  • code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/state.stories.js (1 hunks)
  • docs/get-started/frameworks/sveltekit.mdx (3 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.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} : Group related mocks together
📚 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} : Implement mock behaviors in beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preview.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 mock implementations outside of beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preview.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 be placed in beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preview.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/src/preview.ts
🧬 Code graph analysis (1)
code/frameworks/sveltekit/src/preview.ts (1)
code/frameworks/sveltekit/src/types.ts (1)
  • SvelteKitParameters (108-133)
🔇 Additional comments (5)
code/frameworks/sveltekit/src/preview.ts (1)

3-7: Verify internal export/alias for state mock resolves in consumers

Ensure @storybook/sveltekit/internal/mocks/app/state.svelte.js is exported in package.json and mapped in build config (extraOutputs), otherwise this import can fail downstream.

#!/bin/bash
set -euo pipefail

echo "Checking package export for internal state mock..."
rg -nP '"\./internal/mocks/app/state\.svelte\.js"\s*:' code/frameworks/sveltekit/package.json -C2 || true

echo -e "\nChecking build extraOutputs mapping..."
rg -nP 'extraOutputs.*state\.svelte\.js' code/frameworks/sveltekit -C3 || true

echo -e "\nChecking for Vite alias to \$app/state..."
rg -nP '\$app/state|mocks/app/state\.svelte\.js' code/frameworks/sveltekit/src -C3 || true
code/frameworks/sveltekit/template/stories_svelte-kit-skeleton-ts/modules/state.stories.js (3)

10-34: LGTM: state.page fixture looks correct and consistent with types

Data, form, params, route.id, status, and url are well-shaped.


36-63: LGTM: state.navigating shape is valid

Includes from, to, allowed type, willUnload, delta, and complete: Promise<void>.


65-75: LGTM: state.updated uses { current: boolean }

Matches the types and should interop with the setters.

docs/get-started/frameworks/sveltekit.mdx (1)

83-85: Confirm minimal SvelteKit version for $app/state — v2.12 is correct

SvelteKit introduced $app/state in v2.12, so "Requires SvelteKit v2.12 or newer" is accurate.

Comment on lines +3 to +7
import {
setAppStateNavigating,
setAppStatePage,
setAppStateUpdated, // @ts-expect-error no declaration file for this JS module
} from '@storybook/sveltekit/internal/mocks/app/state.svelte.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix @ts-expect-error placement; TypeScript ignores inline trailing annotations

// @ts-expect-error must be on the preceding line of the failing statement. As written, it won’t suppress the error for this import.

Apply:

-import {
-  setAppStateNavigating,
-  setAppStatePage,
-  setAppStateUpdated, // @ts-expect-error no declaration file for this JS module
-} from '@storybook/sveltekit/internal/mocks/app/state.svelte.js';
+// @ts-expect-error no declaration file for this JS module
+import {
+  setAppStateNavigating,
+  setAppStatePage,
+  setAppStateUpdated,
+} from '@storybook/sveltekit/internal/mocks/app/state.svelte.js';

Optional (preferred): add a declaration file to remove the need for ts-expect-error:

// code/frameworks/sveltekit/src/mocks/app/state.svelte.d.ts
declare module '@storybook/sveltekit/internal/mocks/app/state.svelte.js' {
  import type { Page, Navigation } from '../../types'; // adjust path if needed

  export function setAppStatePage(page?: Partial<Page> | undefined): void;
  export function setAppStateNavigating(navigating?: Partial<Navigation> | undefined): void;
  export function setAppStateUpdated(updated?: { current: boolean } | undefined): void;
}
🤖 Prompt for AI Agents
In code/frameworks/sveltekit/src/preview.ts around lines 3 to 7, the trailing
"// @ts-expect-error" on the import does not suppress the TypeScript error
because the annotation must be on the line immediately before the failing
statement; move the "// @ts-expect-error no declaration file for this JS module"
to its own line immediately above the import statement so TS recognizes it, and
optionally add a declaration file at
code/frameworks/sveltekit/src/mocks/app/state.svelte.d.ts declaring the module
and exported functions to remove the need for the ts-expect-error.

@jonniebigodes jonniebigodes removed the request for review from kylegach September 24, 2025 11:16
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
code/frameworks/sveltekit/src/preset.ts (1)

38-43: Optional: unify optimizeViteDeps entry style

Other entries omit extensions; if exports also support the extensionless specifier for state, consider aligning for consistency. If the .svelte.js suffix is required by the exports map, keep as-is.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ef0ff8 and e873a2f.

📒 Files selected for processing (1)
  • code/frameworks/sveltekit/src/preset.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
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
📚 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/src/preset.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/src/preset.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/src/preset.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/src/preset.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/src/preset.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/src/preset.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 be placed in beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preset.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 access and implement mock behaviors

Applied to files:

  • code/frameworks/sveltekit/src/preset.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 direct function mocking without vi.mocked()

Applied to files:

  • code/frameworks/sveltekit/src/preset.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 mock implementations outside of beforeEach blocks

Applied to files:

  • code/frameworks/sveltekit/src/preset.ts
⏰ 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: daily
  • GitHub Check: Core Unit Tests, windows-latest
🔇 Additional comments (1)
code/frameworks/sveltekit/src/preset.ts (1)

42-42: Approve — .svelte.js specifier consistent across exports/imports

package.json, preview.ts, and mock-sveltekit-stores.ts all reference '@storybook/sveltekit/internal/mocks/app/state.svelte.js' (extension matches), so Vite prebundling should work.

@JReinhold JReinhold merged commit 6b9ff97 into storybookjs:next Sep 24, 2025
90 checks passed
@github-actions github-actions bot mentioned this pull request Sep 24, 2025
18 tasks
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. feature request sveltekit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants