fix(solid-router): RouteProvider Wrap and Match InnerWrap support propagating context to their children #5033
Conversation
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
…pagating context to their children
WalkthroughRefactors RouterProvider and Matches to always apply an optional wrapper (Wrap/InnerWrap or SafeFragment), inlines Suspense fallback rendering, and adds tests ensuring Wrap/InnerWrap-provided Solid context is visible in route components and pending fallbacks. No public API signature changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant RouterProvider
participant OptionalWrap as Wrap/SafeFragment
participant RouterCtx
participant Matches
participant OptionalInner as InnerWrap/SafeFragment
participant Suspense
participant RouteComponent
participant PendingFallback
App->>RouterProvider: render(router, children)
RouterProvider->>OptionalWrap: OptionalWrap(children)
OptionalWrap->>RouterCtx: provide router context
RouterCtx->>Matches: render routes/outlet
Matches->>OptionalInner: OptionalInner(ResolvedSuspense)
OptionalInner->>Suspense: render route content
alt loader pending
Suspense->>PendingFallback: render defaultPendingComponent (wrapped)
else loaded
Suspense->>RouteComponent: render route component (wrapped)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit ee01696
☁️ Nx Cloud last updated this comment at |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-plugin
@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-plugin
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-server-functions-client
@tanstack/start-server-functions-fetcher
@tanstack/start-server-functions-server
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![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 (7)
packages/solid-router/src/RouterProvider.tsx (1)
33-41: Avoid potential breaking change: let Wrap access router context; also tighten typing and prefer nullish coalescing.Currently, Wrap is rendered outside the router context provider, which prevents Wrap from calling useRouter. Invert the order so Wrap still wraps all children (including pending fallbacks) while retaining access to router context. Also, explicitly type the wrapper to “accepts children” and use “??” over “||”.
- const OptionalWrapper = router.options.Wrap || SafeFragment + const OptionalWrapper = + (router.options.Wrap ?? SafeFragment) as Solid.Component<{ + children?: Solid.JSX.Element + }> - return ( - <OptionalWrapper> - <routerContext.Provider value={router as AnyRouter}> - {children()} - </routerContext.Provider> - </OptionalWrapper> - ) + return ( + <routerContext.Provider value={router as AnyRouter}> + <OptionalWrapper>{children()}</OptionalWrapper> + </routerContext.Provider> + )Action: Please confirm there are no known Wrap implementations relying on useRouter; if there are, the above change is necessary to avoid a regression. If not, consider documenting that Wrap must not read router context.
packages/solid-router/src/Matches.tsx (2)
47-62: InnerWrap placement is correct; small typing/coalescing tweak suggested.Great fix—placing InnerWrap outside Suspense ensures defaultPendingComponent inherits the context. Suggest minor type and “??” coalescing adjustment for robustness.
- const OptionalWrapper = router.options.InnerWrap || SafeFragment + const OptionalWrapper = + (router.options.InnerWrap ?? SafeFragment) as Solid.Component<{ + children?: Solid.JSX.Element + }>
51-56: Optional rendering micro-tweak.Using a JSX element for defaultPendingComponent is fine. If you want slightly clearer intent and avoid creating the element when undefined, Dynamic also works:
- fallback={ - router.options.defaultPendingComponent ? ( - <router.options.defaultPendingComponent /> - ) : null - } + fallback={ + router.options.defaultPendingComponent ? ( + <Solid.Dynamic component={router.options.defaultPendingComponent} /> + ) : null + }packages/solid-router/tests/RouterProvider.test.tsx (1)
1-37: Test clearly validates Wrap context propagation; tiny readability nit.Consider declaring ctx before defining the component to reduce cognitive load when reading the closure.
- const rootRoute = createRootRoute({ + const ctx = createContext<string>() + const rootRoute = createRootRoute({ component: () => { const contextValue = useContext(ctx) expect(contextValue, 'Context is not provided').not.toBeUndefined() return <div>{contextValue}</div> }, }) @@ - const ctx = createContext<string>() - render(() => ( <RouterProvider router={router} Wrap={(props) => { return <ctx.Provider value={'findMe'}>{props.children}</ctx.Provider> }} /> ))packages/solid-router/tests/Matches.test.tsx (3)
34-37: Remove stray console.log from tests.This can add noise to CI output.
- console.log('Matchse', matches())
129-161: Good coverage for InnerWrap → route components; rename local “screen” to avoid shadowing import.Shadowing the imported screen with the render result is confusing.
- const screen = render(() => ( + const app = render(() => ( @@ - const indexElem = await screen.findByText('context-for-children') + const indexElem = await app.findByText('context-for-children')
163-223: Good coverage for InnerWrap → defaultPendingComponent; avoid shadowing and consider fake timers.
- Rename local screen as above.
- Optional: use vi.useFakeTimers() and advanceTimersByTime(300) to reduce flakiness from real timers.
- const screen = render(() => ( + const app = render(() => ( @@ - const linkToHome = await screen.findByRole('link', { + const linkToHome = await app.findByRole('link', { @@ - const indexElem = await screen.findByText('context-for-default-pending') + const indexElem = await app.findByText('context-for-default-pending')If you want to switch to fake timers:
import { vi } from 'vitest' // inside test vi.useFakeTimers() ... fireEvent.click(linkToHome) vi.advanceTimersByTime(300) ... vi.useRealTimers()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/solid-router/src/Matches.tsx(2 hunks)packages/solid-router/src/RouterProvider.tsx(2 hunks)packages/solid-router/tests/Matches.test.tsx(2 hunks)packages/solid-router/tests/RouterProvider.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/solid-router/src/Matches.tsx (2)
packages/solid-router/src/Transitioner.tsx (1)
Transitioner(11-140)packages/solid-router/src/Match.tsx (1)
Match(22-155)
packages/solid-router/tests/RouterProvider.test.tsx (1)
packages/solid-router/src/RouterProvider.tsx (1)
RouterProvider(44-53)
packages/solid-router/tests/Matches.test.tsx (1)
packages/solid-router/src/RouterProvider.tsx (1)
RouterProvider(44-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (4)
packages/solid-router/src/RouterProvider.tsx (1)
3-3: LGTM: SafeFragment import is appropriate.packages/solid-router/src/Matches.tsx (1)
77-80: LGTM: safer Match rendering.The guard around matchId avoids non-null assertions and is clearer.
packages/solid-router/tests/Matches.test.tsx (2)
3-3: LGTM: imports for context usage.
8-9: createMemoryHistory re-export confirmed – The barrel atpackages/solid-router/src/index.tsexportscreateMemoryHistory(line 203); tests can rely on it.
This was referenced Oct 16, 2025
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Refactored a bit router options.Wrap and options.InnerWrap usage in RouterProvider and Match components.
Now they will be able to provide context for their children components.
Added few tests for scenarios that were failing before.
Fixes #3744
Summary by CodeRabbit
New Features
Refactor
Tests