Repository navigation
TanStack: Respect routeOverrides component overrides in stories - #35497
huang-julien merged 5 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:
📝 WalkthroughWalkthroughTanStack story routing now resolves component overrides using original route identifiers across duplicated, provided, and synthetic route trees. Route override typing, injection logic, and regression tests were updated. ChangesRoute override matching
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 |
1c80658 to
5fce2c5
Compare
5fce2c5 to
294749e
Compare
The guard in injectStoryComponent that respects a user's component override looked the override up by the cloned leaf's id getter, which is init-backed and undefined before createRouter() runs — the lookup always missed and the story render overwrote the component the override had installed. Resolve the original (pre-clone) id via the duplication byId map and use it as the key.
…t test Review feedback on storybookjs#35465: keep plain Vitest unit tests free of the Storybook test layer.
The plain-options branch builds the story child directly instead of going through duplicateRouteTree, so a story-supplied component/loader override was never merged onto it and got dropped. Merge the leaf's override into the synthetic child, matching the route-instance branches.
RouteOverrideOptions exposed loader/beforeLoad/etc. but not component, even though a component override is supported. Add the field so stories can override a route's component without casting.
cce8a23 to
783345f
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 | 🚨 +1 B 🚨 |
| Dependency size | 35.79 MB | 36.18 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/angular
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 160 KB | 160 KB | 0 B |
| Dependency size | 30.25 MB | 30.64 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/ember
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 185 | 185 | 0 |
| Self size | 13 KB | 13 KB | 🚨 +18 B 🚨 |
| Dependency size | 31.14 MB | 31.53 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 531 | 531 | 0 |
| Self size | 640 KB | 640 KB | 🚨 +120 B 🚨 |
| Dependency size | 62.47 MB | 62.86 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 94 | 93 | 🎉 -1 🎉 |
| Self size | 1.37 MB | 1.37 MB | 🚨 +48 B 🚨 |
| Dependency size | 23.91 MB | 23.84 MB | 🎉 -74 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 | 🚨 +18 B 🚨 |
| Dependency size | 25.90 MB | 24.90 MB | 🎉 -994 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 83 | 83 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +8 B 🚨 |
| Dependency size | 21.23 MB | 21.61 MB | 🚨 +386 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.25 MB | 48.64 MB | 🚨 +392 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.05 MB | 37.44 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 | 🎉 -22 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.70 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 | 112 KB | 🎉 -367 B 🎉 |
| Dependency size | 21.26 MB | 21.64 MB | 🚨 +386 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/vue3-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 103 | 102 | 🎉 -1 🎉 |
| Self size | 34 KB | 34 KB | 🎉 -37 B 🎉 |
| Dependency size | 43.10 MB | 43.03 MB | 🎉 -74 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/preset-react-webpack
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 161 | 161 | 0 |
| Self size | 19 KB | 19 KB | 🚨 +25 B 🚨 |
| Dependency size | 32.76 MB | 33.15 MB | 🚨 +392 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
…act-route-overrides-by-id TanStack: Respect routeOverrides component overrides in stories (cherry picked from commit 52cd3b2)
Follow-up to #35465.
What I did
routeOverridescomponent overrides were silently ignored. The guard ininjectStoryComponentlooked up the override with the cloned leaf'sidgetter, which is undefined beforeinit().The lookup never matched and the story component always replaced the component the override had installed.
The lookup now resolves the original (pre-clone) route id through the duplication map, which is the key users write in
routeOverrides.The second commit applies the unit-test review note from #35465: use
vi.fn()instead offnfromstorybook/test.Manual testing
cd code/frameworks/tanstack-react && yarn vitest run(55 tests).and a TanStack Start app:
https://github.com/unpunnyfuns/storybook-tanstack-conformance. The
routeOverridesscenarios pass with this fix applied.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
routeOverridesso injected route components correctly honor overrides after route duplication and duringrouter.load(), including pathless and synthetic route structures.New Features
RouteOverrideOptionswith an optionalcomponentoverride, with improved typing for file-based pre-loader routes.Tests
createStoryRouterapplyingrouteOverrides.