Core: Invalidate cache on Storybook version upgrade#33717
Conversation
- Modified resolvePathInStorybookCache to include storybook version in cache path
- Added comprehensive unit tests for the new versioned cache functionality
- Cache path now includes version: node_modules/.cache/storybook/{version}/{sub}/{file}
- This ensures automatic cache invalidation when upgrading Storybook versions
Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
…allback test - Use path.join() for all expected path assertions to ensure cross-platform compatibility - Add test for 'unknown' fallback when storybook version is not available - Improve path comparison logic to be platform-agnostic - All 7 tests now pass Co-authored-by: valentinpalkovic <5889929+valentinpalkovic@users.noreply.github.com>
|
View your CI Pipeline Execution ↗ for commit 22690a6
☁️ Nx Cloud last updated this comment at |
📝 WalkthroughWalkthroughUtility updated to include the Storybook version in computed cache paths and accept an optional sub-directory; tests added to validate behaviors including empathic cache usage, cwd fallback, default subdir, empty names, version consistency, and unknown-version fallback. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@code/core/src/common/utils/resolve-path-in-sb-cache.test.ts`:
- Around line 20-105: The tests for resolvePathInStorybookCache currently set
mock return values inline; move all vi.mocked(pkg.cache).mockReturnValue(...)
calls and any temporary assignments to versions.storybook into beforeEach hooks
(use nested describe blocks for scenarios: e.g., "when cache available", "when
cache unavailable", "when version missing") so test bodies only assert behavior;
ensure each beforeEach sets the appropriate pkg.cache return and/or
versions.storybook value and use vi.clearAllMocks() (and restore original
versions.storybook value in an afterEach if modified) to avoid cross-test
pollution.
| 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'; |
There was a problem hiding this comment.
🧩 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 in beforeEach
vi.mock() is hoisted and runs before imports, so using it inside beforeEach usually doesn’t do what you expect. If you need shared variables for a hoisted mock factory, define them with vi.hoisted. Also note: modules imported in a setupFiles file may already be cached by the time a test runs, making later mocking ineffective (use vi.resetModules() if you must). [1]
2) Use beforeEach for per-test state, not for “declaring mocks”
Good beforeEach usage:
- reset counters / data fixtures
- set default return values for already-created mocks/spies
- re-stub globals that tests mutate (or do it in
afterEachcleanup)
Vitest explicitly warns to clear/restore mocks between tests to avoid state leaking. [2]
3) Clean up consistently: prefer afterEach + the right reset function
Pick 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() in afterEach (or enabling the equivalent config) when you rely on spies. [3]
4) Use setupFiles for global test environment defaults, not heavy mocking
setupFiles runs 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:
- Per-test-file mocks: live next to the test (easy to see what’s mocked)
- Reusable mock builders/factories:
test/factories/*(pure functions returning data / mock objects) - Shared environment stubs (global APIs): in
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:
- inject dependencies (pass collaborators in)
- wrap third-party modules behind your own small “adapter” module, and only mock that adapter
This reduces hoisting/caching surprises and makes mocks more local and readable. [1]
Sources: Vitest docs on vi.mock hoisting/caching and vi.hoisted [1], mocking guide warning about clearing/restoring [2], vi.clearAllMocks/resetAllMocks/restoreAllMocks semantics and afterEach suggestion [3], migration note on changed restoreAllMocks behavior [4], setupFiles behavior/constraints [5], and vi.stubGlobal example [6].
🏁 Script executed:
head -20 code/core/src/common/utils/resolve-path-in-sb-cache.test.tsRepository: storybookjs/storybook
Length of output: 514
Move mock behavior configuration into beforeEach hooks; avoid setting return values inline in test cases.
Mock behavior (via .mockReturnValue() and property assignment) should live in beforeEach blocks to keep state management consistent and separate from test logic. Use nested describe blocks with their own beforeEach to 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
In `@code/core/src/common/utils/resolve-path-in-sb-cache.test.ts` around lines 20
- 105, The tests for resolvePathInStorybookCache currently set mock return
values inline; move all vi.mocked(pkg.cache).mockReturnValue(...) calls and any
temporary assignments to versions.storybook into beforeEach hooks (use nested
describe blocks for scenarios: e.g., "when cache available", "when cache
unavailable", "when version missing") so test bodies only assert behavior;
ensure each beforeEach sets the appropriate pkg.cache return and/or
versions.storybook value and use vi.clearAllMocks() (and restore original
versions.storybook value in an afterEach if modified) to avoid cross-test
pollution.
…tion-on-upgrade Core: Invalidate cache on Storybook version upgrade (cherry picked from commit 5176cb7)
Closes #33711
Problem
Users encounter build errors and runtime issues after upgrading Storybook due to stale cache. Currently requires manual deletion of
node_modules/.cache/storybook.Changes
Cache path now includes version
Modified
resolvePathInStorybookCache()to inject version fromversions.storybook:Tests added
path.join()Impact
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
Summary by CodeRabbit
Tests
Chores