-
Notifications
You must be signed in to change notification settings - Fork 3k
[WEB-5042] feat: sites vite migration #7965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…Router components
…e unnecessary optimizeDeps configuration in vite.config
Merge branch 'preview' of github.com:makeplane/plane into feat/admin-react-router-migration
- Removed unused dependencies from `apps/admin/package.json`. - Added and updated dependencies in `packages/propel`, `packages/services`, and `packages/ui` to ensure consistency and compatibility. - Cleaned up `pnpm-lock.yaml` to reflect the changes in dependencies.
…react-router-migration
- Introduced React Router for improved routing capabilities. - Refactored multiple components to align with the new routing structure. - Added new assets and configuration files for enhanced functionality. - Updated ESLint and TypeScript configurations for better type handling. - Removed deprecated files and streamlined the project structure. - Enhanced Dockerfile and Nginx configuration for optimized deployment.
WalkthroughLarge-scale type-annotation cleanup (FC/ReactNode → React.FC/React.ReactNode) across admin and space apps, plus a migration of the space app from Next.js to React Router: new server (Express/Vite), route shims, client/server loaders, Docker/Nginx changes, and progress-bar integration. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant Client as React Router App
participant Server as Express/Vite Server
participant API as Backend API
Browser->>Server: GET /... (dev -> Vite SSR / prod -> static assets)
alt Dev (Vite SSR)
Server->>Client: Hand off to Vite SSR renderer
Client->>API: fetch publish/settings
API-->>Client: settings
Client-->>Browser: HTML/redirect
else Prod (static)
Server->>Browser: serve client bundle
Browser->>Client: clientLoader runs
Client->>API: fetch publish/settings
API-->>Client: settings
Client->>Browser: redirect or render route
end
Browser->>Client: Navigation event
Client->>AppProgressBar: navigation starts
AppProgressBar->>BProgress: start/stop based on navigation state
sequenceDiagram
participant Component
participant Shim as navigation shim
participant AppProgressBar
participant BProgress
Component->>Shim: useRouter().push('/path')
Shim->>Shim: ensureTrailingSlash()
Shim->>AppProgressBar: notify navigation start
AppProgressBar->>BProgress: BProgress.start() (after delay)
BProgress-->>AppProgressBar: visual progress
Shim->>AppProgressBar: navigation idle
AppProgressBar->>BProgress: BProgress.done()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-10-21T17:22:05.204ZApplied to files:
📚 Learning: 2025-09-02T08:14:49.260ZApplied to files:
⏰ 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)
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 |
| } | ||
|
|
||
| export function notFound(): never { | ||
| throw new Error("Not found"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prateekshourya29 did you check that this works as expected? I would of expected you to throw a Response instead.
| }; | ||
|
|
||
| // TODO: Convert into SSR in order to generate metadata | ||
| export async function generateMetadata({ params }: Props) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prateekshourya29 should convert to meta function the args are exported via the react-router types that are generated. You can look at my other PR for details.
| const { anchor } = params; | ||
| const IssuesPage = observer(() => { | ||
| // params | ||
| const params = useParams<{ anchor: string }>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
params already come from the Route.ComponentProps and its type safe too so you don't need this
| "use client"; | ||
|
|
||
| import type { FC, FormEvent } from "react"; | ||
| import type { FormEvent } from "react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you're getting rid of FC might as well get rid of FormEvent too.
1727e32 to
5e336af
Compare
5e336af to
0eda662
Compare
Merge branch 'preview' of github.com:makeplane/plane into feat/admin-react-router-migration
Added a useEffect hook to redirect users to the "/general" route if they are logged in. This enhances user experience by preventing access to the home page for authenticated users.
Merge branch 'preview' of github.com:makeplane/plane into feat/admin-react-router-migration
- Introduced a new route for Gitea authentication in the admin panel. - Updated the AdminHeader component to include Gitea as an authentication option. - Removed the unused Gitea layout file. - Enhanced the Gitea authentication page with metadata for improved SEO.
…ane/plane into feat-sites-vite-migration
- Added AppProgressBar component to display a progress bar during navigation.
- Configured BProgress with custom settings for improved user experience.
- Updated dependencies in pnpm-lock and package.json to include @bprogress/core.
- Added global styles for the progress bar in globals.css.
…avigation handling - Updated IssuesPage component to use useRouter for navigation instead of redirect and notFound functions. - Removed deprecated redirect and notFound functions from next/navigation compatibility layer. - Adjusted dependencies in the IssuesPage effect hook to include router for proper functionality.
…ane/plane into feat-sites-vite-migration
…EORIGIN" This reverts commit bd092a5.
…ane/plane into feat-sites-vite-migration
- Replaced useParams, useSearchParams, and useRouter with react-router's ClientLoaderFunctionArgs for improved navigation handling. - Simplified redirect logic by validating parameters and handling errors directly within the loader function. - Enhanced code readability and maintainability by removing the useEffect hook for fetching project settings.
…react-router-migration
…ane/plane into feat-sites-vite-migration
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
Merge branch 'preview' of github.com:makeplane/plane into feat-sites-vite-migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/space/core/components/account/user-logged-in.tsx (1)
1-1: Replace next/image import with the compatibility shim.The file directly imports from
"next/image"instead of using the available compatibility shim atapps/space/app/compat/next/image.tsx. Update the import on line 4:- import Image from "next/image"; + import Image from "@/app/compat/next/image";Also remove the
"use client"directive on line 1, as it has no meaning in a Vite/React Router environment and should not be carried over from the Next.js codebase.apps/space/core/components/instance/instance-failure-view.tsx (1)
25-27: Fix grammatical error in user-facing message.The phrase "connectivity work items" is grammatically incorrect. It should be "connectivity issue" or "connection issue".
Apply this diff to fix the error:
<p className="font-medium text-base text-center"> We were unable to fetch the details of the instance. <br /> - Fret not, it might just be a connectivity work items. + Fret not, it might just be a connectivity issue. </p>apps/space/core/components/account/auth-forms/auth-root.tsx (1)
174-180: Fix inverted theme-to-logo mapping to be consistent with apps/web implementation.The code incorrectly maps
darktheme toGithubLightLogo(a black PNG) andlighttheme toGithubDarkLogo. This is inverted and inconsistent with the parallel implementation inapps/web/core/components/account/auth-forms/auth-root.tsx(line 126), which correctly usesresolvedTheme === "dark" ? GithubDarkLogo : GithubLightLogo.Change line 176 from:
src={resolvedTheme === "dark" ? GithubLightLogo : GithubDarkLogo}to:
src={resolvedTheme === "dark" ? GithubDarkLogo : GithubLightLogo}This pattern is also inverted in
apps/space/core/components/common/logo-spinner.tsx(line 11).
♻️ Duplicate comments (1)
apps/space/app/issues/[anchor]/layout.tsx (1)
12-51: Convert generateMetadata to React Router meta function.As noted in the past review comment and TODO, this
generateMetadatafunction uses the Next.js pattern and should be converted to React Router'smetaexport function. React Router provides generated types for route parameters via the Route type from./+types/layout.Example conversion:
import type { Route } from "./+types/layout"; export const meta: Route.MetaFunction = async ({ params }) => { const { anchor } = params; // ... fetch logic ... return [ { title: data?.name || DEFAULT_TITLE }, { name: "description", content: data?.description || DEFAULT_DESCRIPTION }, { property: "og:title", content: data?.name || DEFAULT_TITLE }, // ... other meta tags ]; };
🧹 Nitpick comments (9)
apps/admin/core/components/instance/setup-form.tsx (1)
56-57: Clarify component props signature.The component uses
React.FCwithout a type parameter but still destructuresprops. Either explicitly type it asReact.FC<{}>to indicate no props, or remove the props parameter entirely since it's unused.Apply this diff for explicit typing:
-export const InstanceSetupForm: React.FC = (props) => { - const {} = props; +export const InstanceSetupForm: React.FC<{}> = () => {apps/space/app/types/next-image.d.ts (1)
1-10: Consider using React.FC for consistency.This ambient module declaration correctly provides a Next.js compatibility shim. However, for consistency with the project-wide refactor in this PR (which changes
FCtoReact.FCacross all components), consider updating the declaration to useReact.FC.Apply this diff for consistency:
declare module "next/image" { - import type { FC, ImgHTMLAttributes } from "react"; + import type { ImgHTMLAttributes } from "react"; type NextImageProps = ImgHTMLAttributes<HTMLImageElement> & { src: string; }; - const Image: FC<NextImageProps>; + const Image: React.FC<NextImageProps>; export default Image; }apps/space/app/types/next-link.d.ts (1)
1-14: Align type annotations with the PR's React.FC pattern.The ambient declaration imports and uses
FCdirectly, while the broader PR consistently adoptsReact.FCacross components (e.g.,apps/space/core/components/account/auth-forms/auth-root.tsxline 36,apps/space/app/compat/next/image.tsxline 12). For consistency, consider usingReact.FChere as well.Apply this diff to align with the project-wide pattern:
declare module "next/link" { - import type { FC, ComponentProps } from "react"; + import type React from "react"; - type NextLinkProps = ComponentProps<"a"> & { + type NextLinkProps = React.ComponentProps<"a"> & { href: string; replace?: boolean; prefetch?: boolean; scroll?: boolean; shallow?: boolean; }; - const Link: FC<NextLinkProps>; + const Link: React.FC<NextLinkProps>; export default Link; }apps/admin/app/(all)/(dashboard)/header.tsx (2)
13-13: Consider explicit React.FC annotation for consistency.The type annotation was removed here, while other files in this PR explicitly adopt
React.FC(e.g.,apps/space/core/components/account/auth-forms/auth-root.tsxline 36). Explicit typing improves code clarity and aligns with the broader refactoring pattern described in the PR summary.Based on learnings
25-25: Consider explicit React.FC annotation for consistency.Similar to
HamburgerToggle, adding an explicitReact.FCtype annotation would align with the pattern adopted elsewhere in this PR.Based on learnings
apps/space/app/[workspaceSlug]/[projectId]/page.tsx (1)
28-32: Consider logging errors before redirecting.The empty catch block silently redirects to
/404without logging the error. This can make debugging production issues difficult.Apply this diff to add error logging:
try { response = await publishService.retrieveSettingsByProjectId(workspaceSlug, projectId); - } catch { + } catch (error) { + console.error("Failed to fetch project publish settings:", error); throw redirect("/404"); }apps/space/app/compat/next/link.tsx (1)
3-3: Consider removing the explicit React import (optional).The React import is only used for the
React.FCtype annotation. When@types/reactis installed, React types are globally available and can be referenced without importing. However, this is a minor stylistic choice and doesn't affect functionality.Based on learnings
apps/space/nginx/nginx.conf (1)
15-15: Consider adding burst parameter to rate limiting.The current rate limit of
10r/sis quite restrictive and doesn't allow for burst traffic. Legitimate users loading a page might trigger multiple simultaneous requests (assets, API calls) and hit the limit.Consider adding a burst parameter:
- limit_req_zone $binary_remote_addr zone=mylimit:10m rate=10r/s; + limit_req_zone $binary_remote_addr zone=mylimit:10m rate=10r/s;And in the server or location block, add:
limit_req zone=mylimit burst=20 nodelay;This allows up to 20 requests in a burst while maintaining the average 10r/s rate.
apps/space/app/root.tsx (1)
51-51: Consider making the og:url dynamic.The hardcoded
og:urlof "https://sites.plane.so/" won't match individual published issue boards. Consider making this dynamic based on the actual URL or removing it so browsers use the current URL by default.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (31)
apps/space/app/assets/404.svgis excluded by!**/*.svgapps/space/app/assets/auth/background-pattern-dark.svgis excluded by!**/*.svgapps/space/app/assets/auth/background-pattern.svgis excluded by!**/*.svgapps/space/app/assets/favicon/apple-touch-icon.pngis excluded by!**/*.pngapps/space/app/assets/favicon/favicon-16x16.pngis excluded by!**/*.pngapps/space/app/assets/favicon/favicon-32x32.pngis excluded by!**/*.pngapps/space/app/assets/favicon/favicon.icois excluded by!**/*.icoapps/space/app/assets/images/logo-spinner-dark.gifis excluded by!**/*.gifapps/space/app/assets/images/logo-spinner-light.gifis excluded by!**/*.gifapps/space/app/assets/instance/instance-failure-dark.svgis excluded by!**/*.svgapps/space/app/assets/instance/instance-failure.svgis excluded by!**/*.svgapps/space/app/assets/instance/intake-sent-dark.pngis excluded by!**/*.pngapps/space/app/assets/instance/intake-sent-light.pngis excluded by!**/*.pngapps/space/app/assets/instance/plane-takeoff.pngis excluded by!**/*.pngapps/space/app/assets/logos/gitea-logo.svgis excluded by!**/*.svgapps/space/app/assets/logos/github-black.pngis excluded by!**/*.pngapps/space/app/assets/logos/github-dark.svgis excluded by!**/*.svgapps/space/app/assets/logos/github-square.svgis excluded by!**/*.svgapps/space/app/assets/logos/github-white.svgis excluded by!**/*.svgapps/space/app/assets/logos/gitlab-logo.svgis excluded by!**/*.svgapps/space/app/assets/logos/google-logo.svgis excluded by!**/*.svgapps/space/app/assets/plane-logo.svgis excluded by!**/*.svgapps/space/app/assets/plane-logos/black-horizontal-with-blue-logo.pngis excluded by!**/*.pngapps/space/app/assets/plane-logos/blue-without-text-new.pngis excluded by!**/*.pngapps/space/app/assets/plane-logos/blue-without-text.pngis excluded by!**/*.pngapps/space/app/assets/plane-logos/white-horizontal-with-blue-logo.pngis excluded by!**/*.pngapps/space/app/assets/plane-logos/white-horizontal.svgis excluded by!**/*.svgapps/space/app/assets/project-not-published.svgis excluded by!**/*.svgapps/space/app/assets/something-went-wrong.svgis excluded by!**/*.svgapps/space/app/assets/user-logged-in.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (90)
.gitignore(1 hunks)apps/admin/app/(all)/(dashboard)/ai/form.tsx(2 hunks)apps/admin/app/(all)/(dashboard)/authentication/github/form.tsx(1 hunks)apps/admin/app/(all)/(dashboard)/authentication/gitlab/form.tsx(1 hunks)apps/admin/app/(all)/(dashboard)/authentication/google/form.tsx(2 hunks)apps/admin/app/(all)/(dashboard)/email/email-config-form.tsx(2 hunks)apps/admin/app/(all)/(dashboard)/email/test-email-modal.tsx(4 hunks)apps/admin/app/(all)/(dashboard)/general/form.tsx(1 hunks)apps/admin/app/(all)/(dashboard)/general/intercom.tsx(1 hunks)apps/admin/app/(all)/(dashboard)/header.tsx(2 hunks)apps/admin/app/(all)/(dashboard)/image/form.tsx(1 hunks)apps/admin/app/(all)/(dashboard)/sidebar-help-section.tsx(1 hunks)apps/admin/app/(all)/(dashboard)/sidebar.tsx(1 hunks)apps/admin/app/(all)/(home)/auth-banner.tsx(1 hunks)apps/admin/app/(all)/(home)/sign-in-form.tsx(1 hunks)apps/admin/app/(all)/instance.provider.tsx(1 hunks)apps/admin/app/(all)/store.provider.tsx(1 hunks)apps/admin/app/(all)/user.provider.tsx(1 hunks)apps/admin/core/components/authentication/authentication-method-card.tsx(1 hunks)apps/admin/core/components/authentication/gitlab-config.tsx(0 hunks)apps/admin/core/components/authentication/google-config.tsx(0 hunks)apps/admin/core/components/common/empty-state.tsx(1 hunks)apps/admin/core/components/instance/setup-form.tsx(1 hunks)apps/admin/core/utils/public-asset.ts(1 hunks)apps/space/.eslintrc.cjs(2 hunks)apps/space/.gitignore(1 hunks)apps/space/Dockerfile.space(1 hunks)apps/space/app/[workspaceSlug]/[projectId]/page.ts(0 hunks)apps/space/app/[workspaceSlug]/[projectId]/page.tsx(1 hunks)apps/space/app/compat/next/helper.ts(1 hunks)apps/space/app/compat/next/image.tsx(1 hunks)apps/space/app/compat/next/link.tsx(1 hunks)apps/space/app/compat/next/navigation.ts(1 hunks)apps/space/app/issues/[anchor]/client-layout.tsx(3 hunks)apps/space/app/issues/[anchor]/layout.tsx(3 hunks)apps/space/app/issues/[anchor]/page.tsx(2 hunks)apps/space/app/layout.tsx(0 hunks)apps/space/app/not-found.tsx(1 hunks)apps/space/app/providers.tsx(2 hunks)apps/space/app/root.tsx(1 hunks)apps/space/app/routes.ts(1 hunks)apps/space/app/types/next-image.d.ts(1 hunks)apps/space/app/types/next-link.d.ts(1 hunks)apps/space/app/types/next-navigation.d.ts(1 hunks)apps/space/app/types/react-router-virtual.d.ts(1 hunks)apps/space/app/views/[anchor]/layout.tsx(0 hunks)apps/space/app/views/[anchor]/page.tsx(0 hunks)apps/space/core/components/account/auth-forms/auth-banner.tsx(1 hunks)apps/space/core/components/account/auth-forms/auth-header.tsx(1 hunks)apps/space/core/components/account/auth-forms/auth-root.tsx(3 hunks)apps/space/core/components/account/auth-forms/email.tsx(2 hunks)apps/space/core/components/account/terms-and-conditions.tsx(1 hunks)apps/space/core/components/account/user-logged-in.tsx(1 hunks)apps/space/core/components/common/logo-spinner.tsx(1 hunks)apps/space/core/components/common/powered-by.tsx(1 hunks)apps/space/core/components/instance/instance-failure-view.tsx(1 hunks)apps/space/core/components/issues/filters/applied-filters/root.tsx(1 hunks)apps/space/core/components/issues/filters/root.tsx(1 hunks)apps/space/core/components/issues/issue-layouts/error.tsx(1 hunks)apps/space/core/components/issues/issue-layouts/kanban/headers/group-by-card.tsx(1 hunks)apps/space/core/components/issues/issue-layouts/kanban/headers/sub-group-by-card.tsx(1 hunks)apps/space/core/components/issues/issue-layouts/list/blocks-list.tsx(2 hunks)apps/space/core/components/issues/issue-layouts/root.tsx(1 hunks)apps/space/core/components/issues/issue-layouts/with-display-properties-HOC.tsx(1 hunks)apps/space/core/components/issues/navbar/controls.tsx(1 hunks)apps/space/core/components/issues/navbar/layout-selection.tsx(1 hunks)apps/space/core/components/issues/navbar/root.tsx(1 hunks)apps/space/core/components/issues/navbar/user-avatar.tsx(1 hunks)apps/space/core/components/issues/peek-overview/layout.tsx(1 hunks)apps/space/core/components/ui/not-found.tsx(1 hunks)apps/space/core/lib/b-progress/AppProgressBar.tsx(1 hunks)apps/space/core/lib/b-progress/index.tsx(1 hunks)apps/space/core/lib/instance-provider.tsx(1 hunks)apps/space/core/lib/store-provider.tsx(1 hunks)apps/space/core/lib/toast-provider.tsx(1 hunks)apps/space/helpers/authentication.helper.tsx(1 hunks)apps/space/helpers/common.helper.ts(0 hunks)apps/space/middleware.js(1 hunks)apps/space/next-env.d.ts(0 hunks)apps/space/next.config.js(0 hunks)apps/space/nginx/nginx.conf(1 hunks)apps/space/package.json(2 hunks)apps/space/react-router.config.ts(1 hunks)apps/space/server.mjs(1 hunks)apps/space/server/app.ts(1 hunks)apps/space/styles/globals.css(1 hunks)apps/space/tsconfig.json(1 hunks)apps/space/vite.config.ts(1 hunks)apps/web/app/(all)/layout.tsx(1 hunks)packages/ui/package.json(1 hunks)
💤 Files with no reviewable changes (9)
- apps/space/app/layout.tsx
- apps/space/app/[workspaceSlug]/[projectId]/page.ts
- apps/space/helpers/common.helper.ts
- apps/space/app/views/[anchor]/page.tsx
- apps/admin/core/components/authentication/gitlab-config.tsx
- apps/space/next.config.js
- apps/admin/core/components/authentication/google-config.tsx
- apps/space/app/views/[anchor]/layout.tsx
- apps/space/next-env.d.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/admin/app/(all)/store.provider.tsxapps/admin/app/(all)/(home)/auth-banner.tsxapps/admin/core/components/instance/setup-form.tsxapps/space/core/components/issues/filters/applied-filters/root.tsxapps/admin/app/(all)/(home)/sign-in-form.tsxapps/space/core/components/account/terms-and-conditions.tsxapps/space/core/components/issues/navbar/user-avatar.tsxapps/space/core/lib/toast-provider.tsxapps/space/tsconfig.jsonapps/space/core/components/issues/issue-layouts/root.tsxapps/space/app/types/next-image.d.tsapps/space/core/components/issues/navbar/root.tsxapps/space/core/components/issues/issue-layouts/list/blocks-list.tsxapps/space/core/components/issues/peek-overview/layout.tsxapps/admin/app/(all)/(dashboard)/sidebar-help-section.tsxapps/admin/app/(all)/(dashboard)/authentication/github/form.tsxapps/space/core/components/account/auth-forms/email.tsxapps/space/core/lib/store-provider.tsxapps/admin/app/(all)/(dashboard)/image/form.tsxapps/admin/app/(all)/(dashboard)/general/intercom.tsxapps/space/core/components/common/powered-by.tsxapps/space/core/components/issues/navbar/layout-selection.tsxapps/space/core/components/account/auth-forms/auth-banner.tsxapps/admin/app/(all)/(dashboard)/authentication/gitlab/form.tsxapps/admin/app/(all)/(dashboard)/email/test-email-modal.tsxapps/admin/app/(all)/(dashboard)/sidebar.tsxapps/space/app/types/next-link.d.tsapps/space/core/components/issues/navbar/controls.tsxapps/admin/app/(all)/(dashboard)/ai/form.tsxapps/space/core/components/issues/issue-layouts/kanban/headers/group-by-card.tsxapps/admin/app/(all)/(dashboard)/header.tsxapps/admin/core/components/authentication/authentication-method-card.tsxapps/space/core/components/issues/issue-layouts/kanban/headers/sub-group-by-card.tsxapps/space/app/routes.tsapps/admin/app/(all)/(dashboard)/authentication/google/form.tsxapps/admin/app/(all)/(dashboard)/email/email-config-form.tsxapps/space/core/lib/instance-provider.tsxapps/admin/app/(all)/instance.provider.tsxapps/space/helpers/authentication.helper.tsxapps/space/core/components/account/auth-forms/auth-root.tsxapps/admin/app/(all)/(dashboard)/general/form.tsxapps/web/app/(all)/layout.tsxapps/space/core/components/instance/instance-failure-view.tsxapps/space/core/components/account/auth-forms/auth-header.tsxapps/space/core/components/issues/issue-layouts/with-display-properties-HOC.tsxapps/space/core/components/issues/filters/root.tsxpackages/ui/package.jsonapps/space/package.json
📚 Learning: 2025-09-02T08:14:49.260Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7697
File: apps/web/app/(all)/[workspaceSlug]/(projects)/header.tsx:12-13
Timestamp: 2025-09-02T08:14:49.260Z
Learning: The star-us-link.tsx file in apps/web/app/(all)/[workspaceSlug]/(projects)/ already has "use client" directive at the top, making it a proper Client Component for hook usage.
Applied to files:
apps/admin/app/(all)/store.provider.tsxapps/admin/core/components/instance/setup-form.tsxapps/space/core/components/issues/filters/applied-filters/root.tsxapps/admin/app/(all)/(home)/sign-in-form.tsxapps/space/core/components/account/terms-and-conditions.tsxapps/space/core/components/issues/navbar/user-avatar.tsxapps/space/core/components/issues/issue-layouts/root.tsxapps/space/core/components/issues/navbar/root.tsxapps/space/core/components/issues/peek-overview/layout.tsxapps/admin/app/(all)/(dashboard)/sidebar-help-section.tsxapps/admin/app/(all)/(dashboard)/authentication/github/form.tsxapps/space/core/lib/store-provider.tsxapps/admin/app/(all)/(dashboard)/image/form.tsxapps/admin/app/(all)/(dashboard)/general/intercom.tsxapps/space/core/components/common/powered-by.tsxapps/space/core/components/issues/navbar/layout-selection.tsxapps/space/core/components/account/auth-forms/auth-banner.tsxapps/space/app/issues/[anchor]/page.tsxapps/admin/app/(all)/(dashboard)/sidebar.tsxapps/space/core/components/issues/navbar/controls.tsxapps/admin/app/(all)/(dashboard)/ai/form.tsxapps/space/core/components/issues/issue-layouts/kanban/headers/group-by-card.tsxapps/admin/app/(all)/(dashboard)/header.tsxapps/admin/core/components/authentication/authentication-method-card.tsxapps/space/app/[workspaceSlug]/[projectId]/page.tsxapps/admin/app/(all)/(dashboard)/authentication/google/form.tsxapps/space/app/issues/[anchor]/client-layout.tsxapps/admin/app/(all)/(dashboard)/email/email-config-form.tsxapps/space/app/compat/next/image.tsxapps/space/core/components/account/auth-forms/auth-root.tsxapps/admin/app/(all)/(dashboard)/general/form.tsxapps/space/app/compat/next/link.tsxapps/space/core/components/account/auth-forms/auth-header.tsxapps/space/core/components/issues/filters/root.tsxapps/space/app/issues/[anchor]/layout.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/admin/app/(all)/store.provider.tsxapps/admin/app/(all)/(home)/auth-banner.tsxapps/admin/core/components/instance/setup-form.tsxapps/admin/app/(all)/(home)/sign-in-form.tsxapps/space/app/types/next-navigation.d.tsapps/space/tsconfig.jsonapps/space/react-router.config.tsapps/space/core/components/issues/navbar/root.tsxapps/space/app/compat/next/navigation.tsapps/space/core/components/issues/peek-overview/layout.tsxapps/space/core/lib/store-provider.tsxapps/admin/app/(all)/(dashboard)/image/form.tsxapps/space/core/components/common/powered-by.tsxapps/space/core/components/issues/navbar/layout-selection.tsxapps/space/app/root.tsxapps/space/app/issues/[anchor]/page.tsxapps/admin/app/(all)/(dashboard)/email/test-email-modal.tsxapps/admin/app/(all)/(dashboard)/sidebar.tsxapps/space/core/components/issues/navbar/controls.tsxapps/admin/app/(all)/(dashboard)/ai/form.tsxapps/admin/app/(all)/(dashboard)/header.tsxapps/admin/core/components/authentication/authentication-method-card.tsxapps/space/app/[workspaceSlug]/[projectId]/page.tsxapps/space/app/routes.tsapps/space/app/issues/[anchor]/client-layout.tsxapps/admin/app/(all)/(dashboard)/email/email-config-form.tsxapps/space/core/lib/instance-provider.tsxapps/web/app/(all)/layout.tsxapps/space/app/compat/next/link.tsxapps/space/core/components/issues/issue-layouts/with-display-properties-HOC.tsxpackages/ui/package.jsonapps/space/app/issues/[anchor]/layout.tsx
📚 Learning: 2025-10-09T22:12:26.424Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T22:12:26.424Z
Learning: When `types/react` is installed in a TypeScript project (which is standard for React + TypeScript codebases), React types (React.FC, React.ReactNode, React.ComponentProps, etc.) are globally available by design. These type annotations can and should be used without explicitly importing the React namespace. This is a TypeScript/DefinitelyTyped feature, not codebase-specific configuration.
Applied to files:
apps/admin/app/(all)/store.provider.tsxapps/admin/app/(all)/(home)/auth-banner.tsxapps/admin/core/components/instance/setup-form.tsxapps/space/core/components/issues/filters/applied-filters/root.tsxapps/space/core/components/account/terms-and-conditions.tsxapps/space/core/components/issues/navbar/user-avatar.tsxapps/space/core/lib/toast-provider.tsxapps/space/core/components/issues/issue-layouts/root.tsxapps/space/core/components/issues/navbar/root.tsxapps/space/core/components/issues/issue-layouts/list/blocks-list.tsxapps/space/core/components/issues/peek-overview/layout.tsxapps/admin/app/(all)/(dashboard)/sidebar-help-section.tsxapps/admin/app/(all)/(dashboard)/authentication/github/form.tsxapps/space/core/components/account/auth-forms/email.tsxapps/space/core/lib/store-provider.tsxapps/admin/app/(all)/(dashboard)/image/form.tsxapps/admin/app/(all)/(dashboard)/general/intercom.tsxapps/space/core/components/common/powered-by.tsxapps/space/core/components/issues/navbar/layout-selection.tsxapps/space/core/components/account/auth-forms/auth-banner.tsxapps/admin/app/(all)/(dashboard)/authentication/gitlab/form.tsxapps/admin/app/(all)/(dashboard)/email/test-email-modal.tsxapps/admin/app/(all)/(dashboard)/sidebar.tsxapps/space/app/types/next-link.d.tsapps/space/core/components/issues/navbar/controls.tsxapps/admin/app/(all)/(dashboard)/ai/form.tsxapps/space/core/components/issues/issue-layouts/kanban/headers/group-by-card.tsxapps/admin/core/components/authentication/authentication-method-card.tsxapps/space/core/components/issues/issue-layouts/kanban/headers/sub-group-by-card.tsxapps/admin/app/(all)/(dashboard)/authentication/google/form.tsxapps/admin/app/(all)/(dashboard)/email/email-config-form.tsxapps/space/core/lib/instance-provider.tsxapps/admin/app/(all)/instance.provider.tsxapps/space/helpers/authentication.helper.tsxapps/space/core/components/account/auth-forms/auth-root.tsxapps/admin/app/(all)/(dashboard)/general/form.tsxapps/space/core/components/instance/instance-failure-view.tsxapps/space/core/components/account/auth-forms/auth-header.tsxapps/space/core/components/issues/issue-layouts/with-display-properties-HOC.tsxapps/space/core/components/issues/filters/root.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/admin/app/(all)/(home)/auth-banner.tsxapps/space/core/components/account/terms-and-conditions.tsxapps/space/core/components/issues/navbar/user-avatar.tsxapps/space/core/components/issues/issue-layouts/list/blocks-list.tsxapps/admin/app/(all)/(dashboard)/email/test-email-modal.tsxapps/space/core/components/issues/issue-layouts/error.tsxapps/web/app/(all)/layout.tsxapps/space/core/components/issues/issue-layouts/with-display-properties-HOC.tsx
📚 Learning: 2025-10-09T20:43:07.762Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/core/components/new-user-popup.tsx:4-6
Timestamp: 2025-10-09T20:43:07.762Z
Learning: The `next-themes` library is React-compatible and can be used outside of Next.js applications. It's not Next.js-specific despite its name.
Applied to files:
apps/space/core/lib/toast-provider.tsxapps/space/app/compat/next/image.tsx
📚 Learning: 2025-10-06T01:44:38.472Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/collapsible/collapsible.stories.tsx:4-4
Timestamp: 2025-10-06T01:44:38.472Z
Learning: In Storybook v9, imports use bare paths instead of scoped packages. For example, `import { useArgs } from "storybook/preview-api"` is correct (not `storybook/preview-api`). This applies to other Storybook modules as well - the scoped storybook/* packages were consolidated into bare "storybook/*" imports.
Applied to files:
apps/space/core/components/issues/issue-layouts/error.tsx
📚 Learning: 2025-07-14T11:22:43.964Z
Learnt from: gakshita
Repo: makeplane/plane PR: 7393
File: apps/admin/app/(all)/(dashboard)/email/email-config-form.tsx:104-104
Timestamp: 2025-07-14T11:22:43.964Z
Learning: In the Plane project's SMTP configuration implementation, the email configuration form (email-config-form.tsx) hardcodes ENABLE_SMTP to "1" in form submission because the form is only rendered when SMTP is enabled. The enable/disable functionality is managed at the page level (page.tsx) with a toggle, and the form only handles configuration details when SMTP is already enabled.
Applied to files:
apps/admin/app/(all)/(dashboard)/email/email-config-form.tsx
📚 Learning: 2025-07-08T13:41:19.928Z
Learnt from: prateekshourya29
Repo: makeplane/plane PR: 7363
File: apps/web/helpers/react-hook-form.helper.ts:9-19
Timestamp: 2025-07-08T13:41:19.928Z
Learning: In the apps/web/helpers/react-hook-form.helper.ts file, the getNestedError function's FieldError detection logic using "message" property check is sufficient for their use case, and more robust type checking is not required.
Applied to files:
apps/space/helpers/authentication.helper.tsx
📚 Learning: 2025-06-04T16:22:44.344Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7164
File: packages/ui/.storybook/main.ts:24-47
Timestamp: 2025-06-04T16:22:44.344Z
Learning: In packages/ui/.storybook/main.ts, the webpackFinal function intentionally overrides the CSS loader strategy by finding and replacing existing CSS rules. This is a temporary workaround for a known upstream issue in Storybook's CSS handling that has been communicated to the Storybook maintainers. The current implementation should not be changed until the upstream issue is resolved.
Applied to files:
apps/web/app/(all)/layout.tsx
📚 Learning: 2025-08-29T08:45:15.953Z
Learnt from: sriramveeraghanta
Repo: makeplane/plane PR: 7672
File: pnpm-workspace.yaml:8-9
Timestamp: 2025-08-29T08:45:15.953Z
Learning: The makeplane/plane repository uses pnpm v10.12.1, which supports onlyBuiltDependencies configuration in pnpm-workspace.yaml files.
Applied to files:
packages/ui/package.jsonapps/space/package.json
🧬 Code graph analysis (13)
apps/space/app/types/next-navigation.d.ts (1)
apps/space/app/compat/next/navigation.ts (4)
useRouter(7-24)usePathname(26-29)useSearchParams(31-34)useParams(36-38)
apps/space/app/compat/next/navigation.ts (1)
apps/space/app/compat/next/helper.ts (1)
ensureTrailingSlash(6-33)
apps/space/core/lib/b-progress/AppProgressBar.tsx (1)
apps/admin/core/lib/b-progress/AppProgressBar.tsx (1)
AppProgressBar(66-140)
apps/space/app/root.tsx (1)
apps/space/app/providers.tsx (1)
AppProviders(11-24)
apps/space/app/issues/[anchor]/page.tsx (1)
apps/space/app/compat/next/navigation.ts (1)
useParams(36-38)
apps/space/server/app.ts (1)
apps/space/server.mjs (1)
app(29-29)
apps/space/app/[workspaceSlug]/[projectId]/page.tsx (1)
apps/space/core/components/common/logo-spinner.tsx (1)
LogoSpinner(8-18)
apps/space/app/providers.tsx (2)
apps/space/core/lib/store-provider.tsx (1)
StoreProvider(26-35)apps/space/core/lib/b-progress/AppProgressBar.tsx (1)
AppProgressBar(66-140)
apps/space/app/issues/[anchor]/client-layout.tsx (1)
apps/space/core/components/common/powered-by.tsx (1)
PoweredBy(11-30)
apps/space/vite.config.ts (1)
packages/utils/src/string.ts (1)
joinUrlPath(388-427)
apps/space/core/components/account/auth-forms/auth-root.tsx (1)
apps/web/core/components/account/auth-forms/auth-root.tsx (1)
AuthRoot(36-178)
apps/space/server.mjs (1)
apps/space/server/app.ts (1)
app(20-20)
apps/space/app/compat/next/link.tsx (2)
apps/space/app/compat/next/helper.ts (1)
ensureTrailingSlash(6-33)apps/admin/app/compat/next/link.tsx (1)
prefetch(15-22)
⏰ 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: Analyze (javascript)
| build/ | ||
| .react-router/ | ||
| AGENTS.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate ignore entries.
Lines 106-108 duplicate the entries at lines 110-112 (build/, .react-router/, AGENTS.md). Remove one set to avoid redundancy.
Apply this diff to remove the duplicate entries:
-build/
-.react-router/
-AGENTS.md
-
build/
.react-router/
AGENTS.mdCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In .gitignore around lines 106 to 108, the entries "build/", ".react-router/",
and "AGENTS.md" are duplicated later at lines 110-112; remove one of the
duplicate blocks so each ignore pattern appears only once — delete the redundant
lines 106-108 (or the later duplicate) to avoid redundancy and keep the file
tidy.
| @@ -0,0 +1 @@ | |||
| export {}; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty module file should be removed or documented.
This file exports nothing and contains no functionality. If it's a placeholder for future public asset utilities, add a TODO comment explaining the intended purpose. Otherwise, remove the file to avoid clutter.
🤖 Prompt for AI Agents
In apps/admin/core/utils/public-asset.ts at line 1, the module currently exports
nothing and is an empty placeholder; either delete the file if unused to avoid
clutter or, if intended for future use, add a brief TODO comment explaining its
purpose and expected exports (e.g., "TODO: utilities for public asset handling —
add functions for X, Y") so reviewers know it's intentional.
| const urlParams = new URLSearchParams(); | ||
| if (board) urlParams.append("board", String(board)); | ||
| if (peekId) urlParams.append("peekId", String(peekId)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Remove unnecessary String() coercion.
searchParams.get() already returns string | null, so the String() coercion is redundant and converts null to "null" instead of preserving the nullish value for proper conditionals.
Apply this diff:
let redirectUrl = `/issues/${response?.anchor}`;
const urlParams = new URLSearchParams();
- if (board) urlParams.append("board", String(board));
- if (peekId) urlParams.append("peekId", String(peekId));
+ if (board) urlParams.append("board", board);
+ if (peekId) urlParams.append("peekId", peekId);
if (urlParams.toString()) redirectUrl += `?${urlParams.toString()}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const urlParams = new URLSearchParams(); | |
| if (board) urlParams.append("board", String(board)); | |
| if (peekId) urlParams.append("peekId", String(peekId)); | |
| let redirectUrl = `/issues/${response?.anchor}`; | |
| const urlParams = new URLSearchParams(); | |
| if (board) urlParams.append("board", board); | |
| if (peekId) urlParams.append("peekId", peekId); | |
| if (urlParams.toString()) redirectUrl += `?${urlParams.toString()}`; |
🤖 Prompt for AI Agents
In apps/space/app/[workspaceSlug]/[projectId]/page.tsx around lines 36 to 38,
remove the unnecessary String() coercions when appending URL params: because
searchParams.get() returns string | null and the code already guards with if
(board) and if (peekId), append the raw value (board and peekId) instead of
String(board)/String(peekId) so null isn’t converted to "null" and the presence
checks remain correct.
| // return <IssuesClientLayout params={{ anchor }}>{children}</IssuesClientLayout>; | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Layout returns null, breaking the UI.
The layout function returns null instead of rendering its children, which means the entire issues section won't display. The commented-out code on line 57 shows the correct implementation. This appears to be incomplete migration work.
Additionally:
- The
"use server"directive (line 1) is Next.js-specific and not valid for React Router - The
generateMetadatafunction is a Next.js pattern that should be converted to React Router'smetaexport function - A past review comment on line 13 confirms this needs conversion to a
metafunction
Please uncomment line 57 and complete the React Router migration:
- // return <IssuesClientLayout params={{ anchor }}>{children}</IssuesClientLayout>;
- return null;
+ return <IssuesClientLayout params={{ anchor }}>{children}</IssuesClientLayout>;And address the TODO on line 12 by converting generateMetadata to a React Router meta function as mentioned in the past review comment.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // return <IssuesClientLayout params={{ anchor }}>{children}</IssuesClientLayout>; | |
| return null; | |
| return <IssuesClientLayout params={{ anchor }}>{children}</IssuesClientLayout>; |
🤖 Prompt for AI Agents
In apps/space/app/issues/[anchor]/layout.tsx around lines 1 and 12 and 57-58,
the layout currently returns null (line 58) and still contains Next.js-specific
patterns; uncomment and restore the client layout return so the component
renders: return <IssuesClientLayout params={{ anchor
}}>{children}</IssuesClientLayout>; remove the invalid "use server" directive at
the top (Next.js-only) and implement a React Router-compatible export named meta
(per the TODO on line 12) by converting the generateMetadata logic into a meta
function that returns the same metadata shape expected by React Router; ensure
the file exports meta and the layout renders children via the
IssuesClientLayout.
| import { next } from "@vercel/edge"; | ||
|
|
||
| export default function middleware() { | ||
| return next({ | ||
| headers: { | ||
| "Referrer-Policy": "origin-when-cross-origin", | ||
| "X-Frame-Options": "SAMEORIGIN", | ||
| "X-Content-Type-Options": "nosniff", | ||
| "X-DNS-Prefetch-Control": "on", | ||
| "Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload", | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify necessity of Vercel Edge middleware in Vite migration.
This middleware imports @vercel/edge, which is Vercel-specific. According to the PR description, the migration introduces Express server integration (server.mjs, server/app.ts). If deployment is now handled by Express/Node rather than Vercel Edge, this middleware file may not execute. The security headers should be configured in the Express middleware or Nginx configuration instead.
Verify whether this file is still needed, or if these headers should be configured elsewhere:
🏁 Script executed:
#!/bin/bash
# Check if Express server configures similar headers
fd -e ts -e js -e mjs server | xargs rg -n "Referrer-Policy|X-Frame-Options|Strict-Transport-Security" -A2 -B2
# Check for Nginx configuration
fd nginx.conf | xargs cat
# Check for Vercel-specific configuration
fd vercel.jsonLength of output: 1091
I need to gather more information about the deployment setup and whether this middleware file is still being used.
🏁 Script executed:
#!/bin/bash
# Check if middleware.js is imported or referenced anywhere
rg -n "middleware" --type js --type ts --type json -i | head -20
# Look for Express app configuration
fd -e ts -e js server | xargs cat | grep -i "header\|middleware" -A2 -B2 | head -30
# Check for build/deployment configuration files
fd -e json -e yaml -e yml | grep -E "(package\.json|tsconfig|vercel|build|deploy|nginx)" | head -20Length of output: 3413
🏁 Script executed:
#!/bin/bash
# Check for vercel.json file specifically
find . -name "vercel.json" -type f
# Check package.json for Vercel dependencies
fd package.json | xargs grep -l "@vercel" 2>/dev/null
# Check if middleware.js is imported anywhere
rg "from.*middleware|import.*middleware" --type js --type ts --type mjs
# Check if middleware.js is referenced in build config or tsconfig
fd -e json | xargs grep -l "middleware.js" 2>/dev/nullLength of output: 83
🏁 Script executed:
#!/bin/bash
# Check what @vercel dependencies are in apps/space/package.json
grep "@vercel" git/apps/space/package.json
# Verify helmet() usage and that it sets security headers
rg -n "helmet" --type ts --type js -A5 -B2Length of output: 1141
Remove unused middleware.js file — security headers are already configured via helmet() in Express.
The middleware.js file is orphaned code that won't execute in the new Express/Node deployment. It imports @vercel/edge (Vercel Edge Runtime specific), but is never imported or referenced anywhere in the codebase.
Security headers (Referrer-Policy, X-Frame-Options, X-Content-Type-Options, Strict-Transport-Security) are already configured via the helmet() middleware in the Express server (apps/live/src/server.ts:52). Delete apps/space/middleware.js as dead code.
🤖 Prompt for AI Agents
In apps/space/middleware.js lines 1-13, this file is orphaned Vercel Edge
runtime code importing @vercel/edge and providing headers that are already
applied by helmet() in apps/live/src/server.ts; delete apps/space/middleware.js
to remove dead/unreferenced code and update any project-level references
(build/config) if present to ensure no broken imports remain.
|
|
||
| default_type application/octet-stream; | ||
|
|
||
| set_real_ip_from 0.0.0.0/0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restrict trusted proxy sources for real IP header.
The configuration set_real_ip_from 0.0.0.0/0 trusts ANY source for the real IP header, which is a security risk. Clients can spoof the X-Forwarded-For header to bypass rate limiting or hide their true IP address.
Restrict this to only trusted proxy/load balancer IP ranges:
- set_real_ip_from 0.0.0.0/0;
+ # Replace with your actual proxy/load balancer CIDR ranges
+ set_real_ip_from 10.0.0.0/8;
+ set_real_ip_from 172.16.0.0/12;
+ set_real_ip_from 192.168.0.0/16;Or if deployed behind a cloud provider load balancer, use their specific IP ranges (e.g., CloudFlare, AWS ELB, etc.).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| set_real_ip_from 0.0.0.0/0; | |
| # Replace with your actual proxy/load balancer CIDR ranges | |
| set_real_ip_from 10.0.0.0/8; | |
| set_real_ip_from 172.16.0.0/12; | |
| set_real_ip_from 192.168.0.0/16; |
🤖 Prompt for AI Agents
In apps/space/nginx/nginx.conf around line 12, the directive set_real_ip_from
0.0.0.0/0 trusts any source for the real IP header and allows client IP
spoofing; replace the wildcard with only the CIDR ranges of your trusted
proxies/load balancers (or the cloud provider’s published ranges for
Cloudflare/AWS/Google/etc.), adding one set_real_ip_from line per trusted CIDR
and keeping real_ip_header/real_ip_recursive settings as needed so only upstream
proxies can set the client IP.
|
|
||
| set_real_ip_from 0.0.0.0/0; | ||
| real_ip_recursive on; | ||
| real_ip_header X-Forward-For; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in header name.
The header name should be X-Forwarded-For (with -ed), not X-Forward-For. This is the standard HTTP header name.
Apply this diff:
- real_ip_header X-Forward-For;
+ real_ip_header X-Forwarded-For;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| real_ip_header X-Forward-For; | |
| real_ip_header X-Forwarded-For; |
🤖 Prompt for AI Agents
In apps/space/nginx/nginx.conf around line 14, the real_ip_header value is
misspelled as "X-Forward-For"; change it to the correct standard header name
"X-Forwarded-For" (i.e., replace the current string with "X-Forwarded-For") so
nginx reads the proper client IP header.
| parsedBaseUrl = new URL(DEFAULT_BASE_URL); | ||
| } | ||
|
|
||
| const PORT = Number.parseInt(parsedBaseUrl.port, 10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle SPACE_BASE_URL values without an explicit port
If NEXT_PUBLIC_SPACE_BASE_URL is something like https://space.example.com, parsedBaseUrl.port is an empty string and Number.parseInt("", 10) becomes NaN, so app.listen(NaN) throws and the server never boots. Please fall back to process.env.PORT (and finally the default) when no port is present.
-const PORT = Number.parseInt(parsedBaseUrl.port, 10);
+const coercePort = (value) => {
+ if (!value) return undefined;
+ const parsed = Number.parseInt(value, 10);
+ return Number.isNaN(parsed) ? undefined : parsed;
+};
+
+const DEFAULT_PORT = Number.parseInt(new URL(DEFAULT_BASE_URL).port, 10);
+const PORT =
+ coercePort(parsedBaseUrl.port) ??
+ coercePort(process.env.PORT) ??
+ DEFAULT_PORT;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/space/server.mjs around line 26, parsedBaseUrl.port can be an empty
string which makes Number.parseInt("") return NaN; change the logic to use the
parsed port only when it is non-empty and parseInt yields a valid number,
otherwise fall back to process.env.PORT (and if that's undefined, use the
existing default port value). Ensure the final PORT variable is a valid integer
(coerce process.env.PORT to a number) before passing it to app.listen.
| target: NEXT_PUBLIC_API_BASE_URL, | ||
| changeOrigin: true, | ||
| secure: false, | ||
| pathRewrite: (path: string) => | ||
| NORMALIZED_API_BASE_PATH === "/api" ? path : path.replace(/^\/api/, NORMALIZED_API_BASE_PATH), | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t disable TLS verification in the API proxy
secure: false tells http-proxy-middleware to skip certificate validation for HTTPS targets. That’s fine for local dev but it downgrades TLS guarantees in production, making MITM trivial. Please either drop the option (defaults to true) or gate it to development only.
- secure: false,
+ secure: process.env.NODE_ENV === "production",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| target: NEXT_PUBLIC_API_BASE_URL, | |
| changeOrigin: true, | |
| secure: false, | |
| pathRewrite: (path: string) => | |
| NORMALIZED_API_BASE_PATH === "/api" ? path : path.replace(/^\/api/, NORMALIZED_API_BASE_PATH), | |
| }) | |
| target: NEXT_PUBLIC_API_BASE_URL, | |
| changeOrigin: true, | |
| secure: process.env.NODE_ENV === "production", | |
| pathRewrite: (path: string) => | |
| NORMALIZED_API_BASE_PATH === "/api" ? path : path.replace(/^\/api/, NORMALIZED_API_BASE_PATH), | |
| }) |
🤖 Prompt for AI Agents
In apps/space/server/app.ts around lines 30 to 35, the proxy is configured with
secure: false which disables TLS certificate verification; remove the insecure
default or make it environment-gated so certificate validation remains enabled
in non-development environments. Change the config to either omit the secure
option (defaults to true) or set secure to true in production and only false for
local/dev (use NODE_ENV or an explicit env var to toggle), and ensure any
dev-only disabling is clearly documented and not shipped to production.
Description
Type of Change
Summary by CodeRabbit
Infrastructure & Build
New Features
Code Quality