Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the SolidJS Router implementation to be compatible with a newer version of @solidjs/router, replacing the deprecated Routes component with Router and updating the API usage throughout the application.
- Updated
@solidjs/routerfrom v0.5.0 to v0.14.0 and other dependencies - Replaced
Routescomponent withRouteracross all route definitions - Changed
elementprop tocomponentprop for route definitions
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| package.json | Updated dependencies including @solidjs/router, solid-js, vite, and Node.js version |
| src/index.tsx | Replaced Routes with Router and removed unused Routes import |
| src/components/App.tsx | Updated router implementation and changed element prop to component |
| src/admin/index.tsx | Replaced Routes with Router in admin routing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/index.tsx
Outdated
| </Routes> | ||
| <Route path="/admin/*" component={Admin} /> | ||
| {/* <Route path="/map" element={<Map />} /> */} | ||
| <Route path="/admin/*" component={Admin} /> |
There was a problem hiding this comment.
Duplicate route definition for '/admin/*'. The route on line 21 will never be reached as it's identical to line 19.
| <Route path="/admin/*" component={Admin} /> |
src/components/App.tsx
Outdated
| @@ -1,4 +1,4 @@ | |||
| import { Route, Routes, useLocation, useNavigate } from '@solidjs/router'; | |||
| import { Route, Router, useLocation, useNavigate } from '@solidjs/router'; | |||
There was a problem hiding this comment.
Importing Router is incorrect for nested routing. Since this component is already inside a Router (from index.tsx), you should use Routes instead of Router for nested route definitions.
src/components/App.tsx
Outdated
| <Router> | ||
| <Route path="/" component={() => <></>} /> |
There was a problem hiding this comment.
Using Router here creates a nested router instance which will break routing. This should be Routes to define nested routes within the existing router context.
| import { | ||
| Route, | ||
| Routes, | ||
| Router, |
There was a problem hiding this comment.
Importing Router is incorrect for nested routing. Since this is a component rendered within a route, you should import Routes instead of Router.
| Router, |
| return ( | ||
| <> | ||
| <Routes> | ||
| <Router> |
There was a problem hiding this comment.
Using Router here creates a nested router instance which will break routing. This should be Routes to define nested routes within the existing router context.
Uh oh!
There was an error while loading. Please reload this page.