fix(solid-router): make useLoaderDeps return Accessor for reactivity #5348
Conversation
- Update useLoaderDeps return type to Accessor for consistency with Solid.js patterns - Add import for Accessor type from solid-js - Reorganize imports following project conventions (types first, then implementations) - Add comprehensive tests for useLoaderDeps Accessor behavior - Update type tests to reflect new Accessor return type
WalkthroughReturn types for useLoaderDeps in the Solid Router were changed to be wrapped in Solid's Accessor; the implementation and types were updated accordingly, and tests were added/updated to expect and verify the Accessor-returning API. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Consumer Component
participant RouteAPI as Route / getRouteApi
participant useLoaderDeps as useLoaderDeps (API)
rect rgb(230, 250, 230)
Note over useLoaderDeps: Public API now returns an Accessor (function)
end
Component->>RouteAPI: call Route.useLoaderDeps() or api.useLoaderDeps()
RouteAPI->>useLoaderDeps: return Accessor<LoaderDepsResult>
alt when consumer needs data
Component->>useLoaderDeps: invoke Accessor() -> ()
useLoaderDeps-->>Component: LoaderDepsResult (object)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/{react-router,solid-router}/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/solid-router/tests/route.test.tsx (2)
207-227: LGTM: Comprehensive test for Accessor behavior.The test correctly verifies that
useLoaderDeps()returns an Accessor (function) and that calling it yields the expectedloaderDepsresult. The test structure is clear and covers the direct route usage path.Minor refinement: the
loaderDepscallback has an unused{ search }parameter. Consider removing it for clarity:- loaderDeps: ({ search }) => ({ testDep: 'value' }), + loaderDeps: () => ({ testDep: 'value' }),
229-249: LGTM: Route API test covers Accessor usage.This test validates the same Accessor behavior via
getRouteApi('/'), ensuring both access paths (direct route and route API) behave consistently.Minor refinement: the
loaderDepscallback has an unused{ search }parameter. Consider removing it for clarity:- loaderDeps: ({ search }) => ({ testDep: 'api-value' }), + loaderDeps: () => ({ testDep: 'api-value' }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/solid-router/src/useLoaderDeps.tsx(2 hunks)packages/solid-router/tests/route.test.tsx(2 hunks)packages/solid-router/tests/routeApi.test-d.tsx(2 hunks)
🔇 Additional comments (6)
packages/solid-router/tests/route.test.tsx (1)
2-2: LGTM: Import reorganization.The vitest import has been reformatted with no change to the imported symbols.
packages/solid-router/tests/routeApi.test-d.tsx (2)
1-2: LGTM: Type imports added for Accessor support.The added imports (
MakeRouteMatch,UseNavigateResult,Accessor) are necessary for the updated type tests and align with the new Accessor-wrapped return types.
91-95: LGTM: Type test updated for Accessor return type.The type expectation correctly verifies that
useLoaderDeps()returnsAccessor<{ dep: number }>instead of a plain object, consistent with the implementation change.packages/solid-router/src/useLoaderDeps.tsx (3)
8-9: LGTM: Accessor type imported for reactivity support.The
Accessortype import fromsolid-jsis necessary for the new return type and aligns with Solid.js reactivity patterns. The existinguseMatchimport is retained.
26-31: LGTM: UseLoaderDepsRoute return type wrapped in Accessor.The return type correctly wraps the result in
Accessor<...>, making the API reactive and consistent with Solid.js patterns.
33-47: LGTM: useLoaderDeps implementation updated to return Accessor.The implementation correctly:
- Updates the return type to
Accessor<UseLoaderDepsResult<...>>- Delegates to
useMatch, which already returns an Accessor- Applies the
selecttransformation to extractloaderDepsfrom the match state- Uses a type assertion on Line 46 to align the return type (safe because
useMatchreturnsAccessorand the select function transforms the wrapped value)
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
Docstrings generation was requested by @edmolima. * #5348 (comment) The following files were modified: * `packages/solid-router/src/useLoaderDeps.tsx`
|
Note Generated docstrings for this pull request at #5349 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx affected --targets=test:eslint,test:unit,tes... |
❌ Failed | 5m 37s | View ↗ |
nx run-many --target=build --exclude=examples/*... |
✅ Succeeded | 25s | View ↗ |
☁️ Nx Cloud last updated this comment at 2025-10-04 18:45:50 UTC
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
Fixes #5343
Summary by CodeRabbit