-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Core: Invalidate cache on Storybook version upgrade #33717
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
Merged
valentinpalkovic
merged 6 commits into
next
from
copilot/fix-cache-invalidation-on-upgrade
Feb 3, 2026
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0c50dad
Initial plan
Copilot 1098e81
feat: Add version-based cache invalidation to prevent stale cache issues
Copilot e207b82
test: Fix test expectation for empty file path normalization
Copilot 2c2cfed
test: Fix platform-specific path separators and add unknown version f…
Copilot f315f64
Fix type error
valentinpalkovic 22690a6
Merge branch 'next' into copilot/fix-cache-invalidation-on-upgrade
valentinpalkovic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
107 changes: 107 additions & 0 deletions
107
code/core/src/common/utils/resolve-path-in-sb-cache.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| import { join } from 'node:path'; | ||
|
|
||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import * as pkg from 'empathic/package'; | ||
|
|
||
| import versions from '../versions'; | ||
| import { resolvePathInStorybookCache } from './resolve-path-in-sb-cache'; | ||
|
|
||
| vi.mock('empathic/package', () => ({ | ||
| cache: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock('../versions', () => ({ | ||
| default: { | ||
| storybook: '10.3.0-alpha.1', | ||
| }, | ||
| })); | ||
|
|
||
| describe('resolvePathInStorybookCache', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('should include version in the cache path when using empathic cache', () => { | ||
| const mockCacheDir = '/mock/node_modules/.cache/storybook'; | ||
| vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); | ||
|
|
||
| const result = resolvePathInStorybookCache('test-file', 'test-sub'); | ||
|
|
||
| expect(result).toContain(versions.storybook); | ||
| expect(result).toBe(join(mockCacheDir, versions.storybook, 'test-sub', 'test-file')); | ||
| }); | ||
|
|
||
| it('should include version in the cache path when falling back to cwd', () => { | ||
| vi.mocked(pkg.cache).mockReturnValue(undefined); | ||
| const cwd = process.cwd(); | ||
|
|
||
| const result = resolvePathInStorybookCache('test-file', 'test-sub'); | ||
|
|
||
| expect(result).toContain(versions.storybook); | ||
| expect(result).toBe( | ||
| join(cwd, 'node_modules', '.cache', 'storybook', versions.storybook, 'test-sub', 'test-file') | ||
| ); | ||
| }); | ||
|
|
||
| it('should use default sub directory when not provided', () => { | ||
| const mockCacheDir = '/mock/node_modules/.cache/storybook'; | ||
| vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); | ||
|
|
||
| const result = resolvePathInStorybookCache('test-file'); | ||
|
|
||
| expect(result).toBe(join(mockCacheDir, versions.storybook, 'default', 'test-file')); | ||
| }); | ||
|
|
||
| it('should handle empty file or directory name', () => { | ||
| const mockCacheDir = '/mock/node_modules/.cache/storybook'; | ||
| vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); | ||
|
|
||
| const result = resolvePathInStorybookCache('', 'test-sub'); | ||
|
|
||
| // Note: path.join() normalizes away the trailing slash for empty strings | ||
| expect(result).toBe(join(mockCacheDir, versions.storybook, 'test-sub')); | ||
| }); | ||
|
|
||
| it('should create consistent paths for the same version', () => { | ||
| const mockCacheDir = '/mock/node_modules/.cache/storybook'; | ||
| vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); | ||
|
|
||
| const result1 = resolvePathInStorybookCache('file1', 'sub1'); | ||
| const result2 = resolvePathInStorybookCache('file2', 'sub1'); | ||
|
|
||
| expect(result1).toContain(versions.storybook); | ||
| expect(result2).toContain(versions.storybook); | ||
| // Verify both paths share the same base directory by comparing parent directories | ||
| const parent1 = result1.substring(0, result1.lastIndexOf(join('sub1', 'file1'))); | ||
| const parent2 = result2.substring(0, result2.lastIndexOf(join('sub1', 'file2'))); | ||
| expect(parent1).toBe(parent2); | ||
| }); | ||
|
|
||
| it('should handle different subdirectories', () => { | ||
| const mockCacheDir = '/mock/node_modules/.cache/storybook'; | ||
| vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); | ||
|
|
||
| const result1 = resolvePathInStorybookCache('test-file', 'dev-server'); | ||
| const result2 = resolvePathInStorybookCache('test-file', 'telemetry'); | ||
|
|
||
| expect(result1).toBe(join(mockCacheDir, versions.storybook, 'dev-server', 'test-file')); | ||
| expect(result2).toBe(join(mockCacheDir, versions.storybook, 'telemetry', 'test-file')); | ||
| }); | ||
|
|
||
| it('should use "unknown" as version when storybook version is not available', () => { | ||
| const mockCacheDir = '/mock/node_modules/.cache/storybook'; | ||
| vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); | ||
|
|
||
| // Mock the versions module to return a falsy value | ||
| vi.mocked(versions).storybook = '' as any; | ||
|
|
||
| const result = resolvePathInStorybookCache('test-file', 'test-sub'); | ||
|
|
||
| expect(result).toContain('unknown'); | ||
| expect(result).toBe(join(mockCacheDir, 'unknown', 'test-sub', 'test-file')); | ||
|
|
||
| // Reset the mock | ||
| vi.mocked(versions).storybook = '10.3.0-alpha.1'; | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
🧩 Analysis chain
🌐 Web query:
Vitest beforeEach mock setup best practices organizing test mocks💡 Result:
Vitest mock setup best practices (esp. around
beforeEach)1) Put module mocks (
vi.mock) at top-level, not inbeforeEachvi.mock()is hoisted and runs before imports, so using it insidebeforeEachusually doesn’t do what you expect. If you need shared variables for a hoisted mock factory, define them withvi.hoisted. Also note: modules imported in asetupFilesfile may already be cached by the time a test runs, making later mocking ineffective (usevi.resetModules()if you must). [1]2) Use
beforeEachfor per-test state, not for “declaring mocks”Good
beforeEachusage:afterEachcleanup)Vitest explicitly warns to clear/restore mocks between tests to avoid state leaking. [2]
3) Clean up consistently: prefer
afterEach+ the right reset functionPick the cleanup level you actually need:
vi.clearAllMocks()→ clears call history only (keeps implementations) [3]vi.resetAllMocks()→ clears history and resets implementations [3]vi.restoreAllMocks()→ restores original implementations forvi.spyOnspies (and in newer Vitest behavior, it doesn’t “reset everything” the way people often assume) [3][4]Vitest docs even suggest calling
vi.restoreAllMocks()inafterEach(or enabling the equivalent config) when you rely on spies. [3]4) Use
setupFilesfor global test environment defaults, not heavy mockingsetupFilesruns before each test file (in that worker process). It’s ideal for things like registering custom matchers, test-library config, global polyfills/stubs—but be careful: if isolation is off, you can end up doing repeated work in the same global scope. [5]5) Organize mocks by intent
A common, maintainable layout:
test/factories/*(pure functions returning data / mock objects)setupFiles(e.g.,vi.stubGlobal(...)) [6]6) Prefer dependency boundaries you can swap
If you find yourself re-mocking deep imports in
beforeEach, it’s often simpler to:This reduces hoisting/caching surprises and makes mocks more local and readable. [1]
Sources: Vitest docs on
vi.mockhoisting/caching andvi.hoisted[1], mocking guide warning about clearing/restoring [2],vi.clearAllMocks/resetAllMocks/restoreAllMockssemantics andafterEachsuggestion [3], migration note on changedrestoreAllMocksbehavior [4],setupFilesbehavior/constraints [5], andvi.stubGlobalexample [6].🏁 Script executed:
Repository: storybookjs/storybook
Length of output: 514
Move mock behavior configuration into
beforeEachhooks; avoid setting return values inline in test cases.Mock behavior (via
.mockReturnValue()and property assignment) should live inbeforeEachblocks to keep state management consistent and separate from test logic. Use nesteddescribeblocks with their ownbeforeEachto organize scenarios with different mock configurations (e.g., when cache is unavailable, or version is missing).♻️ Suggested refactor sketch
describe('resolvePathInStorybookCache', () => { + const mockCacheDir = '/mock/node_modules/.cache/storybook'; + beforeEach(() => { vi.clearAllMocks(); + vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); + vi.mocked(versions).storybook = '10.3.0-alpha.1'; }); it('should include version in the cache path when using empathic cache', () => { - const mockCacheDir = '/mock/node_modules/.cache/storybook'; - vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); const result = resolvePathInStorybookCache('test-file', 'test-sub'); ... }); - it('should include version in the cache path when falling back to cwd', () => { - vi.mocked(pkg.cache).mockReturnValue(undefined); - const cwd = process.cwd(); - ... - }); + describe('when empathic cache is unavailable', () => { + beforeEach(() => { + vi.mocked(pkg.cache).mockReturnValue(undefined); + }); + + it('should include version in the cache path when falling back to cwd', () => { + const cwd = process.cwd(); + ... + }); + }); - it('should use "unknown" as version when storybook version is not available', () => { - const mockCacheDir = '/mock/node_modules/.cache/storybook'; - vi.mocked(pkg.cache).mockReturnValue(mockCacheDir); - vi.mocked(versions).storybook = '' as any; + describe('when storybook version is not available', () => { + beforeEach(() => { + vi.mocked(versions).storybook = '' as any; + }); + + it('should use "unknown" as version when storybook version is not available', () => { const result = resolvePathInStorybookCache('test-file', 'test-sub'); ... - vi.mocked(versions).storybook = '10.3.0-alpha.1'; - }); + }); + }); });🤖 Prompt for AI Agents