Repository navigation
TanStack: Resolve mock redirects through Vite's resolver - #35501
huang-julien merged 6 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe TanStack React module interception plugin now resolves intercepted mock modules through Vite, falling back to configured mock paths when resolution is unavailable. Tests cover interception, guard conditions, resolver fallback, and unrelated imports. ChangesTanStack mock module resolution
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/frameworks/tanstack-react/src/plugins/module-interception.test.ts (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
.toEqual()to avoid type assertions.Since the mocked
resolvefunction directly returns an object structured as{ id: string }, you can eliminate the(result as { id: string })type assertion by checking the deep equality of the entire returned object.Note: This pattern also applies to similar assertions on lines 47, 76, 81, and 88.
💡 Proposed refactor
- expect((result as { id: string }).id).toBe('/mocks/react-router.ts?v=abc123'); + expect(result).toEqual({ id: '/mocks/react-router.ts?v=abc123' });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/frameworks/tanstack-react/src/plugins/module-interception.test.ts` at line 42, Update the assertions for the mocked resolve results to use deep equality against the expected `{ id: string }` object, removing the `(result as { id: string })` type assertions. Apply this consistently to the similar assertions referenced on lines 47, 76, 81, and 88.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@code/frameworks/tanstack-react/src/plugins/module-interception.test.ts`:
- Line 42: Update the assertions for the mocked resolve results to use deep
equality against the expected `{ id: string }` object, removing the `(result as
{ id: string })` type assertions. Apply this consistently to the similar
assertions referenced on lines 47, 76, 81, and 88.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35dc34c9-1de9-4535-9194-781592f0c15e
📒 Files selected for processing (1)
code/frameworks/tanstack-react/src/plugins/module-interception.test.ts
The interception plugin returned raw file paths for the mock modules. Direct user imports of the same mocks (the documented '@storybook/tanstack-react/react-router' spy-assertion path) resolve through Vite, which stamps a version query on the id, so the same file registered as two module instances and user assertions saw spies the app never called. Resolve through this.resolve() so both import paths share one module. Found by asserting on the useNavigate spy in the conformance suite.
Add unit tests for the interception plugin: router and start mocks resolve through Vite (so direct mock imports share one module instance), the circular guard for mock importers, intercepted patterns, and the raw-path fallback.
de5d6e2 to
b7b5488
Compare
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 186 | 186 | 0 |
| Self size | 79 KB | 79 KB | 🚨 +49 B 🚨 |
| Dependency size | 35.73 MB | 36.13 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 21.39 MB | 21.31 MB | 🎉 -81 KB 🎉 |
| Dependency size | 36.75 MB | 31.22 MB | 🎉 -5.53 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 160 KB | 160 KB | 🎉 -54 B 🎉 |
| Dependency size | 30.13 MB | 30.52 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 36 | 36 | 0 |
| Self size | 33.41 MB | 33.41 MB | 🎉 -2 KB 🎉 |
| Dependency size | 21.34 MB | 15.80 MB | 🎉 -5.54 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 13 KB | 13 KB | 0 B |
| Dependency size | 31.08 MB | 31.47 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 531 | 531 | 0 |
| Self size | 641 KB | 640 KB | 🎉 -1 KB 🎉 |
| Dependency size | 62.41 MB | 62.81 MB | 🚨 +404 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 94 | 93 | 🎉 -1 🎉 |
| Self size | 1.42 MB | 1.37 MB | 🎉 -48 KB 🎉 |
| Dependency size | 23.91 MB | 23.85 MB | 🎉 -62 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 125 | 122 | 🎉 -3 🎉 |
| Self size | 29 KB | 29 KB | 0 B |
| Dependency size | 25.90 MB | 24.91 MB | 🎉 -982 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 32 KB | 32 KB | 🎉 -10 B 🎉 |
| Dependency size | 21.22 MB | 21.62 MB | 🚨 +398 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 272 | 272 | 0 |
| Self size | 23 KB | 23 KB | 🎉 -12 B 🎉 |
| Dependency size | 48.19 MB | 48.59 MB | 🚨 +404 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/server-webpack5
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 15 KB | 15 KB | 0 B |
| Dependency size | 37.00 MB | 37.39 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/svelte-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 20 | 20 | 0 |
| Self size | 54 KB | 54 KB | 🎉 -10 B 🎉 |
| Dependency size | 26.65 MB | 27.04 MB | 🚨 +386 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/sveltekit
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 21 | 21 | 0 |
| Self size | 56 KB | 56 KB | 0 B |
| Dependency size | 26.71 MB | 27.09 MB | 🚨 +386 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/tanstack-react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 84 | 84 | 0 |
| Self size | 113 KB | 113 KB | 🎉 -169 B 🎉 |
| Dependency size | 21.25 MB | 21.65 MB | 🚨 +398 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 103 | 102 | 🎉 -1 🎉 |
| Self size | 34 KB | 34 KB | 🎉 -252 B 🎉 |
| Dependency size | 43.11 MB | 43.04 MB | 🎉 -73 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 205 | 0 |
| Self size | 827 KB | 827 KB | 🚨 +52 B 🚨 |
| Dependency size | 91.60 MB | 85.99 MB | 🎉 -5.61 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 90.08 MB | 84.46 MB | 🎉 -5.61 MB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 74 | 0 |
| Self size | 1.09 MB | 1.09 MB | 🚨 +53 B 🚨 |
| Dependency size | 58.14 MB | 52.52 MB | 🎉 -5.61 MB 🎉 |
| Bundle Size Analyzer | node | node |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 161 | 161 | 0 |
| Self size | 19 KB | 19 KB | 🚨 +25 B 🚨 |
| Dependency size | 32.70 MB | 33.09 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/react
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 59 | 59 | 0 |
| Self size | 1.48 MB | 1.49 MB | 🚨 +12 KB 🚨 |
| Dependency size | 12.30 MB | 12.30 MB | 🚨 +143 B 🚨 |
| Bundle Size Analyzer | Link | Link |
…act-mock-module-resolution TanStack: Resolve mock redirects through Vite's resolver (cherry picked from commit cdd4f9e)
Follows #35500
What I did
Spy assertions through
@storybook/tanstack-react/react-router(the documented module for asserting on the automatic mocks) never saw calls.The interception plugin returned raw file paths for mock redirects, while a direct import of the same module resolves through Vite, which stamps a version query on the id. Same file, two module instances: the spy a story imports is not the spy the app calls.
The plugin now resolves through
this.resolve(), so both import paths share one module.Covered end to end rather than by a unit test: reproducing the split requires Vite's resolver and import analysis, which unit tests stub away.
Manual testing
cd code/frameworks/tanstack-react && yarn vitest run(54 tests).End to end: story-level conformance suite against a TanStack Router SPA
and a TanStack Start app:
https://github.com/unpunnyfuns/storybook-tanstack-conformance.
A story clicks a pagination button and asserts
expect(useNavigate).toHaveBeenCalled()withuseNavigateimported from@storybook/tanstack-react/react-router; it fails onnextand passes with this fix.AI disclosure
Root-cause analysis, fix, and tests developed with Claude (Claude Code), reviewed and submitted by me.
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Tests
resolveIdinterception behavior, including cases where interception is skipped, resolver fallback is used, and unrelated imports remain unaffected.