Skip to content

Conversation

@devjv
Copy link
Member

@devjv devjv commented Sep 20, 2025

  • routing doesnt work :8

@devjv devjv requested review from Copilot and httnn September 20, 2025 12:03
@devjv devjv changed the title Audit fix update packages Sep 20, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/router from v0.5.0 to v0.14.0 and other dependencies
  • Replaced Routes component with Router across all route definitions
  • Changed element prop to component prop 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} />
Copy link

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate route definition for '/admin/*'. The route on line 21 will never be reached as it's identical to line 19.

Suggested change
<Route path="/admin/*" component={Admin} />

Copilot uses AI. Check for mistakes.
@@ -1,4 +1,4 @@
import { Route, Routes, useLocation, useNavigate } from '@solidjs/router';
import { Route, Router, useLocation, useNavigate } from '@solidjs/router';
Copy link

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines 175 to 176
<Router>
<Route path="/" component={() => <></>} />
Copy link

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
import {
Route,
Routes,
Router,
Copy link

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Importing Router is incorrect for nested routing. Since this is a component rendered within a route, you should import Routes instead of Router.

Suggested change
Router,

Copilot uses AI. Check for mistakes.
return (
<>
<Routes>
<Router>
Copy link

Copilot AI Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@devjv devjv merged commit 47c931e into master Sep 27, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants