Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMigrates the frontend from Next.js to Vite and TanStack Router: adds Vite tooling and config, creates a Vite HTML entry and main.tsx bootstrap, converts Pages routes into TanStack file routes, updates components to use a router-compat layer, switches client env reads to import.meta.env, and updates infra/config files. ChangesFrontend Migration to Vite and TanStack Router
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/caddy/Caddyfile`:
- Around line 38-46: The current handle_errors block rewrites every error
(rewrite * /index.html) causing non-404 errors to return the SPA HTML; change
the handle_errors behavior to only rewrite and serve /index.html for 404s by
adding an error matcher (e.g., `@spa_404` using expression {err.status_code} ==
404) and move the rewrite/file_server into a handle `@spa_404` { ... } block so
other errors fall through to Caddy’s default handling; update the existing
handle_errors block that contains rewrite * /index.html and file_server
accordingly and keep the SPA try_files {path} /index.html behavior unchanged.
In `@src/frontend/eslint.config.mjs`:
- Around line 16-17: Update the ESLint rules in src/frontend/eslint.config.mjs:
change the rule values for "react-hooks/exhaustive-deps" and "react-hooks/refs"
from "off" to "warn" so React Hooks correctness checks run as warnings instead
of being disabled; locate the rules in the exported config object and replace
their string values accordingly.
In `@src/frontend/package.json`:
- Around line 37-38: Update the TanStack router-related dependencies so their
versions align with `@tanstack/react-router`@1.170.8: change
"`@tanstack/react-router-devtools`" from 1.167.0 to 1.170.8 and change
"`@tanstack/router-plugin`" (if present) to 1.170.8 as well; ensure package.json
entries for "`@tanstack/react-router`", "`@tanstack/react-router-devtools`", and
"`@tanstack/router-plugin`" all use the same 1.170.8 version, then run npm/yarn
install and verify no peer dependency warnings remain.
In `@src/frontend/README.md`:
- Line 1: Add a top-level (level-1) heading as the very first line of
src/frontend/README.md to satisfy MD041: insert a line like "# Messages" (or
another appropriate H1) above the existing first line "This is the Messages
frontend, built with [Vite] and [TanStack Router]." so the file's first line is
a heading.
In `@src/frontend/src/routes/__root.tsx`:
- Line 50: The THEME constant initialization uses JSON.parse on
process.env.NEXT_PUBLIC_THEME_CONFIG which can throw and crash the app during
module init; change it to safely parse with a try/catch (or a small safeParse
helper) around JSON.parse(process.env.NEXT_PUBLIC_THEME_CONFIG) and fall back to
a default object/string (e.g., "white-label") if parsing fails or the env var is
missing, ensuring THEME is assigned a robust value; update the reference where
THEME is declared so it never throws during module load.
- Around line 79-80: Wrap the unconditional devtools render in a dev-only guard:
only render <ReactQueryDevtools ... /> and <TanStackRouterDevtools ... /> when
the app is running in development (e.g. check import.meta.env.DEV or
process.env.NODE_ENV !== 'production') inside
src/frontend/src/routes/__root.tsx; keep the existing props (initialIsOpen and
buttonPosition) and simply conditionally render those two components so they are
excluded from production bundles.
In `@src/frontend/src/routes/mailbox/`$mailboxId/autoreplies/index.tsx:
- Line 26: The page root div with className "admin-page" is missing the shared
skip-link target id used by sibling mailbox settings routes; locate the root
element (the div with className "admin-page") in autoreplies/index.tsx and add
the same skip-link target id string used elsewhere (the id used by the other
mailbox settings pages) so keyboard users can use consistent skip navigation
across mailbox routes.
In `@src/frontend/src/routes/mailbox/`$mailboxId/integrations/index.tsx:
- Around line 35-46: The page renders header/actions even when selectedMailbox
is not yet available; add an early guard that returns nothing (or the same
empty/redirect used by sibling mailbox settings) when selectedMailbox is falsy
to avoid transient UI. Specifically, in the component containing
selectedMailbox, add an early check like "if (!selectedMailbox) return null;"
(or the project-standard empty/redirect) before the JSX that currently renders
<div className="admin-page" ...> so the header, <CreateIntegrationAction />, and
<IntegrationsPageContent /> are only rendered when selectedMailbox is present.
In `@src/frontend/vite.config.ts`:
- Around line 52-55: The build currently disables sourcemaps (build.sourcemap:
false) which prevents Sentry symbolication; change the Vite config
(build.sourcemap) to generate production sourcemaps (e.g., "hidden" or true) and
ensure the Sentry initialization (Sentry.init) is wired with matching
release/dist identifiers so artifact association works; also add or document a
CI step to upload those sourcemaps (e.g., via sentry-cli upload-sourcemaps)
using the same release/dist values so stack traces are symbolicated in
production.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f07d48cc-d5ed-4115-8fd4-9dd8b588513b
⛔ Files ignored due to path filters (1)
src/frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (73)
.gitignoreMakefileREADME.mdenv.d/development/frontend.defaultssrc/frontend/.gitignoresrc/frontend/Dockerfilesrc/frontend/README.mdsrc/frontend/caddy/Caddyfilesrc/frontend/eslint.config.mjssrc/frontend/index.htmlsrc/frontend/instrumentation-client.tssrc/frontend/next.config.tssrc/frontend/package.jsonsrc/frontend/scripts/print-bundle-stats.mjssrc/frontend/src/features/controlled-modals/message-importer/step-form.tsxsrc/frontend/src/features/forms/components/message-form/index.tsxsrc/frontend/src/features/forms/components/search-input/index.tsxsrc/frontend/src/features/layouts/components/admin/admin-layout.tsxsrc/frontend/src/features/layouts/components/admin/modal-create-update-mailbox/index.tsxsrc/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-actions/index.tsxsrc/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-form-modal/index.tsxsrc/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-item/index.tsxsrc/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-list/index.tsxsrc/frontend/src/features/layouts/components/mailbox-panel/index.tsxsrc/frontend/src/features/layouts/components/main/authenticated-view.tsxsrc/frontend/src/features/layouts/components/main/header/authenticated.tsxsrc/frontend/src/features/layouts/components/main/index.tsxsrc/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsxsrc/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsxsrc/frontend/src/features/layouts/components/thread-panel/hooks/use-thread-panel-filters.tssrc/frontend/src/features/layouts/components/thread-panel/index.tsxsrc/frontend/src/features/layouts/components/thread-selection-placeholder.tsxsrc/frontend/src/features/layouts/components/thread-view/components/thread-summary/index.tsxsrc/frontend/src/features/message/use-delete-label.tsxsrc/frontend/src/features/message/use-split-thread.tsxsrc/frontend/src/features/providers/admin-maildomain.tsxsrc/frontend/src/features/providers/mailbox.tsxsrc/frontend/src/features/providers/modal-store/index.tsxsrc/frontend/src/features/providers/thread-selection.tsxsrc/frontend/src/features/ui/components/label-badge/index.tsxsrc/frontend/src/features/utils/errors/index.tssrc/frontend/src/hooks/use-safe-router-push.tssrc/frontend/src/main.tsxsrc/frontend/src/pages/_app.tsxsrc/frontend/src/pages/_document.tsxsrc/frontend/src/pages/domain/[maildomainId]/signatures.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/autoreplies/index.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/index.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/integrations/index.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/message-templates/index.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/new/index.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/signatures/index.tsxsrc/frontend/src/pages/mailbox/[mailboxId]/thread/[threadId].tsxsrc/frontend/src/routes/__root.tsxsrc/frontend/src/routes/domain/$maildomainId/dns.tsxsrc/frontend/src/routes/domain/$maildomainId/index.tsxsrc/frontend/src/routes/domain/$maildomainId/signatures.tsxsrc/frontend/src/routes/domain/index.tsxsrc/frontend/src/routes/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/autoreplies/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/integrations/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/message-templates/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/new/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/signatures/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/thread/$threadId.tsxsrc/frontend/src/styles/main.scsssrc/frontend/src/styles/views/_index.scsssrc/frontend/src/styles/views/domain/_index.scsssrc/frontend/src/styles/views/mailbox/_index.scsssrc/frontend/src/styles/views/mailbox/new/_index.scsssrc/frontend/tsconfig.jsonsrc/frontend/vite.config.ts
💤 Files with no reviewable changes (12)
- src/frontend/src/pages/mailbox/[mailboxId]/index.tsx
- src/frontend/src/pages/mailbox/[mailboxId]/autoreplies/index.tsx
- src/frontend/src/pages/domain/[maildomainId]/signatures.tsx
- src/frontend/src/pages/mailbox/[mailboxId]/signatures/index.tsx
- src/frontend/src/pages/mailbox/[mailboxId]/integrations/index.tsx
- src/frontend/src/pages/mailbox/[mailboxId]/message-templates/index.tsx
- src/frontend/src/pages/mailbox/[mailboxId]/thread/[threadId].tsx
- src/frontend/src/pages/_document.tsx
- src/frontend/src/pages/_app.tsx
- src/frontend/next.config.ts
- env.d/development/frontend.defaults
- src/frontend/src/pages/mailbox/[mailboxId]/new/index.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/frontend/vite.config.ts`:
- Around line 40-42: The envPrefix in vite.config.ts currently uses the
Next.js-specific string 'NEXT_PUBLIC_' which can be confusing post-migration;
change envPrefix to a framework-agnostic value (e.g., 'PUBLIC_' or 'VITE_') by
updating the envPrefix property in vite.config.ts and then rename all usages of
import.meta.env.NEXT_PUBLIC_* to the new prefix (e.g., import.meta.env.PUBLIC_*
or import.meta.env.VITE_*), and update any docs, tests, CI config, and .env
files that reference NEXT_PUBLIC_ so everything remains consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b3e262a-8565-4fb2-b718-ca9510b3c9a0
📒 Files selected for processing (18)
src/frontend/README.mdsrc/frontend/caddy/Caddyfilesrc/frontend/instrumentation-client.tssrc/frontend/src/features/api/utils.tssrc/frontend/src/features/controlled-modals/message-importer/use-bucket-upload.tsxsrc/frontend/src/features/i18n/conf.tssrc/frontend/src/features/layouts/components/mailbox-settings/modal-compose-integration/widget-integration-form.tsxsrc/frontend/src/features/providers/theme.tsxsrc/frontend/src/features/ui/components/feedback-button/index.tsxsrc/frontend/src/features/ui/components/feedback-widget/index.tsxsrc/frontend/src/features/ui/components/lagaufre/index.tsxsrc/frontend/src/features/utils/errors/index.tssrc/frontend/src/routes/__root.tsxsrc/frontend/src/routes/mailbox/$mailboxId/autoreplies/index.tsxsrc/frontend/src/routes/mailbox/$mailboxId/integrations/index.tsxsrc/frontend/src/vite-env.d.tssrc/frontend/vite.config.tssrc/frontend/vitest.config.ts
Comment on lines
+40
to
+42
| // App env vars are read via `import.meta.env.NEXT_PUBLIC_*`. envPrefix | ||
| // tells Vite which env vars to expose to client code at build time. | ||
| envPrefix: 'NEXT_PUBLIC_', |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Consider renaming the environment variable prefix post-migration.
envPrefix: 'NEXT_PUBLIC_' retains the Next.js naming convention after migrating to Vite. While functionally correct, this may confuse future developers. Consider renaming to a framework-agnostic prefix like PUBLIC_ or VITE_ in a follow-up refactor to avoid coupling the codebase to Next.js naming.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/frontend/vite.config.ts` around lines 40 - 42, The envPrefix in
vite.config.ts currently uses the Next.js-specific string 'NEXT_PUBLIC_' which
can be confusing post-migration; change envPrefix to a framework-agnostic value
(e.g., 'PUBLIC_' or 'VITE_') by updating the envPrefix property in
vite.config.ts and then rename all usages of import.meta.env.NEXT_PUBLIC_* to
the new prefix (e.g., import.meta.env.PUBLIC_* or import.meta.env.VITE_*), and
update any docs, tests, CI config, and .env files that reference NEXT_PUBLIC_ so
everything remains consistent.
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
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.
Too many CVEs on Next.js lately, this is just to evaluate how we'd migrate off it.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes / Chores
Refactor