ECHO-487 Update react router to latest version#289
Conversation
WalkthroughFrontend migrates router dependency/imports from react-router-dom to react-router across package.json, Vite vendor chunk, core router setup, components, hooks, and route files; no runtime logic or public API signatures were changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
LGTM. Pre-merge checks (5 passed)✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (23)
echo/frontend/src/components/layout/AuthLayout.tsx (2)
16-21: Use replace on auth redirect to avoid back-nav to the login screenRedirecting after auth should generally replace history so Back doesn’t bounce users back to Auth. If your useI18nNavigate mirrors useNavigate’s signature, prefer replace: true.
- navigate(nextLink); + navigate(nextLink, { replace: true });[useNavigate options include replace.] (reactrouter.com)
16-21: Effect deps: include navigate and query (or document omission)To satisfy exhaustive-deps and ensure correctness if navigate/query change, include them in the deps (assuming useI18nNavigate is stable).
- }, [auth.isAuthenticated]); + }, [auth.isAuthenticated, navigate, query]);echo/frontend/src/components/report/ReportModalNavigationButton.tsx (1)
26-33: Add projectId to the handleClick deps to avoid a stale param.If the component remains mounted across a route change, the callback may capture the old projectId.
Apply:
- }, [projectReport, navigate, open]); + }, [projectId, projectReport, navigate, open]);echo/frontend/src/routes/project/ProjectsHome.tsx (1)
78-84: Use replace navigation to prevent history bloat while typing.Every debounced keystroke currently pushes a new entry. Prefer replace for query updates.
Apply:
- if (search) { - setSearchParams({ search }); - } else { - setSearchParams({}); - } + if (search) { + setSearchParams({ search }, { replace: true }); + } else { + setSearchParams({}, { replace: true }); + }setSearchParams accepts NavigateOptions (e.g., replace). (reactrouter.docschina.org)
echo/frontend/package.json (1)
59-60: Stray key name likely accidental (“notifications\n”).This property name includes a literal “\n”, which will create a separate dependency key and can confuse tooling. Consider normalizing to "notifications".
Apply:
- "notifications\n": "link:@mantine/notifications\n", + "notifications": "link:@mantine/notifications",echo/frontend/src/routes/participant/ParticipantReport.tsx (1)
23-38: Tiny polish: clear the print timeout on unmount.Not functionally blocking, but avoiding a fire-after-unmount is cheap.
Apply:
- useEffect(() => { + useEffect(() => { if (report) { mutate({ payload: { project_report_id: Number(report.id), type: "view", }, }); if (print) { - setTimeout(() => { + const id = setTimeout(() => { window.print(); }, 1000); + return () => clearTimeout(id); } } - }, [report, print]); + }, [report, print, mutate]);echo/frontend/src/routes/project/library/ProjectLibraryView.tsx (1)
21-26: Nit: add param typing for stronger TS safety.Explicitly type route params to avoid accidental undefined propagation.
- const { projectId, viewId } = useParams(); + const { projectId, viewId } = useParams<{ projectId: string; viewId: string }>();echo/frontend/src/routes/project/report/ProjectReportRoute.tsx (1)
49-50: Nit: tighten types on params.Helps TS catch missing/renamed params early.
- const { projectId, language } = useParams(); + const { projectId, language } = useParams<{ projectId: string; language: string }>();echo/frontend/src/components/conversation/ConversationLinks.tsx (1)
12-23: Type params and undefined guard for safer routing.If projectId is ever missing, you’ll generate broken hrefs. Type the params and early-return.
Apply:
- const { projectId } = useParams(); + const { projectId } = useParams<{ projectId?: string }>(); + if (!projectId) return null;echo/frontend/src/components/participant/MicrophoneTest.tsx (3)
272-276: Fix stale state on confirm (uses outdated selectedDeviceId).setState is async; onContinue(selectedDeviceId) can send the old value. Use displayDeviceId and close the modal.
- const handleConfirmMicChange = () => { - // Apply the pending device change - setSelectedDeviceId(displayDeviceId); - onContinue(selectedDeviceId); - }; + const handleConfirmMicChange = () => { + // Apply the pending device change + setSelectedDeviceId(displayDeviceId); + setShowSecondModal(false); + onContinue(displayDeviceId); + };
54-56: Route check is brittle; avoid substring includes.pathname.includes("start") can false-positive (e.g., "/restart"). Prefer a route match or exact segment test.
import { matchPath, useLocation } from "react-router"; // … const isStartPage = !!matchPath({ path: "/:language?/participant/start/*" }, pathname);
57-107: Avoid state updates after unmount in async init.Add an abort guard to prevent setState calls if the component unmounts mid-permission flow.
- useEffect(() => { - const initializeDevices = async () => { + useEffect(() => { + let alive = true; + const initializeDevices = async () => { setIsLoadingDevices(true); try { const stream = await navigator.mediaDevices.getUserMedia({ audio: true }); - // Now enumerate devices - this will include labels - if (navigator.mediaDevices?.enumerateDevices) { + if (!alive) return; + if (navigator.mediaDevices?.enumerateDevices) { const all = await navigator.mediaDevices.enumerateDevices(); const inputs = all.filter((d) => d.kind === "audioinput"); setDevices(inputs); // … } stream.getTracks().forEach((track) => track.stop()); - // Mark that we have mic access - setMicAccessGranted(true); - setMicAccessDenied(false); + if (!alive) return; + setMicAccessGranted(true); + setMicAccessDenied(false); } catch (error) { console.error("Failed to get microphone permission or enumerate devices:", error); - setMicAccessDenied(true); - setErrorMessage("Microphone permission is required. Please allow access to proceed."); + if (alive) { + setMicAccessDenied(true); + setErrorMessage("Microphone permission is required. Please allow access to proceed."); + } } finally { - setIsLoadingDevices(false); + if (alive) setIsLoadingDevices(false); } }; initializeDevices(); - }, []); + return () => { alive = false; }; + }, []);echo/frontend/src/components/conversation/MoveConversationButton.tsx (1)
37-38: Type the route params; drop the cast downstream.Avoid projectId as string casts by typing useParams.
- const { projectId } = useParams(); + const { projectId } = useParams<{ projectId?: string }>();And remove
as stringwhere used in filters.echo/frontend/src/components/conversation/AutoSelectConversations.tsx (2)
28-29: Type the params for safety.Prevents accidental undefined in downstream hooks.
- const { chatId, projectId } = useParams(); + const { chatId, projectId } = useParams<{ chatId?: string; projectId?: string }>();
61-61: Drop stray console.log in prod.Leftover logging clutters the console and can leak data.
- console.log(hasProcessedConversations, conversations); + // console.debug("[AutoSelectConversations]", { hasProcessedConversations, conversations });echo/frontend/src/routes/project/library/ProjectLibraryAspect.tsx (2)
22-27: Type the params to avoid undefined casts.- const { projectId, viewId, aspectId } = useParams(); + const { projectId, viewId, aspectId } = + useParams<{ projectId?: string; viewId?: string; aspectId?: string }>();
83-87: Simplify truthiness check for insights.The double length check is redundant.
- {aspect?.aspect_segment?.length && aspect?.aspect_segment?.length > 0 && ( + {(aspect?.aspect_segment?.length ?? 0) > 0 && ( <Title order={2}> <Trans>Insights</Trans> </Title> )}echo/frontend/src/hooks/useLanguage.ts (1)
26-29: Type params; avoid falling back to i18n.locale unexpectedly.Make the expected param explicit.
- const params = useParams(); + const params = useParams<{ language?: string }>(); const language = params.language ?? i18n.locale ?? defaultLanguage;echo/frontend/src/components/layout/Header.tsx (1)
81-98: Type the language param for clarity.Tighten types and keep default fallback logic.
- const { language } = useParams(); + const { language } = useParams<{ language?: string }>();echo/frontend/src/components/error/ErrorPage.tsx (1)
27-29: Prefer SPA navigation over full reload.Use Link or useNavigate("/") to avoid a hard refresh and preserve SPA performance, unless you intentionally want a full reset after errors.
- <Button onClick={() => (window.location.href = "/")}> + <Button component="a" href="/" /* or use useNavigate("/") */>echo/frontend/src/hooks/useI18nNavigate.ts (1)
12-31: Bug: to.toString() breaks object/relative navigations; language prefixing can corrupt paths.
- To may be an object; toString() -> "[object Object]".
- Relative strings like "settings" end up as "/ensettings".
- Use createPath and only prefix absolute paths starting with "/".
Apply this refactor:
@@ -import { NavigateOptions, To, useNavigate, useParams } from "react-router"; +import { NavigateOptions, To, useNavigate, useParams, createPath } from "react-router"; @@ - return (to: To, options?: NavigateOptions) => { - if (typeof to === "number") { - navigate(to, options); - return; - } - - const toString = to.toString(); - - // Check if 'to' starts with any supported language prefix - const hasLanguagePrefix = SUPPORTED_LANGUAGES.some((lang) => - toString.startsWith(`/${lang}`), - ); - - if (hasLanguagePrefix) { - navigate(to, options); - } else { - // Add the language prefix if it's not already present - const languagePrefix = finalLanguage ? `/${finalLanguage}` : ""; - navigate(`${languagePrefix}${to}`, options); - } - }; + return (to: To, options?: NavigateOptions) => { + if (typeof to === "number") return navigate(to, options); + + // Compute a string path only when absolute (leading "/"); otherwise leave as-is (relative/object) + const pathStr = + typeof to === "string" ? to : "pathname" in (to as any) ? createPath(to as any) : null; + + const isAbsolute = typeof pathStr === "string" && pathStr.startsWith("/"); + if (!isAbsolute) return navigate(to as any, options); + + const hasLanguagePrefix = SUPPORTED_LANGUAGES.some((lang) => + (pathStr as string).startsWith(`/${lang}`), + ); + if (hasLanguagePrefix) return navigate(to as any, options); + + const languagePrefix = finalLanguage ? `/${finalLanguage}` : ""; + const withPrefix = `${languagePrefix}${pathStr}`; + return typeof to === "string" + ? navigate(withPrefix, options) + : navigate({ ...(to as any), pathname: withPrefix }, options); + };Reference: createPath for safe To->string conversion. (reactrouter.com)
echo/frontend/vite.config.ts (1)
1-56: Confirm v7 migration; enforce Node >=20react-router ^7.8.2 is present in echo/frontend/package.json; no react-router-dom imports found in echo/frontend/src (61 react-router import matches); React is ^19.0.0. package.json has no engines.node — add "engines": { "node": ">=20" } to echo/frontend/package.json and pin Node 20+ in CI/Docker/.nvmrc/.node-version, then re-run CI. LGTM after that change.
echo/frontend/src/components/common/i18nLink.tsx (1)
12-20: Bug: early return inside forEach is a no-op; language-prefixed links get double-prefixed.Returning from the forEach callback doesn’t exit the component, so the guard never applies.
Apply this diff:
- SUPPORTED_LANGUAGES.map((lang) => `/${lang}`).forEach((lang) => { - if (to.toString().startsWith(lang)) { - return <Link to={to} {...props} />; - } - }); + const toStr = to.toString(); + if (SUPPORTED_LANGUAGES.some((lang) => toStr.startsWith(`/${lang}`))) { + return <Link to={to} {...props} />; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
echo/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (63)
echo/frontend/package.json(1 hunks)echo/frontend/src/App.tsx(1 hunks)echo/frontend/src/Router.tsx(1 hunks)echo/frontend/src/components/aspect/AspectCard.tsx(1 hunks)echo/frontend/src/components/aspect/hooks/useCopyAspect.tsx(1 hunks)echo/frontend/src/components/aspect/hooks/useCopyQuote.ts(1 hunks)echo/frontend/src/components/auth/hooks/index.ts(1 hunks)echo/frontend/src/components/chat/ChatAccordion.tsx(1 hunks)echo/frontend/src/components/chat/ChatHistoryMessage.tsx(1 hunks)echo/frontend/src/components/common/NavigationButton.tsx(1 hunks)echo/frontend/src/components/common/i18nLink.tsx(1 hunks)echo/frontend/src/components/conversation/AutoSelectConversations.tsx(1 hunks)echo/frontend/src/components/conversation/ConversationAccordion.tsx(1 hunks)echo/frontend/src/components/conversation/ConversationDangerZone.tsx(1 hunks)echo/frontend/src/components/conversation/ConversationLinks.tsx(1 hunks)echo/frontend/src/components/conversation/MoveConversationButton.tsx(1 hunks)echo/frontend/src/components/error/ErrorPage.tsx(1 hunks)echo/frontend/src/components/insight/Insight.tsx(1 hunks)echo/frontend/src/components/language/LanguagePicker.tsx(1 hunks)echo/frontend/src/components/layout/AuthLayout.tsx(1 hunks)echo/frontend/src/components/layout/BaseLayout.tsx(1 hunks)echo/frontend/src/components/layout/Header.tsx(1 hunks)echo/frontend/src/components/layout/LanguageLayout.tsx(1 hunks)echo/frontend/src/components/layout/ParticipantLayout.tsx(1 hunks)echo/frontend/src/components/layout/ProjectConversationLayout.tsx(1 hunks)echo/frontend/src/components/layout/ProjectLayout.tsx(1 hunks)echo/frontend/src/components/layout/ProjectLibraryLayout.tsx(1 hunks)echo/frontend/src/components/layout/ProjectOverviewLayout.tsx(1 hunks)echo/frontend/src/components/layout/TabsWithRouter.tsx(1 hunks)echo/frontend/src/components/participant/MicrophoneTest.tsx(1 hunks)echo/frontend/src/components/participant/ParticipantOnboardingCards.tsx(1 hunks)echo/frontend/src/components/participant/UserChunkMessage.tsx(1 hunks)echo/frontend/src/components/project/ProjectCard.tsx(1 hunks)echo/frontend/src/components/project/ProjectListItem.tsx(1 hunks)echo/frontend/src/components/project/ProjectSidebar.tsx(1 hunks)echo/frontend/src/components/quote/Quote.tsx(1 hunks)echo/frontend/src/components/report/ConversationStatusTable.tsx(1 hunks)echo/frontend/src/components/report/CreateReportForm.tsx(1 hunks)echo/frontend/src/components/report/ReportModalNavigationButton.tsx(1 hunks)echo/frontend/src/components/report/UpdateReportModalButton.tsx(1 hunks)echo/frontend/src/components/view/View.tsx(1 hunks)echo/frontend/src/components/view/hooks/useCopyView.tsx(1 hunks)echo/frontend/src/hooks/useI18nNavigate.ts(1 hunks)echo/frontend/src/hooks/useLanguage.ts(1 hunks)echo/frontend/src/routes/Debug.tsx(1 hunks)echo/frontend/src/routes/auth/Login.tsx(1 hunks)echo/frontend/src/routes/auth/PasswordReset.tsx(1 hunks)echo/frontend/src/routes/auth/VerifyEmail.tsx(1 hunks)echo/frontend/src/routes/participant/ParticipantConversation.tsx(1 hunks)echo/frontend/src/routes/participant/ParticipantPostConversation.tsx(1 hunks)echo/frontend/src/routes/participant/ParticipantReport.tsx(1 hunks)echo/frontend/src/routes/participant/ParticipantStart.tsx(1 hunks)echo/frontend/src/routes/project/ProjectRoutes.tsx(1 hunks)echo/frontend/src/routes/project/ProjectsHome.tsx(1 hunks)echo/frontend/src/routes/project/chat/ProjectChatRoute.tsx(1 hunks)echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx(1 hunks)echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx(1 hunks)echo/frontend/src/routes/project/library/ProjectLibrary.tsx(1 hunks)echo/frontend/src/routes/project/library/ProjectLibraryAspect.tsx(1 hunks)echo/frontend/src/routes/project/library/ProjectLibraryView.tsx(1 hunks)echo/frontend/src/routes/project/report/ProjectReportRoute.tsx(1 hunks)echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx(1 hunks)echo/frontend/vite.config.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-08T10:39:31.114Z
Learnt from: ussaama
PR: Dembrane/echo#259
File: echo/frontend/src/components/layout/ParticipantLayout.tsx:33-33
Timestamp: 2025-08-08T10:39:31.114Z
Learning: In echo/frontend/src/components/layout/ParticipantLayout.tsx, prefer using simple pathname.includes("start") and pathname.includes("finish") to control the settings button visibility. No need to switch to segment-based matching or add a useEffect to auto-close the modal for these routes, per ussaama’s preference in PR #259.
Applied to files:
echo/frontend/src/components/layout/ProjectOverviewLayout.tsxecho/frontend/src/components/report/ReportModalNavigationButton.tsxecho/frontend/src/routes/participant/ParticipantPostConversation.tsxecho/frontend/src/routes/participant/ParticipantReport.tsxecho/frontend/src/routes/participant/ParticipantStart.tsxecho/frontend/src/components/layout/ParticipantLayout.tsxecho/frontend/src/components/layout/AuthLayout.tsxecho/frontend/src/components/layout/ProjectConversationLayout.tsxecho/frontend/src/components/project/ProjectSidebar.tsxecho/frontend/src/components/participant/ParticipantOnboardingCards.tsxecho/frontend/src/components/layout/TabsWithRouter.tsxecho/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsxecho/frontend/src/components/layout/BaseLayout.tsxecho/frontend/src/routes/participant/ParticipantConversation.tsx
📚 Learning: 2025-08-06T13:38:30.769Z
Learnt from: ussaama
PR: Dembrane/echo#256
File: echo/frontend/src/components/participant/MicrophoneTest.tsx:54-54
Timestamp: 2025-08-06T13:38:30.769Z
Learning: In echo/frontend/src/components/participant/MicrophoneTest.tsx, the useDisclosure hook is used where only the `close` function is needed locally (called in handleConfirmMicChange and handleCancelMicChange), while `opened` and `open` are unused because the modal state is managed by a parent component. The `showSecondModal` state variable is used separately to control content switching within the modal between the main UI and confirmation step.
Applied to files:
echo/frontend/src/components/participant/MicrophoneTest.tsx
⏰ 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: ci-check-server
🔇 Additional comments (59)
echo/frontend/src/components/layout/AuthLayout.tsx (2)
4-4: Imports correctly migrated to react-router v7 — LGTMOutlet, useNavigate, and useSearchParams are exported from "react-router" in v7, so this swap is correct. (reactrouter.com)
4-4: Repo-wide sanity check: no lingering react-router-dom importsAutomated sandbox scan couldn't verify (ripgrep skipped files; package manifests missing). Run from repo root and paste the output:
rg -nP -C1 $'from\\s+[\"\\\']react-router-dom(?:\\/[^\"\\\']*)?[\"\\\']' --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true rg -nP -C1 "require\\(['\"]react-router-dom(?:\\/[^'\"]*)?['\"]\\)" --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true rg -n --hidden -uu "react-router-dom" --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || true rg -n '"react-router-dom"' package.json package-lock.json yarn.lock pnpm-lock.yaml || true git grep -n "react-router-dom" || trueLGTM.
echo/frontend/src/components/report/ReportModalNavigationButton.tsx (1)
9-9: React Router v7 import source is correct. LGTM.useParams/useLocation now come from "react-router" after the v7 package collapse; this aligns with the migration plan. (reactrouter.com)
echo/frontend/src/routes/project/ProjectsHome.tsx (1)
46-46: Good switch to react-router.useSearchParams is exported from "react-router" in v7; import looks right. (reactrouter.com)
echo/frontend/src/routes/participant/ParticipantPostConversation.tsx (1)
27-27: Import update to react-router is on point.useParams from "react-router" matches v7 guidance. LGTM. (reactrouter.com)
echo/frontend/src/routes/project/conversation/ProjectConversationTranscript.tsx (1)
36-36: Import source migration looks solid.useParams now from "react-router" per v7; no functional deltas here. (reactrouter.com)
echo/frontend/src/components/quote/Quote.tsx (1)
2-2: Clean import swap.useParams from "react-router" is the correct v7 target. LGTM. (reactrouter.com)
echo/frontend/package.json (1)
70-70: LGTM — react-router@7.8.2 bump looks appropriate; verification requiredBump addresses the advisory, but the ripgrep run returned "No files were searched" so I could not confirm removal of react-router-dom imports or that Node >=20 is pinned.
Action (run from repo root and confirm zero matches + check engines):
# find any react-router-dom usage rg -n --hidden -S -g '!**/node_modules/**' -C2 "react-router-dom" || true # confirm key imports are from react-router (not react-router-dom) rg -n --hidden -S -g '!**/node_modules/**' -C2 "import\s+\{(Link|Navigate|Outlet|RouterProvider|createBrowserRouter|useSearchParams|useParams|useLocation)\s*\}\s+from\s+['\"]react-router['\"]" || true # check engines in package.json jq -r '."engines" // "MISSING"' echo/frontend/package.json || cat echo/frontend/package.json | sed -n '1,120p'If those commands return no react-router-dom hits and engines show Node >=20 (or CI is pinned to Node >=20), approve the change.
echo/frontend/src/routes/participant/ParticipantReport.tsx (1)
1-1: Imports look right.useParams/useSearchParams from "react-router" matches the v7 package collapse. LGTM. (reactrouter.com)
echo/frontend/src/routes/auth/PasswordReset.tsx (1)
15-15: Import swap is correct.useSearchParams sourced from "react-router" is the v7-way. LGTM. (reactrouter.com)
echo/frontend/src/components/view/View.tsx (1)
16-16: React Router v7 import switch — LGTM.Correctly sourcing useParams from react-router. No behavior change.
echo/frontend/src/components/layout/ProjectOverviewLayout.tsx (1)
4-4: Import path migration — LGTM.useParams now from react-router aligns with the v7 consolidation.
echo/frontend/src/components/aspect/AspectCard.tsx (1)
5-5: Router import source update — LGTM.Matches the repo-wide migration.
echo/frontend/src/routes/project/ProjectRoutes.tsx (1)
19-19: useParams import moved to react-router — LGTM.No functional change; types remain compatible.
echo/frontend/src/components/conversation/ConversationAccordion.tsx (1)
49-49: useLocation/useParams from react-router — LGTM.Consistent with v7 package consolidation.
echo/frontend/src/components/layout/ProjectLayout.tsx (1)
2-2: LGTM — react-router v7 migration verified.
No react-router-dom imports remain; echo/frontend/package.json declares react-router ^7.8.2; common router APIs (Outlet, Link, useParams, RouterProvider, createBrowserRouter, Navigate, useLocation, useSearchParams) are imported from "react-router".echo/frontend/src/routes/project/library/ProjectLibraryView.tsx (1)
17-17: LGTM: migrated useParams import to react-router.This aligns with v7’s consolidated entrypoint.
echo/frontend/src/components/layout/ProjectLibraryLayout.tsx (1)
3-3: LGTM: Outlet from react-router is correct in v7.echo/frontend/src/components/chat/ChatHistoryMessage.tsx (1)
22-22: LGTM: useParams now sourced from react-router.v7 exports the hook from the core package.
echo/frontend/src/routes/auth/Login.tsx (2)
27-27: LGTM: useSearchParams import updated to react-router.Matches v7 API.
27-27: Action: Import DOM-only router APIs from 'react-router/dom' (react-router v7 detected)package.json shows react-router ^7.8.2 — v7 exposes DOM-specific exports under "react-router/dom". Replace DOM imports (RouterProvider, BrowserRouter, useSearchParams, Link, etc.) that currently come from "react-router" with "react-router/dom".
Example: echo/frontend/src/routes/auth/Login.tsx imports useSearchParams from "react-router" — change to "react-router/dom".
Repo scan in the sandbox failed (ripgrep didn't recognize tsx); run a repo-wide search for imports from "react-router" and fix all DOM-specific matches. LGTM.echo/frontend/src/routes/project/report/ProjectReportRoute.tsx (1)
1-1: LGTM: useParams import switched to react-router.echo/frontend/src/components/view/hooks/useCopyView.tsx (1)
4-4: LGTM: hook now pulls useParams from react-router.echo/frontend/src/components/project/ProjectCard.tsx (1)
8-8: LGTM: router imports updated to react-router.echo/frontend/src/routes/project/conversation/ProjectConversationOverview.tsx (1)
12-12: LGTM: useParams import migrated to react-router.echo/frontend/src/components/layout/LanguageLayout.tsx (1)
1-1: RR v7 import swap LGTM.Outlet from react-router is correct in v7. react-router-dom is deprecated and re-exports from react-router; migrating imports is the right call. (api.reactrouter.com)
echo/frontend/src/components/conversation/ConversationLinks.tsx (1)
2-2: RR v7 import swap LGTM.useParams from react-router is correct post-v7. (reactrouter.com)
echo/frontend/src/components/participant/MicrophoneTest.tsx (1)
19-19: RR v7 import swap LGTM.useLocation from react-router is correct. (reactrouter.com)
echo/frontend/src/components/conversation/MoveConversationButton.tsx (1)
25-25: RR v7 import swap LGTM.useParams from react-router is correct post-v7. (reactrouter.com)
echo/frontend/src/components/conversation/AutoSelectConversations.tsx (1)
15-15: RR v7 import swap LGTM.useParams from react-router is correct post-v7. (reactrouter.com)
echo/frontend/src/routes/project/library/ProjectLibraryAspect.tsx (1)
11-11: RR v7 import swap LGTM.useParams from react-router is correct post-v7. (reactrouter.com)
echo/frontend/src/hooks/useLanguage.ts (1)
4-4: RR v7 import swap LGTM.useParams from react-router is correct post-v7. (reactrouter.com)
echo/frontend/src/components/layout/Header.tsx (1)
27-27: RR v7 import swap LGTM.useParams from react-router is correct post-v7. (reactrouter.com)
echo/frontend/src/components/auth/hooks/index.ts (1)
15-15: LGTM — react-router v7 import paths are correctSweep found no react-router-dom imports; echo/frontend/package.json depends on react-router ^7.8.2 and source files import from "react-router".
echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx (1)
1-1: LGTM: useSearchParams from react-router matches v7 APIImporting from react-router (not react-router-dom) is the v7 path. (reactrouter.com)
echo/frontend/src/components/project/ProjectSidebar.tsx (1)
15-15: LGTM — react-router v7 imports are correct; confirm Node runtimeImporting useLocation/useParams from "react-router" is the v7 approach (react-router-dom is a re-export shim). (api.reactrouter.com)
package.json shows react ^19.0.0 and react-router ^7.8.2, so the React requirement (>=18) is met; v7 requires node@20+, react@18+, react-dom@18+ — package.json has no "engines" field, ensure CI/runtime uses Node >=20. (reactrouter.com)echo/frontend/src/components/error/ErrorPage.tsx (1)
2-2: Import swap to react-router is correct for v7. LGTM.v7 consolidates web APIs under react-router; replacing react-router-dom imports here is the right move. (reactrouter.com)
echo/frontend/src/components/layout/BaseLayout.tsx (1)
3-3: Outlet import updated to react-router. Good upgrade hygiene.Matches v7 guidance; no behavior change. (reactrouter.com)
echo/frontend/src/hooks/useI18nNavigate.ts (1)
2-2: Hook imports migrated to react-router. Solid.The types/hooks are exported from react-router in v7. (reactrouter.com)
echo/frontend/src/components/report/UpdateReportModalButton.tsx (1)
13-13: useParams import moved to react-router. All good.Consistent with v7 exports.
echo/frontend/src/components/layout/ParticipantLayout.tsx (1)
1-1: Outlet/useLocation from react-router—migration looks clean.No functional drift; aligns with prior preference to use pathname.includes. LGTM.
echo/frontend/src/components/language/LanguagePicker.tsx (1)
6-6: useLocation import switch is correct.Change is scoped and safe; behavior unchanged.
echo/frontend/vite.config.ts (1)
29-31: Vendor chunk updated to react-router.Matches dependency switch; should improve tree-shaking by avoiding the dom re-export layer. LGTM.
echo/frontend/src/components/report/ConversationStatusTable.tsx (1)
4-4: Link import from react-router is v7-appropriate.Mantine Anchor-as-Link usage remains valid. LGTM.
echo/frontend/src/components/layout/TabsWithRouter.tsx (2)
3-3: React Router v7 import path switch — correct. LGTM.
Outlet,useLocation, anduseParamsare first-class exports fromreact-routerin v7; moving offreact-router-domaligns with the official upgrade guide. (reactrouter.com)
3-3: LGTM — react-router-dom removed; routing APIs import from react-routerNo react-router-dom imports or dependency found; package.json shows react-router ^7.8.2. Confirmed provider APIs import from react-router in:
- echo/frontend/src/Router.tsx (createBrowserRouter)
- echo/frontend/src/App.tsx (RouterProvider)
- echo/frontend/src/components/common/i18nLink.tsx (Link, LinkProps, useParams)
echo/frontend/src/routes/participant/ParticipantConversation.tsx (1)
32-32: Hook imports from react-router — good to go.
useParamsanduseSearchParamsare exported fromreact-routerin v7; behavior is unchanged. (reactrouter.com)echo/frontend/src/components/aspect/hooks/useCopyAspect.tsx (1)
4-4: LGTM on the import swap.
useParamsfromreact-routeris the correct v7 source. (reactrouter.com)echo/frontend/src/routes/project/chat/ProjectChatRoute.tsx (1)
32-32: Import path update — approved.
useParamsis valid fromreact-routerin v7. Stay mindful of v7 minimums (React 18, Node 20) in CI/runtime. (reactrouter.com)Verify tool/runtime targets meet v7 minimums (React 18/Node 20) in your deployment pipeline configs.
echo/frontend/src/routes/auth/VerifyEmail.tsx (1)
7-7: useSearchParams from react-router — LGTM.API parity maintained in v7. (reactrouter.com)
echo/frontend/src/routes/participant/ParticipantStart.tsx (1)
6-6: v7 import path is correct.Generic usage of
useParams<{ projectId: string }>()remains supported. (reactrouter.com)echo/frontend/src/routes/Debug.tsx (1)
17-17: Switch to react-router imports — approved.No behavior change expected. (reactrouter.com)
echo/frontend/src/components/participant/ParticipantOnboardingCards.tsx (1)
4-4: Importing useSearchParams from react-router — LGTM.Matches v7 guidance. (reactrouter.com)
echo/frontend/src/components/common/i18nLink.tsx (1)
4-4: Imports updated to v7 correctly. LGTM.Link, LinkProps, and useParams are valid exports from react-router v7. (api.reactrouter.com)
echo/frontend/src/components/insight/Insight.tsx (1)
2-2: Import swap to react-router looks good.useParams/Link are first-class in v7. (reactrouter.com)
echo/frontend/src/components/chat/ChatAccordion.tsx (1)
20-20: useParams import from react-router is correct for v7. LGTM. (reactrouter.com)echo/frontend/src/components/conversation/ConversationDangerZone.tsx (1)
4-4: useParams import source updated correctly.Matches v7 guidance. LGTM. (reactrouter.com)
echo/frontend/src/App.tsx (1)
1-39: Sanity check react-router migration — ensure no lingering react-router-dom deps/imports (echo/frontend/src/App.tsx:1-39).Automated sandbox check failed to find package.json; run and paste outputs from these commands so I can re-verify:
find . -type f -iname package.json -not -path "/node_modules/" -not -path "/.git/" -print
for P in $(find . -type f -iname package.json -not -path "/node_modules/" -not -path "/.git/"); do echo "---- $P ----"; if command -v jq >/dev/null 2>&1; then jq '.dependencies,.devDependencies,.peerDependencies' "$P" | rg -n "react-router" || echo "no react-router"; else grep -nE '"react-router(|-dom)"' "$P" || echo "no react-router"; fi; done
rg -n -S --hidden -uu --glob '!/node_modules/' "from\s+['"]react-router(-dom)?['"]|require(['"]react-router(-dom)?['"])" || echo "no imports found"
LGTM.
echo/frontend/src/Router.tsx (1)
1-1: createBrowserRouter/Navigate import from react-router is correct for v7. LGTM.This aligns with the v7 upgrade guidance. (reactrouter.com)
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
echo/frontend/src/routes/project/library/ProjectLibrary.tsx (1)
43-55: v7 import + typed params — LGTM. Consider hard-guarding on missing projectId.Right now hooks run with "" when the param is absent. Cheap win: bail early to avoid accidental network calls.
export const ProjectLibraryRoute = () => { - const { projectId } = useParams<{ projectId: string }>(); + const { projectId } = useParams<{ projectId: string }>(); + if (!projectId) { + // No route param — avoid firing queries with sentinel values + return null; + }
♻️ Duplicate comments (1)
echo/frontend/src/App.tsx (1)
7-7: RouterProvider from react-router/dom — LGTM.Correct DOM entrypoint; ensures the DOM-specific wiring (flushSync, history).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
echo/frontend/src/App.tsx(1 hunks)echo/frontend/src/components/aspect/hooks/useCopyQuote.ts(2 hunks)echo/frontend/src/components/common/NavigationButton.tsx(0 hunks)echo/frontend/src/components/layout/ProjectConversationLayout.tsx(1 hunks)echo/frontend/src/components/participant/UserChunkMessage.tsx(2 hunks)echo/frontend/src/components/project/ProjectListItem.tsx(0 hunks)echo/frontend/src/components/report/CreateReportForm.tsx(2 hunks)echo/frontend/src/routes/project/library/ProjectLibrary.tsx(2 hunks)
💤 Files with no reviewable changes (2)
- echo/frontend/src/components/common/NavigationButton.tsx
- echo/frontend/src/components/project/ProjectListItem.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T10:39:31.114Z
Learnt from: ussaama
PR: Dembrane/echo#259
File: echo/frontend/src/components/layout/ParticipantLayout.tsx:33-33
Timestamp: 2025-08-08T10:39:31.114Z
Learning: In echo/frontend/src/components/layout/ParticipantLayout.tsx, prefer using simple pathname.includes("start") and pathname.includes("finish") to control the settings button visibility. No need to switch to segment-based matching or add a useEffect to auto-close the modal for these routes, per ussaama’s preference in PR #259.
Applied to files:
echo/frontend/src/App.tsxecho/frontend/src/components/layout/ProjectConversationLayout.tsxecho/frontend/src/components/participant/UserChunkMessage.tsx
⏰ 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: Cursor Bugbot
🔇 Additional comments (3)
echo/frontend/src/App.tsx (1)
5-15: LGTM — no lingering react-router-dom; RouterProvider imports correct. rg found no react-router-dom imports or incorrect RouterProvider root imports; echo/frontend/package.json lists react-router@^7.8.2 and react-router-dom is absent; lockfiles contain no react-router-dom.echo/frontend/src/components/aspect/hooks/useCopyQuote.ts (2)
23-23: Guard route params (language, projectId) before useuseParams() returns Partial, so language/projectId can be undefined — add a small runtime guard or invariant before using them.
File: echo/frontend/src/components/aspect/hooks/useCopyQuote.ts (around line 23)
const { language, projectId } = useParams<{ language: string; projectId: string }>();Suggested quick fix:
const { language, projectId } = useParams<{ language: string; projectId: string }>(); if (!language || !projectId) { throw new Error('Missing route params: language/projectId'); }To confirm route guarantees, re-run a safer grep for route params:
#!/bin/bash rg -nP -C3 '(:language|:projectId)' || true
4-4: LGTM: react-router v7 import is correct — verify no react-router-dom remainsImporting from 'react-router' is canonical for v7. Repo scan here failed ("No files were searched"); run this from the repo root and paste any hits:
rg -n --hidden -S --no-ignore 'react-router-dom' || true
- bump react router version to latest - change all instances of 'react-router-dom' to 'react-router' as per documentation <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - Chores - Updated routing dependency to react-router v7.8.2. - Refactor - Migrated routing imports and hooks to react-router with no expected behavior changes. - Build - Bundling updated to include react-router instead of react-router-dom. Impact - Navigation, links, and UI behavior remain unchanged. - Build now resolves routing from react-router (no user-facing changes anticipated). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
Summary by CodeRabbit
Impact