chore: migrate react/express architecture to nextjs#3
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughConsolidates a pnpm monorepo into a Next.js App Router application: removes legacy backend/web/types packages and Biome tooling; adds Next.js pages, API routes, auth and DB integration, Zod types, ESLint/Prettier, CI, and new client/server libraries and components. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant B as Browser (Next client)
participant S as Next.js Server
participant Auth as better-auth
participant DB as MySQL
U->>B: Open app
B->>S: SSR layout request
S->>Auth: auth.api.getSession(headers)
alt session exists
Auth->>DB: validate session
Auth-->>S: session (user)
S-->>B: Render authenticated layout (sidebar)
B->>S: GET /api/v1/pages/tree
S->>DB: query containers (user scoped)
DB-->>S: branches + children
S-->>B: { data: branches }
B->>B: render PagesTree UI
else no session
Auth-->>S: null
S-->>B: Render unauthenticated layout
end
B->>S: POST /api/v1/pages (create)
S->>DB: get workspace, validate parent, insert container
DB-->>S: created page
S-->>B: { data: createdPage }
B->>B: navigate to /pages/{id}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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: 17
🧹 Nitpick comments (42)
packages/backend/src/modules/containers/api/get-pages-tree.ts (1)
21-21: TODO: Add workspaceId to query.Line 21 contains a TODO comment about adding workspaceId to the query. Verify whether this is a known limitation or an incomplete feature. If multi-workspace support is required, prioritize this change to avoid fetching cross-workspace containers.
Would you like me to open a new issue to track this TODO, or would you prefer to address it as part of this PR?
.prettierrc (1)
1-13: Consider consolidating duplicate Prettier configurations.The same Prettier configuration exists in both the root
.prettierrcandpackages/nextjs/.prettierrc. Unless package-specific overrides are needed, maintain a single configuration at the root level to avoid maintenance overhead and potential inconsistencies.packages/nextjs/src/app/manifest.json (1)
1-41: Add missing PWA manifest fields.The manifest is missing several standard fields that improve the progressive web app experience:
short_name: A shorter version of the name for limited spacestart_url: The URL that loads when launching the appdisplay: How the app should be displayed (e.g., "standalone", "fullscreen")theme_color: Browser UI colorbackground_color: Background color for splash screenExample additions:
{ "name": "Thoth", "short_name": "Thoth", "start_url": "/", "display": "standalone", "theme_color": "#000000", "background_color": "#ffffff", "icons": [.github/workflows/ci.yaml (2)
40-41: Consider adding continue-on-error to audit step.The
pnpm auditstep will fail the entire CI pipeline if any vulnerabilities are found, even low-severity ones. Consider addingcontinue-on-error: trueto report vulnerabilities without blocking the build, or set a severity threshold.Apply this diff:
- name: Run security audit - run: pnpm audit + run: pnpm audit + continue-on-error: trueOr use a severity threshold:
- name: Run security audit - run: pnpm audit + run: pnpm audit --audit-level=high
46-47: Add format check to CI pipeline.Since Prettier configuration was added to the project, consider adding a format check step to ensure code formatting standards are enforced in CI.
Add after the lint step:
- name: Run format check run: pnpm -r format:checkNote: Ensure
format:checkscripts are defined in package.json files (e.g.,"format:check": "prettier --check .").packages/nextjs/README.md (1)
1-36: Customize the README for the Thoth project.This is the default Next.js README from
create-next-app. Consider updating it to:
- Describe the Thoth project's purpose and features
- Document OIDC authentication setup requirements
- Include environment variable configuration
- Explain the MySQL database setup
- Document the workspace/pages hierarchy feature
- Remove references to npm/yarn/bun since the project uses pnpm
packages/nextjs/tsconfig.json (2)
8-16:incrementalis a no‑op withnoEmit; either remove it or switch strategy.Keeping both can confuse devs and CI; consider removing
incrementalhere.- "noEmit": true, + "noEmit": true, @@ - "incremental": true,Alternatively, if you want incremental type‑checks, run
tsc --incremental --noEmitand commit the.tsbuildinfo(or ignore it intentionally).
2-24: Harden cross‑platform imports.Enable casing checks to avoid case‑sensitive path bugs in CI/containers.
"compilerOptions": { "target": "ES2022", @@ "strict": true, + "forceConsistentCasingInFileNames": true,packages/nextjs/next.config.ts (1)
3-5: Good call externalizingmysql2. Consider container‑friendly output.If you plan to deploy in a container or serverless with smaller images, add standalone output.
const nextConfig: NextConfig = { - serverExternalPackages: ["mysql2"], + serverExternalPackages: ["mysql2"], + output: "standalone" };packages/nextjs/src/lib/database/helpers.ts (1)
3-9: Add explicit return types for clarity and safer inference.Make return type
Queryexplicit to avoid accidentalanyand keep chaining types intact.-export const addUserIdToQuery = (query: Query, userId: string) => { +export const addUserIdToQuery = (query: Query, userId: string): Query => { return query.eq('userId', userId); }; -export const addWorkspaceIdToQuery = (query: Query, workspaceId: string) => { +export const addWorkspaceIdToQuery = (query: Query, workspaceId: string): Query => { return query.eq('workspaceId', workspaceId); };Optionally guard against empty IDs if upstream can pass
""/whitespace; otherwise you risk broad scans.packages/nextjs/package.json (1)
1-14: Pin Node runtime viaenginesto match CI (Node 22).Prevents local mismatches and odd Next/Turbopack failures.
"private": true, "type": "module", + "engines": { + "node": ">=22.0.0" + },packages/nextjs/eslint.config.mjs (1)
18-20: Inconsistent quote style in rules configuration.Line 20 uses single quotes while lines 18-19 use double quotes. Maintain consistency throughout the configuration.
rules: { "unicorn/no-useless-undefined": "off", "unicorn/no-null": "off", - 'quotes': 'off' + "quotes": "off" },packages/nextjs/src/components/layout.tsx (3)
18-22: Removerouterfrom useEffect dependency array.The
routerobject returned byuseRouter()is stable and doesn't need to be included in the dependency array. Including it may cause unnecessary effect re-runs and ESLint warnings.useEffect(() => { if (!loading && !user) { router.push('/login'); } - }, [loading, user, router]); + }, [loading, user]);
48-58: Use Next.js Link component instead of anchor tag for client-side navigation.Using an anchor tag for the logout link will cause a full page reload. For better UX and to maintain client-side navigation benefits, use Next.js
Linkcomponent or handle the click with client-side routing.+import Link from 'next/link'; + // ... in the component: - <a - href="/logout" + <Link + href="/logout" style={{ textDecoration: 'none', color: 'inherit', fontWeight: 500, fontSize: '0.95rem', }} > Logout - </a> + </Link>
28-30: Returnnullinstead ofundefinedfor consistency.React components conventionally return
nullwhen rendering nothing, notundefined. While both work,nullis the idiomatic choice.if (!user) { - return undefined; // Will redirect to login + return null; // Will redirect to login }packages/nextjs/src/lib/api/client.ts (2)
13-16: Simplify params handling ingetTreemethod.The ternary operator to conditionally add
parentIdto params is unnecessary. Axios automatically omits undefined values from query parameters, so you can simplify this.- getTree: (parentId?: string) => - apiClient.get('/pages/tree', { - params: parentId ? { parentId } : {}, - }), + getTree: (parentId?: string) => + apiClient.get('/pages/tree', { + params: { parentId }, + }),
10-22: Consider adding response type annotations for type safety.The API methods currently lack return type annotations, which means consumers won't benefit from TypeScript's type checking on response data. Consider defining response types and annotating the methods accordingly.
Example pattern:
import type { AxiosPromise } from 'axios'; interface PageTreeResponse { // ... define structure } interface PageDetailsResponse { // ... define structure } export const api = { pages: { getTree: (parentId?: string): AxiosPromise<PageTreeResponse> => apiClient.get('/pages/tree', { params: { parentId }, }), // ... other methods }, };packages/nextjs/src/app/pages/create/page.tsx (1)
47-53: Remove redundant authentication check.This authentication guard is redundant since the parent
Layoutcomponent (packages/nextjs/src/components/layout.tsx) already handles redirecting unauthenticated users to/login. This code will never be reached by an unauthenticated user.- if (!user) { - return ( - <Container size="md" py="xl"> - <Text>Please log in to create pages.</Text> - </Container> - ); - } - return (Alternatively, add an early guard at the function level for defensive programming:
const handleSubmit = async (values: typeof form.values) => { - if (!user) return; + // User is guaranteed by parent Layout componentpackages/nextjs/src/app/(auth)/layout.tsx (1)
4-6: Consider clarifying the layout hierarchy.This minimal layout in the
(auth)route group differs significantly from the more complexLayoutcomponent atpackages/nextjs/src/components/layout.tsx(which includes AppShell, auth checks, and navigation). While the architecture may be intentional, the naming similarity could cause confusion about which layout handles authentication and UI composition.packages/nextjs/src/lib/auth/session.ts (1)
10-12: Consider adding error context for debugging.The generic error message lacks context that would aid debugging in production (e.g., request path, timestamp). Consider logging the error or enriching the error message before throwing.
packages/nextjs/src/app/page.tsx (1)
1-13: Consider making this a server component.The page contains only static content with no interactivity, state, or browser APIs. Removing the
'use client'directive would make this a server component, reducing the client bundle size and improving initial page load performance.packages/nextjs/src/components/atoms/tree-toggle.tsx (2)
12-12: Consider using Mantine's sizing for consistent alignment.The hardcoded 20px spacer may not perfectly match the ActionIcon's actual rendered size. Consider using Mantine's sizing tokens or measuring the actual button size to ensure proper alignment.
- return <div style={{ width: 20, height: 20 }} />; // Spacer for alignment + return <div style={{ width: 'var(--mantine-spacing-xl)', height: 'var(--mantine-spacing-xl)' }} />; // Spacer for alignmentAlternatively, determine the exact rendered size of
ActionIconwithsize="sm"and use that value explicitly.
16-16: Consider using Mantine's sx prop for styling.Inline styles work but using Mantine's
sxprop would be more consistent with Mantine's API and provide better type safety and theme integration.- <ActionIcon variant="subtle" size="sm" onClick={onToggle} style={{ flexShrink: 0 }}> + <ActionIcon variant="subtle" size="sm" onClick={onToggle} sx={{ flexShrink: 0 }}>packages/nextjs/src/types/schemas/entities/workspace.ts (1)
4-10: Tighten validation and disallow unknown keys.
- Trim and bound name length to prevent empty/whitespace-only or unbounded input.
- Prefer ISO datetime validation for timestamps (in utilities).
- Make the object strict to reject unexpected fields.
export const workspaceSchema = z .object({ - name: z.string().min(1), + name: z.string().trim().min(1).max(128), }) .extend(withTrackUpdatesSchema.shape) .extend(withUserIdSchema.shape) - .extend(withIdSchema.shape); + .extend(withIdSchema.shape) + .strict();If acceptable, also strengthen utilities:
- export const withTrackUpdatesSchema = z.object({ - lastUpdated: z.string(), - createdAt: z.string(), - }); + export const withTrackUpdatesSchema = z.object({ + lastUpdated: z.string().datetime(), + createdAt: z.string().datetime(), + });packages/nextjs/src/lib/auth/client.ts (1)
4-6: Make baseURL robust and avoid empty fallback in production.Use the current origin in browsers and fail clearly if unavailable, rather than defaulting to ''.
-export const authClient = createAuthClient({ - baseURL: process.env.NODE_ENV === 'development' ? 'http://localhost:3000' : process.env.NEXT_PUBLIC_API_URL || '', -}); +const baseURL = + typeof window !== 'undefined' + ? window.location.origin + : process.env.NEXT_PUBLIC_API_URL || (process.env.NODE_ENV === 'development' ? 'http://localhost:3000' : ''); + +export const authClient = createAuthClient({ baseURL }); + +if (process.env.NODE_ENV === 'production' && !process.env.NEXT_PUBLIC_API_URL && typeof window === 'undefined') { + // Surface misconfig early in prod builds + throw new Error('NEXT_PUBLIC_API_URL must be set in production'); +}Please confirm Better Auth supports same-origin base URLs on the client.
packages/nextjs/src/app/layout-client.tsx (2)
20-25: Include 'auto' in the stored type and validate value.Current cast excludes 'auto', making the assignment unsafe. Support all modes and guard invalid values.
- useEffect(() => { - const storedColorScheme = localStorage.getItem('mantine-color-scheme') as 'light' | 'dark' | null; - if (storedColorScheme) { - setColorScheme(storedColorScheme); - } - }, []); + useEffect(() => { + const stored = localStorage.getItem('mantine-color-scheme') as 'light' | 'dark' | 'auto' | null; + if (stored === 'light' || stored === 'dark' || stored === 'auto') { + setColorScheme(stored); + } + }, []);
27-35: Verify html/body rendering strategy with Next.js app router.This client component renders and . Ensure layout.tsx doesn’t also render them to avoid duplication/hydration issues. If layout.tsx is the root layout, prefer keeping html/body there and having this component render only the providers.
Additionally, consider adding Mantine’s ColorSchemeScript in the root layout head to prevent theme flash during hydration.
packages/nextjs/src/app/api/v1/pages/route.ts (2)
27-44: Optional: validate parent type and simplify parentId.If containers can have multiple types, ensure the parent is a 'page'. Also, no need to pre-init parentId.
- let parentId = null; - if (body.parentId) { - const parentPage = await containerRepository.getOneByQuery( - addUserIdToQuery(containerRepository.createQuery().eq('id', body.parentId), session.user.id) - ); + let parentId = null as string | null; + if (body.parentId) { + const q = containerRepository.createQuery().eq('id', body.parentId); + // If applicable: + // q.eq('type', 'page'); + const parentPage = await containerRepository.getOneByQuery( + addUserIdToQuery(q, session.user.id) + ); @@ - } - - // Ensure the parent page belongs to the same workspace + } + // Ensure the parent page belongs to the same workspace if (parentPage.workspaceId !== workspace.id) { throw new Error('Parent page does not belong to the same workspace'); } parentId = body.parentId; }
46-56: Minor: reuse timestamp and drop redundant null coalescing.Compute ISO timestamp once and avoid
|| nullwhen the value is already nullish.- const pageData = { + const now = new Date().toISOString(); + const pageData = { name: body.name, emoji: body.emoji || null, type: 'page' as const, - parentId: parentId || null, + parentId, workspaceId: workspace.id, userId: session.user.id, - lastUpdated: new Date().toISOString(), - createdAt: new Date().toISOString(), + lastUpdated: now, + createdAt: now, };packages/nextjs/src/app/pages/[id]/create/page.tsx (1)
29-48: Improve error handling robustness.Line 44 assumes the caught error has a
messageproperty, but errors can be of any type. Consider adding a fallback for safer error handling.Apply this diff:
} catch (error_: any) { - setError(error_.message); + setError(error_?.message || 'Failed to create subpage'); } finally {packages/nextjs/src/components/molecules/sidebar/logged-in-container.tsx (1)
9-24: LGTM with optional enhancement.The component correctly handles loading and data states. For improved user experience, consider adding error state handling from the store.
Optional: Add error state handling if the store exposes error information:
export function LoggedInContainer() { - const { loading, data: rootPagesTree } = useStore($rootPagesTree); + const { loading, data: rootPagesTree, error } = useStore($rootPagesTree); return ( <Box> <Group justify="space-between" mb="sm"> <Title order={3}>Pages</Title> <ActionIcon variant="subtle" size="sm" component={Link} href="/pages/create" aria-label="Add page"> <IconPlus size={16} /> </ActionIcon> </Group> {loading && <Loader size="sm" />} + {error && <Text c="red">Failed to load pages</Text>} {!loading && rootPagesTree && <PagesTree branches={rootPagesTree.branches} />} </Box> ); }packages/nextjs/src/components/atoms/tree-item.tsx (1)
11-79: Consider using CSS hover states instead of inline event handlers.The current implementation uses
onMouseEnterandonMouseLeaveto directly manipulate DOM styles, which is not idiomatic in React and leads to code duplication between the two render paths.Refactor to use Mantine's
sxprop or CSS modules:+const useStyles = () => ({ + itemBase: { + display: 'flex', + alignItems: 'center', + gap: 8, + padding: '4px 8px', + borderRadius: 4, + transition: 'background-color 0.1s', + }, + itemHoverable: { + '&:hover': { + backgroundColor: 'var(--mantine-color-gray-1)', + }, + }, +}); + export function TreeItem({ name, emoji, to, onClick }: TreeItemProperties) { + const styles = useStyles(); const content = ( <> <span>{emoji ?? '📄'}</span> <span>{name}</span> </> ); if (to) { return ( <Link href={to} style={{ textDecoration: 'none', color: 'inherit' }}> - <Text - size="sm" - style={{ - cursor: 'pointer', - display: 'flex', - alignItems: 'center', - gap: 8, - padding: '4px 8px', - borderRadius: 4, - transition: 'background-color 0.1s', - }} - onMouseEnter={(event) => { - event.currentTarget.style.backgroundColor = 'var(--mantine-color-gray-1)'; - }} - onMouseLeave={(event) => { - event.currentTarget.style.backgroundColor = 'transparent'; - }} - > + <Text size="sm" style={{ ...styles.itemBase, ...styles.itemHoverable, cursor: 'pointer' }}> {content} </Text> </Link> ); } return ( <Text size="sm" - style={{ - cursor: onClick ? 'pointer' : 'default', - display: 'flex', - alignItems: 'center', - gap: 8, - padding: '4px 8px', - borderRadius: 4, - transition: 'background-color 0.1s', - }} + style={{ ...styles.itemBase, ...(onClick ? styles.itemHoverable : {}), cursor: onClick ? 'pointer' : 'default' }} onClick={onClick} - onMouseEnter={(event) => { - if (onClick) { - event.currentTarget.style.backgroundColor = 'var(--mantine-color-gray-1)'; - } - }} - onMouseLeave={(event) => { - if (onClick) { - event.currentTarget.style.backgroundColor = 'transparent'; - } - }} > {content} </Text> ); }packages/nextjs/src/lib/store/tree-expanded-state.ts (1)
20-32: Consider reducing code duplication.
expandPageandcollapsePagehave identical logic except for the boolean value. Consider extracting a shared helper to reduce duplication.Apply this diff to refactor:
+const setPageExpanded = (pageId: string, expanded: boolean) => { + const current = $expandedPages.get(); + const newMap = new Map(current); + newMap.set(pageId, expanded); + $expandedPages.set(newMap); +}; + export const expandPage = (pageId: string) => { - const current = $expandedPages.get(); - const newMap = new Map(current); - newMap.set(pageId, true); - $expandedPages.set(newMap); + setPageExpanded(pageId, true); }; export const collapsePage = (pageId: string) => { - const current = $expandedPages.get(); - const newMap = new Map(current); - newMap.set(pageId, false); - $expandedPages.set(newMap); + setPageExpanded(pageId, false); };packages/nextjs/src/app/pages/[id]/page.tsx (1)
60-97: Remove unnecessary optional chaining after null check.Lines 64-85 use optional chaining (
page?.page) even though the null check at Lines 52-58 guaranteespageis non-null in this section. This creates unnecessary confusion and the<Loader />fallback on Line 65 is particularly misleading.Apply this diff to simplify:
<Group gap="sm"> - <Text size="xl">{page?.page.emoji}</Text> - <Title order={1}>{page?.page.name ?? <Loader />}</Title> + <Text size="xl">{page.page.emoji}</Text> + <Title order={1}>{page.page.name}</Title> </Group> <Card p="md" withBorder> <Stack gap="sm"> <Group gap="xs"> <Badge variant="light">Type</Badge> - <Text>{page?.page.type}</Text> + <Text>{page.page.type}</Text> </Group> <Group gap="xs"> <Badge variant="light">Created</Badge> - <Text>{page?.page.createdAt ? new Date(page.page.createdAt).toLocaleString() : ''}</Text> + <Text>{page.page.createdAt ? new Date(page.page.createdAt).toLocaleString() : ''}</Text> </Group> <Group gap="xs"> <Badge variant="light">Last Updated</Badge> - <Text>{page?.page.lastUpdated ? new Date(page.page.lastUpdated).toLocaleString() : ''}</Text> + <Text>{page.page.lastUpdated ? new Date(page.page.lastUpdated).toLocaleString() : ''}</Text> </Group> - {page?.page.parentId && ( + {page.page.parentId && ( <Group gap="xs"> <Badge variant="light">Parent</Badge> <Text>{page.page.parentId}</Text>packages/nextjs/src/types/schemas/entities/container.ts (1)
19-24: LGTM! Consider stricter emoji validation.The schema correctly extends the base container schema. The emoji field uses basic string validation (
.min(1).nullable()), which allows any non-empty string when non-null. Consider adding stricter validation if you need to ensure actual emoji characters.packages/nextjs/src/app/api/v1/pages/[id]/route.ts (2)
14-15: Drop unnecessary optional chaining; param is guaranteed by schema.
params?.idcan just beparams.id.-const databaseQuery = addUserIdToQuery(containerRepository.createQuery(), session.user.id).eq('id', params?.id); +const databaseQuery = addUserIdToQuery(containerRepository.createQuery(), session.user.id).eq('id', params.id);
22-32: Avoid unsafe cast of type; assert or set explicitly.Casting
page.type as 'page'may hide bad data. Either assert and 404 if mismatched or set literal'page'if the entity is guaranteed to be a page.Option A (assert):
- type: page.type as 'page', + type: (page.type === 'page' ? 'page' : (() => { throw new ApiError(404, 'Page not found'); })()),Option B (explicit literal, if invariant holds):
- type: page.type as 'page', + type: 'page',packages/nextjs/src/types/schemas/utilities.ts (1)
11-14: Tighten Zod constraints for dates and identifiers.
- Use
z.string().datetime()for ISO timestamps.- If IDs are UUIDs, prefer
z.string().uuid(). If not, at least add.min(1)consistently.- If
parentIdmay be absent (not only null), mark itoptional().nullable().-export const withTrackUpdatesSchema = z.object({ - lastUpdated: z.string(), - createdAt: z.string(), -}); +export const withTrackUpdatesSchema = z.object({ + lastUpdated: z.string().datetime(), + createdAt: z.string().datetime(), +}); -export const withIdSchema = z.object({ - id: z.string().min(1), -}); +export const withIdSchema = z.object({ + id: z.string().min(1), // or .uuid() if applicable +}); -export const withParentIdSchema = z.object({ - parentId: z.string().min(1).nullable(), -}); +export const withParentIdSchema = z.object({ + parentId: z.string().min(1).nullable().optional(), +}); -export const withUserIdSchema = z.object({ - userId: z.string().min(1), -}); +export const withUserIdSchema = z.object({ + userId: z.string().min(1), // or .uuid() +}); -export const withWorkspaceIdSchema = z.object({ - workspaceId: z.string().min(1), -}); +export const withWorkspaceIdSchema = z.object({ + workspaceId: z.string().min(1), // or .uuid() +});Also applies to: 3-5, 7-9, 16-22
packages/nextjs/src/lib/database/index.ts (1)
6-18: Guard against double initialization under concurrency/hot reload.Two simultaneous first calls can create two SuperSave instances. Add a promise guard to ensure single initialization.
-let database: SuperSave; +let database: SuperSave | undefined; +let databasePromise: Promise<SuperSave> | undefined; export async function getDatabase() { - if (database) { - return database; - } - - database = await SuperSave.create(environment.DB); + if (database) return database; + if (!databasePromise) { + databasePromise = (async () => { + const db = await SuperSave.create(environment.DB); + await db.addEntity(entities.Container); + await db.addEntity(entities.Workspace); + database = db; + return db; + })(); + } - await database.addEntity(entities.Container); - await database.addEntity(entities.Workspace); - return database; + return databasePromise; }packages/nextjs/src/lib/api/route-wrapper.ts (2)
31-31:paramsshouldn’t be typed as a Promise.Next.js supplies
{ params }as a plain object; theawaitis a no-op but the type is misleading.
64-67: Query parsing: preferrequest.nextUrland consider arrays.
Object.fromEntries(entries())drops repeated keys. If you need arrays (e.g.,?tag=a&tag=b), consider a small parser that usesgetAll.packages/nextjs/src/types/database/index.ts (1)
1-3: Use consistent naming for workspace schema.The workspace schema is imported as
workspaceSchemaEntity(line 3), re-exported asworkspaceSchema(line 20), but then used asworkspaceSchemaEntityfor type inference (line 24). This inconsistency reduces code clarity.Consider either:
- Import and use
workspaceSchemaconsistently throughout, or- Import as
workspaceSchemaEntityand use that name consistently in both the re-export and type inferenceApply this diff for consistent naming:
-import { workspaceSchema as workspaceSchemaEntity } from '../schemas/entities/workspace'; +import { workspaceSchema } from '../schemas/entities/workspace'; /** Workspace Entity Schema */ export { workspaceSchema } from '../schemas/entities/workspace'; -export const workspaceCreateSchema = workspaceSchemaEntity.omit({ id: true }); +export const workspaceCreateSchema = workspaceSchema.omit({ id: true }); -export type Workspace = z.infer<typeof workspaceSchemaEntity>; +export type Workspace = z.infer<typeof workspaceSchema>; export type WorkspaceCreate = z.infer<typeof workspaceCreateSchema>; /** End Workspace Entity Schema */Also applies to: 20-20, 24-24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (31)
packages/nextjs/public/file.svgis excluded by!**/*.svgpackages/nextjs/public/globe.svgis excluded by!**/*.svgpackages/nextjs/public/next.svgis excluded by!**/*.svgpackages/nextjs/public/vercel.svgis excluded by!**/*.svgpackages/nextjs/public/window.svgis excluded by!**/*.svgpackages/nextjs/src/app/favicon.icois excluded by!**/*.icopackages/nextjs/src/app/icons/android-icon-144x144.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/android-icon-192x192.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/android-icon-36x36.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/android-icon-48x48.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/android-icon-72x72.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/android-icon-96x96.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-114x114.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-120x120.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-144x144.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-152x152.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-180x180.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-57x57.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-60x60.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-72x72.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-76x76.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon-precomposed.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/apple-icon.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/favicon-16x16.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/favicon-32x32.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/favicon-96x96.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/ms-icon-144x144.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/ms-icon-150x150.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/ms-icon-310x310.pngis excluded by!**/*.pngpackages/nextjs/src/app/icons/ms-icon-70x70.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (69)
.editorconfig(1 hunks).github/workflows/ci.yaml(1 hunks).prettierrc(1 hunks)biome.json(0 hunks)package.json(1 hunks)packages/backend/biome.json(0 hunks)packages/backend/src/modules/containers/api/get-pages-tree.ts(1 hunks)packages/nextjs/.gitignore(1 hunks)packages/nextjs/.prettierrc(1 hunks)packages/nextjs/README.md(1 hunks)packages/nextjs/eslint.config.mjs(1 hunks)packages/nextjs/next.config.ts(1 hunks)packages/nextjs/package.json(1 hunks)packages/nextjs/src/app/(auth)/layout.tsx(1 hunks)packages/nextjs/src/app/(auth)/login/page.tsx(1 hunks)packages/nextjs/src/app/(auth)/logout/page.tsx(1 hunks)packages/nextjs/src/app/@sidebar/default.tsx(1 hunks)packages/nextjs/src/app/api/auth/[...auth]/route.ts(1 hunks)packages/nextjs/src/app/api/v1/pages/[id]/route.ts(1 hunks)packages/nextjs/src/app/api/v1/pages/route.ts(1 hunks)packages/nextjs/src/app/api/v1/pages/tree/route.ts(1 hunks)packages/nextjs/src/app/globals.css(1 hunks)packages/nextjs/src/app/layout-client.tsx(1 hunks)packages/nextjs/src/app/layout.tsx(1 hunks)packages/nextjs/src/app/manifest.json(1 hunks)packages/nextjs/src/app/page.tsx(1 hunks)packages/nextjs/src/app/pages/[id]/create/page.tsx(1 hunks)packages/nextjs/src/app/pages/[id]/page.tsx(1 hunks)packages/nextjs/src/app/pages/create/page.tsx(1 hunks)packages/nextjs/src/components/atoms/tree-item.tsx(1 hunks)packages/nextjs/src/components/atoms/tree-toggle.tsx(1 hunks)packages/nextjs/src/components/layout.tsx(1 hunks)packages/nextjs/src/components/molecules/pages-tree.tsx(1 hunks)packages/nextjs/src/components/molecules/sidebar/logged-in-container.tsx(1 hunks)packages/nextjs/src/components/molecules/tree-node.tsx(1 hunks)packages/nextjs/src/lib/api/client.ts(1 hunks)packages/nextjs/src/lib/api/route-wrapper.ts(1 hunks)packages/nextjs/src/lib/auth/client.ts(1 hunks)packages/nextjs/src/lib/auth/config.ts(1 hunks)packages/nextjs/src/lib/auth/provider.tsx(1 hunks)packages/nextjs/src/lib/auth/session.ts(1 hunks)packages/nextjs/src/lib/database/entities/container.ts(1 hunks)packages/nextjs/src/lib/database/entities/index.ts(1 hunks)packages/nextjs/src/lib/database/entities/workspace.ts(1 hunks)packages/nextjs/src/lib/database/helpers.ts(1 hunks)packages/nextjs/src/lib/database/index.ts(1 hunks)packages/nextjs/src/lib/environment.ts(1 hunks)packages/nextjs/src/lib/logger.ts(1 hunks)packages/nextjs/src/lib/store/fetcher.ts(1 hunks)packages/nextjs/src/lib/store/query/get-page-details.ts(1 hunks)packages/nextjs/src/lib/store/query/get-pages-tree.ts(1 hunks)packages/nextjs/src/lib/store/tree-expanded-state.ts(1 hunks)packages/nextjs/src/lib/theme.ts(1 hunks)packages/nextjs/src/types/api/endpoints/create-page.ts(1 hunks)packages/nextjs/src/types/api/endpoints/get-page-details.ts(1 hunks)packages/nextjs/src/types/api/endpoints/get-pages-tree.ts(1 hunks)packages/nextjs/src/types/api/endpoints/index.ts(1 hunks)packages/nextjs/src/types/api/entities.ts(1 hunks)packages/nextjs/src/types/api/index.ts(1 hunks)packages/nextjs/src/types/api/utilities.ts(1 hunks)packages/nextjs/src/types/database/index.ts(1 hunks)packages/nextjs/src/types/schemas/entities/container.ts(1 hunks)packages/nextjs/src/types/schemas/entities/index.ts(1 hunks)packages/nextjs/src/types/schemas/entities/workspace.ts(1 hunks)packages/nextjs/src/types/schemas/utilities.ts(1 hunks)packages/nextjs/tsconfig.json(1 hunks)packages/types/biome.json(0 hunks)packages/web/biome.json(0 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- packages/web/biome.json
- biome.json
- packages/types/biome.json
- packages/backend/biome.json
🧰 Additional context used
🧬 Code graph analysis (36)
packages/nextjs/src/types/schemas/entities/workspace.ts (1)
packages/nextjs/src/types/schemas/utilities.ts (3)
withTrackUpdatesSchema(11-14)withUserIdSchema(16-18)withIdSchema(3-5)
packages/nextjs/src/app/(auth)/layout.tsx (1)
packages/nextjs/src/components/layout.tsx (1)
Layout(13-69)
packages/nextjs/src/lib/logger.ts (1)
packages/nextjs/src/lib/environment.ts (1)
environment(3-15)
packages/nextjs/src/app/api/auth/[...auth]/route.ts (1)
packages/nextjs/src/lib/auth/config.ts (1)
auth(10-56)
packages/nextjs/src/lib/auth/session.ts (1)
packages/nextjs/src/lib/auth/config.ts (1)
auth(10-56)
packages/nextjs/src/lib/auth/provider.tsx (1)
packages/nextjs/src/lib/auth/client.ts (1)
authClient(4-6)
packages/nextjs/src/types/api/entities.ts (1)
packages/nextjs/src/types/schemas/entities/container.ts (1)
pageContainerSchema(19-24)
packages/nextjs/src/components/layout.tsx (2)
packages/nextjs/src/app/(auth)/layout.tsx (1)
Layout(4-6)packages/nextjs/src/lib/auth/provider.tsx (1)
useAuth(50-56)
packages/nextjs/src/lib/auth/config.ts (3)
packages/nextjs/src/lib/database/index.ts (3)
getDatabase(8-18)getWorkspaceRepository(25-28)getContainerRepository(20-23)packages/nextjs/src/lib/environment.ts (1)
environment(3-15)packages/nextjs/src/types/database/index.ts (2)
WorkspaceCreate(25-25)ContainerCreate(16-16)
packages/nextjs/src/types/api/endpoints/get-pages-tree.ts (3)
packages/nextjs/src/types/api/entities.ts (1)
pageSchema(3-6)packages/nextjs/src/types/schemas/utilities.ts (1)
withIdSchema(3-5)packages/nextjs/src/types/api/utilities.ts (1)
DataWrapper(2-4)
packages/nextjs/src/app/api/v1/pages/route.ts (4)
packages/nextjs/src/lib/api/route-wrapper.ts (1)
apiRoute(14-119)packages/nextjs/src/types/api/endpoints/create-page.ts (3)
CreatePageResponse(12-12)CreatePageBody(22-22)createPageBodySchema(16-20)packages/nextjs/src/lib/database/index.ts (2)
getWorkspaceRepository(25-28)getContainerRepository(20-23)packages/nextjs/src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)
packages/nextjs/src/app/api/v1/pages/[id]/route.ts (5)
packages/nextjs/src/app/api/v1/pages/tree/route.ts (1)
GET(7-63)packages/nextjs/src/lib/api/route-wrapper.ts (1)
apiRoute(14-119)packages/nextjs/src/types/api/endpoints/get-page-details.ts (3)
GetPageDetailsResponse(11-11)GetPageDetailsParameters(17-17)getPageDetailsParametersSchema(14-16)packages/nextjs/src/lib/database/index.ts (1)
getContainerRepository(20-23)packages/nextjs/src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)
packages/nextjs/src/lib/store/fetcher.ts (1)
packages/nextjs/src/lib/api/client.ts (1)
apiClient(3-8)
packages/nextjs/src/app/(auth)/logout/page.tsx (1)
packages/nextjs/src/lib/auth/provider.tsx (1)
useAuth(50-56)
packages/nextjs/src/components/molecules/pages-tree.tsx (2)
packages/nextjs/src/types/api/endpoints/get-pages-tree.ts (1)
GetPagesTreeResponse(23-23)packages/nextjs/src/components/molecules/tree-node.tsx (1)
TreeNode(26-83)
packages/nextjs/src/app/pages/create/page.tsx (2)
packages/nextjs/src/lib/auth/provider.tsx (1)
useAuth(50-56)packages/nextjs/src/lib/api/client.ts (1)
api(10-22)
packages/nextjs/src/lib/database/entities/container.ts (3)
packages/nextjs/src/lib/database/entities/index.ts (3)
NAME(1-1)NAME(2-2)Container(1-1)packages/nextjs/src/lib/database/entities/workspace.ts (1)
NAME(3-3)packages/nextjs/src/types/database/index.ts (1)
Container(15-15)
packages/nextjs/src/types/schemas/entities/container.ts (2)
packages/nextjs/src/types/database/index.ts (2)
containerSchema(13-13)pageContainerSchema(6-6)packages/nextjs/src/types/schemas/utilities.ts (5)
withTrackUpdatesSchema(11-14)withWorkspaceIdSchema(20-22)withUserIdSchema(16-18)withIdSchema(3-5)withParentIdSchema(7-9)
packages/nextjs/src/components/molecules/sidebar/logged-in-container.tsx (2)
packages/nextjs/src/lib/store/query/get-pages-tree.ts (1)
$rootPagesTree(4-4)packages/nextjs/src/components/molecules/pages-tree.tsx (1)
PagesTree(9-25)
packages/nextjs/src/app/api/v1/pages/tree/route.ts (4)
packages/nextjs/src/lib/api/route-wrapper.ts (1)
apiRoute(14-119)packages/nextjs/src/types/api/endpoints/get-pages-tree.ts (3)
GetPagesTreeResponse(23-23)GetPagesTreeQueryVariables(29-29)getPagesTreeQueryVariablesSchema(26-28)packages/nextjs/src/lib/database/index.ts (1)
getContainerRepository(20-23)packages/nextjs/src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)
packages/nextjs/src/app/pages/[id]/create/page.tsx (2)
packages/nextjs/src/lib/auth/provider.tsx (1)
useAuth(50-56)packages/nextjs/src/lib/api/client.ts (1)
api(10-22)
packages/nextjs/src/types/api/endpoints/get-page-details.ts (2)
packages/nextjs/src/types/api/entities.ts (1)
pageSchema(3-6)packages/nextjs/src/types/api/utilities.ts (1)
DataWrapper(2-4)
packages/nextjs/src/lib/api/route-wrapper.ts (2)
packages/nextjs/src/lib/auth/session.ts (1)
getSession(4-15)packages/nextjs/src/lib/logger.ts (1)
logger(4-17)
packages/nextjs/src/lib/store/query/get-page-details.ts (2)
packages/nextjs/src/lib/store/fetcher.ts (1)
createFetcherStore(4-6)packages/nextjs/src/types/api/endpoints/get-page-details.ts (1)
GetPageDetailsResponse(11-11)
packages/nextjs/src/types/api/endpoints/create-page.ts (2)
packages/nextjs/src/types/api/entities.ts (1)
pageSchema(3-6)packages/nextjs/src/types/api/utilities.ts (1)
DataWrapper(2-4)
packages/nextjs/src/app/@sidebar/default.tsx (1)
packages/nextjs/src/components/molecules/sidebar/logged-in-container.tsx (1)
LoggedInContainer(9-24)
packages/nextjs/src/app/pages/[id]/page.tsx (2)
packages/nextjs/src/lib/auth/provider.tsx (1)
useAuth(50-56)packages/nextjs/src/lib/store/query/get-page-details.ts (2)
$currentPage(7-7)$currentPageId(5-5)
packages/nextjs/src/types/database/index.ts (3)
packages/nextjs/src/types/schemas/entities/container.ts (2)
pageContainerSchema(19-24)containerSchema(10-17)packages/nextjs/src/lib/database/entities/container.ts (1)
Container(5-18)packages/nextjs/src/lib/database/entities/workspace.ts (1)
Workspace(5-15)
packages/nextjs/src/app/layout-client.tsx (2)
packages/nextjs/src/lib/theme.ts (1)
theme(3-5)packages/nextjs/src/lib/auth/provider.tsx (1)
AuthProvider(16-48)
packages/backend/src/modules/containers/api/get-pages-tree.ts (4)
packages/backend/src/core/api/api-route.ts (1)
apiRoute(16-92)packages/types/src/api/endpoints/get-pages-tree.ts (3)
GetPagesTreeResponse(23-23)GetPagesTreeQueryVariables(29-31)getPagesTreeQueryVariablesSchema(26-28)packages/backend/src/modules/database/index.ts (1)
getContainerRepository(23-25)packages/backend/src/modules/database/helpers.ts (1)
addUserIdToQuery(3-5)
packages/nextjs/src/lib/database/entities/workspace.ts (3)
packages/nextjs/src/lib/database/entities/container.ts (1)
NAME(3-3)packages/nextjs/src/lib/database/entities/index.ts (3)
NAME(1-1)NAME(2-2)Workspace(2-2)packages/nextjs/src/types/database/index.ts (1)
Workspace(24-24)
packages/nextjs/src/lib/store/query/get-pages-tree.ts (2)
packages/nextjs/src/lib/store/fetcher.ts (1)
createFetcherStore(4-6)packages/nextjs/src/types/api/endpoints/get-pages-tree.ts (2)
GetPagesTreeResponse(23-23)GET_PAGES_TREE_ENDPOINT(6-6)
packages/nextjs/src/app/layout.tsx (3)
packages/nextjs/src/lib/auth/config.ts (1)
auth(10-56)packages/nextjs/src/app/layout-client.tsx (1)
RootClientLayout(13-36)packages/nextjs/src/components/layout.tsx (1)
Layout(13-69)
packages/nextjs/src/lib/database/index.ts (4)
packages/nextjs/src/lib/environment.ts (1)
environment(3-15)packages/nextjs/src/lib/database/entities/container.ts (1)
Container(5-18)packages/nextjs/src/types/database/index.ts (2)
Container(15-15)Workspace(24-24)packages/nextjs/src/lib/database/entities/workspace.ts (1)
Workspace(5-15)
packages/nextjs/src/app/(auth)/login/page.tsx (1)
packages/nextjs/src/lib/auth/client.ts (1)
authClient(4-6)
packages/nextjs/src/components/molecules/tree-node.tsx (3)
packages/nextjs/src/lib/store/tree-expanded-state.ts (2)
$expandedPages(3-3)togglePageExpanded(6-14)packages/nextjs/src/components/atoms/tree-toggle.tsx (1)
TreeToggle(10-20)packages/nextjs/src/components/atoms/tree-item.tsx (1)
TreeItem(11-79)
packages/nextjs/package.json
Outdated
| "scripts": { | ||
| "dev": "next dev --turbopack", | ||
| "build": "next build --turbopack", | ||
| "start": "next start", | ||
| "lint": "concurrently \"pnpm:lint:*\"", | ||
| "lint:eslint": "eslint . --ext .ts,.tsx", | ||
| "format": "prettier --write src && eslint --ext=js,jsx,ts,tsx --fix src", | ||
| "lint:tsc": "tsc --noEmit" | ||
| }, |
There was a problem hiding this comment.
Fix lint script: missing concurrently dep and unsupported pnpm: pattern.
lint will fail: concurrently isn’t installed, and concurrently supports npm: wildcards, not pnpm:. Call scripts explicitly and add the devDep.
"scripts": {
- "dev": "next dev --turbopack",
- "build": "next build --turbopack",
+ "dev": "next dev --turbopack",
+ "build": "next build --turbopack",
"start": "next start",
- "lint": "concurrently \"pnpm:lint:*\"",
- "lint:eslint": "eslint . --ext .ts,.tsx",
+ "lint": "concurrently -n eslint,types -c auto \"pnpm run lint:eslint\" \"pnpm run lint:tsc\"",
+ "lint:eslint": "eslint . --ext .ts,.tsx",
"format": "prettier --write src && eslint --ext=js,jsx,ts,tsx --fix src",
"lint:tsc": "tsc --noEmit"
},
@@
"devDependencies": {
+ "concurrently": "^9.0.0",
"@eslint/eslintrc": "^3",Also applies to: 35-47
🤖 Prompt for AI Agents
packages/nextjs/package.json lines 6-14 (and similarly lines 35-47): the "lint"
script uses an unsupported "pnpm:" pattern and relies on a missing concurrently
package, causing failures; add "concurrently" to devDependencies and update the
"lint" script to call the child scripts explicitly (e.g., invoke them via "pnpm
run <script>" or list the commands directly) rather than using the "pnpm:"
shortcut, and apply the same change to the other affected script blocks on lines
35-47.
| useEffect(() => { | ||
| signOut(); | ||
| router.push('/'); | ||
| }, [router, signOut]); |
There was a problem hiding this comment.
Await signOut before navigation to prevent race condition.
The signOut() call is not awaited before router.push('/') executes. This creates a race condition where navigation may occur before the sign-out completes, potentially leaving the session in an inconsistent state or failing to properly clear authentication.
Apply this diff to properly await the sign-out:
useEffect(() => {
- signOut();
- router.push('/');
+ const handleLogout = async () => {
+ await signOut();
+ router.push('/');
+ };
+ handleLogout();
}, [router, signOut]);📝 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.
| useEffect(() => { | |
| signOut(); | |
| router.push('/'); | |
| }, [router, signOut]); | |
| useEffect(() => { | |
| const handleLogout = async () => { | |
| await signOut(); | |
| router.push('/'); | |
| }; | |
| handleLogout(); | |
| }, [router, signOut]); |
🤖 Prompt for AI Agents
In packages/nextjs/src/app/(auth)/logout/page.tsx around lines 12 to 15, the
useEffect invokes signOut() and immediately calls router.push('/') causing a
race condition; change the effect to await the signOut call before navigating
(e.g., create and call an async inner function that awaits signOut() and only
after it resolves calls router.push('/')), handle/rethrow errors or log them as
appropriate, and keep router and signOut in the dependency array.
| const refreshSession = async () => { | ||
| try { | ||
| const response = await authClient.getSession(); | ||
| if (response.data?.user) { | ||
| setUser(response.data.user); | ||
| } else { | ||
| setUser(null); | ||
| } | ||
| } catch (error) { | ||
| console.error('Failed to get session:', error); | ||
| setUser(null); | ||
| } | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| refreshSession().finally(() => setLoading(false)); | ||
| }, []); |
There was a problem hiding this comment.
Wrap refreshSession in useCallback to fix missing dependency warning.
The refreshSession function is called in the useEffect but not included in its dependency array. This will trigger React Hook warnings. Since refreshSession depends on setUser, wrap it in useCallback to stabilize the reference.
- const refreshSession = async () => {
+ const refreshSession = useCallback(async () => {
try {
const response = await authClient.getSession();
if (response.data?.user) {
setUser(response.data.user);
} else {
setUser(null);
}
} catch (error) {
console.error('Failed to get session:', error);
setUser(null);
}
- };
+ }, []);
useEffect(() => {
- refreshSession().finally(() => setLoading(false));
- }, []);
+ refreshSession().finally(() => setLoading(false));
+ }, [refreshSession]);Don't forget to import useCallback:
-import { createContext, useContext, useEffect, useState } from 'react';
+import { createContext, useCallback, useContext, useEffect, useState } from 'react';📝 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 refreshSession = async () => { | |
| try { | |
| const response = await authClient.getSession(); | |
| if (response.data?.user) { | |
| setUser(response.data.user); | |
| } else { | |
| setUser(null); | |
| } | |
| } catch (error) { | |
| console.error('Failed to get session:', error); | |
| setUser(null); | |
| } | |
| }; | |
| useEffect(() => { | |
| refreshSession().finally(() => setLoading(false)); | |
| }, []); | |
| import { createContext, useCallback, useContext, useEffect, useState } from 'react'; |
| const refreshSession = async () => { | |
| try { | |
| const response = await authClient.getSession(); | |
| if (response.data?.user) { | |
| setUser(response.data.user); | |
| } else { | |
| setUser(null); | |
| } | |
| } catch (error) { | |
| console.error('Failed to get session:', error); | |
| setUser(null); | |
| } | |
| }; | |
| useEffect(() => { | |
| refreshSession().finally(() => setLoading(false)); | |
| }, []); | |
| const refreshSession = useCallback(async () => { | |
| try { | |
| const response = await authClient.getSession(); | |
| if (response.data?.user) { | |
| setUser(response.data.user); | |
| } else { | |
| setUser(null); | |
| } | |
| } catch (error) { | |
| console.error('Failed to get session:', error); | |
| setUser(null); | |
| } | |
| }, []); | |
| useEffect(() => { | |
| refreshSession().finally(() => setLoading(false)); | |
| }, [refreshSession]); |
🤖 Prompt for AI Agents
In packages/nextjs/src/lib/auth/provider.tsx around lines 20 to 36, wrap the
refreshSession function in useCallback and import useCallback from React to
stabilize its identity; define refreshSession using useCallback(async () => {
... }, [authClient, setUser]) (include any other React state/props used), then
update the useEffect to list refreshSession in its dependency array
(useEffect(() => { refreshSession().finally(() => setLoading(false)); },
[refreshSession])); ensure the useCallback import is added at the top of the
file.
| export const [createFetcherStore, createMutatorStore] = nanoquery({ | ||
| fetcher: (...keys) => apiClient.get(keys.join('')).then((r) => r.data.data), | ||
| }); |
There was a problem hiding this comment.
Fragile URL construction with keys.join('').
Joining all keys risks malformed paths (missing slashes, accidental concatenation) and ignores query objects. Build the URL from the first key and merge remaining object keys into query params. Optionally add a timeout.
-export const [createFetcherStore, createMutatorStore] = nanoquery({
- fetcher: (...keys) => apiClient.get(keys.join('')).then((r) => r.data.data),
-});
+export const [createFetcherStore, createMutatorStore] = nanoquery({
+ fetcher: async (...keys) => {
+ const [path, ...rest] = keys;
+ const url = String(path);
+ const params = Object.assign(
+ {},
+ ...rest.filter((k) => k && typeof k === 'object') as Record<string, any>[]
+ );
+ const res = await apiClient.get(url, {
+ params: Object.keys(params).length ? params : undefined,
+ timeout: 10_000,
+ });
+ return res.data.data;
+ },
+});📝 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.
| export const [createFetcherStore, createMutatorStore] = nanoquery({ | |
| fetcher: (...keys) => apiClient.get(keys.join('')).then((r) => r.data.data), | |
| }); | |
| export const [createFetcherStore, createMutatorStore] = nanoquery({ | |
| fetcher: async (...keys) => { | |
| const [path, ...rest] = keys; | |
| const url = String(path); | |
| const params = Object.assign( | |
| {}, | |
| ...rest.filter((k) => k && typeof k === 'object') as Record<string, any>[] | |
| ); | |
| const res = await apiClient.get(url, { | |
| params: Object.keys(params).length ? params : undefined, | |
| timeout: 10_000, | |
| }); | |
| return res.data.data; | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/nextjs/src/lib/store/fetcher.ts around lines 4 to 6, the fetcher
currently builds URLs by doing keys.join('') which can produce malformed paths
and ignores query objects; change it to treat the first key as the base path and
any remaining keys as query parameter objects (merge them into a single params
object), serialize them via URLSearchParams or pass them as axios/HTTP client
params (to ensure proper encoding), and call apiClient.get(basePath, { params:
mergedParams, timeout: <reasonable-ms> }) so URLs are constructed robustly and
optional timeouts are applied.
| export const containerCreateSchema = pageContainerSchema.omit({ id: true }); | ||
|
|
||
| export const containerSchema = pageContainerSchema; | ||
|
|
||
| export type Container = z.infer<typeof containerSchema>; | ||
| export type ContainerCreate = z.infer<typeof containerCreateSchema>; |
There was a problem hiding this comment.
Container and PageContainer should be distinct types, not aliases.
Lines 13 and 11 alias containerSchema and containerCreateSchema to the page-specific schemas, but based on the schema definitions in container.ts, containerSchema is a base schema that pageContainerSchema extends with additional fields (type: 'page', emoji, parentId). By treating them as aliases, the code loses the base Container type and forces all Container usages to include page-specific fields, breaking type safety.
Apply this diff to import and export the distinct types:
-import type { z } from 'zod';
-import { pageContainerSchema } from '../schemas/entities/container';
+import type { z } from 'zod';
+import { containerSchema, pageContainerSchema } from '../schemas/entities/container';
import { workspaceSchema as workspaceSchemaEntity } from '../schemas/entities/workspace';
+/** Container Entity Schema */
+export { containerSchema } from '../schemas/entities/container';
+export const containerCreateSchema = containerSchema.omit({ id: true });
+export type Container = z.infer<typeof containerSchema>;
+export type ContainerCreate = z.infer<typeof containerCreateSchema>;
+/** End Container Entity Schema */
+
/** Container Entity Schema */
export { pageContainerSchema } from '../schemas/entities/container';
export const pageContainerCreateSchema = pageContainerSchema.omit({ id: true });
export type PageContainer = z.infer<typeof pageContainerSchema>;
export type PageContainerCreate = z.infer<typeof pageContainerCreateSchema>;
-
-export const containerCreateSchema = pageContainerSchema.omit({ id: true });
-
-export const containerSchema = pageContainerSchema;
-
-export type Container = z.infer<typeof containerSchema>;
-export type ContainerCreate = z.infer<typeof containerCreateSchema>;
-/** End Container Entity Schema */
+/** End PageContainer Entity Schema */Committable suggestion skipped: line range outside the PR's diff.
packages/nextjs/tsconfig.json
Outdated
| "paths": { | ||
| "@/*": ["./src/*"] | ||
| } |
There was a problem hiding this comment.
Add baseUrl so the @/* path alias actually resolves.
Without "baseUrl": ".", TypeScript ignores "paths". This will break @/… imports during tsc --noEmit.
Apply this diff:
"compilerOptions": {
+ "baseUrl": ".",
"target": "ES2022",Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/nextjs/tsconfig.json around lines 21 to 23, the "paths" mapping for
"@/*" won't be used by TypeScript unless "baseUrl" is set; add "baseUrl": "." to
the compilerOptions alongside "paths" (i.e., ensure compilerOptions contains
"baseUrl": "." and then keep the existing "paths" entry) so the "@/..." imports
resolve during tsc --noEmit.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
packages/nextjs/package.json (2)
10-10: ** Fix lint script: missingconcurrentlyand unsupportedpnpm:pattern.**The
lintscript will fail because (1)concurrentlyis not in devDependencies, and (2)concurrentlydoes not support thepnpm:wildcard pattern—it usesnpm:instead. This issue was flagged in the previous review. Please addconcurrentlyand call the lint scripts explicitly."scripts": { "dev": "next dev --turbopack", "build": "next build --turbopack", "start": "next start", - "lint": "concurrently \"pnpm:lint:*\"", + "lint": "concurrently -n eslint,tsc -c auto \"pnpm run lint:eslint\" \"pnpm run lint:tsc\"", "lint:eslint": "eslint . --ext .ts,.tsx", "format": "prettier --write src && eslint --ext=js,jsx,ts,tsx --fix src", "lint:tsc": "tsc --noEmit" }, "devDependencies": { + "concurrently": "^9.0.0", "@eslint/eslintrc": "^3",Also applies to: 35-47
10-10: ** Fix lint script: missingconcurrentlyand unsupportedpnpm:pattern.**The
lintscript will fail because (1)concurrentlyis not in devDependencies, and (2)concurrentlydoes not support thepnpm:wildcard pattern—it usesnpm:instead. This issue was flagged in the previous review and remains unresolved."scripts": { "dev": "next dev --turbopack", "build": "next build --turbopack", "start": "next start", - "lint": "concurrently \"pnpm:lint:*\"", + "lint": "concurrently -n eslint,tsc -c auto \"pnpm run lint:eslint\" \"pnpm run lint:tsc\"", "lint:eslint": "eslint . --ext .ts,.tsx", "format": "prettier --write src && eslint --ext=js,jsx,ts,tsx --fix src", "lint:tsc": "tsc --noEmit" }, "devDependencies": { + "concurrently": "^9.0.0", "@eslint/eslintrc": "^3",Also applies to: 35-47
packages/nextjs/src/app/pages/[id]/create/page.tsx (1)
6-8: Unsafe type assertion for route parameter.The
idparameter fromuseParams()can bestring | string[] | undefined, but line 8 casts it tostringwithout validation. This could cause runtime issues if the parameter is an array or undefined.Validate the parameter before use:
export default function CreateSubpagePage() { const parameters = useParams(); - const parentId = parameters.id as string; + const parentId = Array.isArray(parameters.id) ? parameters.id[0] : parameters.id; + + if (!parentId) { + return ( + <Container size="md" py="xl"> + <Text>Invalid page ID.</Text> + </Container> + ); + } return (
🧹 Nitpick comments (3)
packages/nextjs/src/components/layout.tsx (2)
20-24: Removerouterfrom the dependency array.The
routerinstance fromuseRouter()is stable in Next.js and doesn't need to be included in the dependency array. Including it may cause unnecessary effect re-runs.Apply this diff:
useEffect(() => { if (!loading && !user) { router.push('/login'); } - }, [loading, user, router]); + }, [loading, user]);
26-28: Center the Loader component for better UX.The Loader is rendered without any centering or container, which may not provide a good visual experience during the loading state.
Apply this diff to center the loader:
if (loading) { - return <Loader />; + return ( + <div style={{ display: 'flex', justifyContent: 'center', alignItems: 'center', height: '100vh' }}> + <Loader /> + </div> + ); }packages/nextjs/src/lib/hooks/use-cud-api.ts (1)
5-7: Consider handling concurrent requests explicitly.The current implementation uses single
inProgressanderrorstates. If multiple requests are fired concurrently (e.g., multiple delete operations), these states will be overwritten by the latest request. Consider either documenting this behavior or implementing request tracking (e.g., using a request ID map) if concurrent operations are expected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/web/.yarn/releases/yarn-4.10.3.cjsis excluded by!**/.yarn/**packages/web/public/favicon.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (70)
.vscode/settings.json(0 hunks)packages/backend/package.json(0 hunks)packages/backend/scripts/clear-auth.ts(0 hunks)packages/backend/src/core/api/api-route.ts(0 hunks)packages/backend/src/core/api/errors.ts(0 hunks)packages/backend/src/core/api/get-session.ts(0 hunks)packages/backend/src/core/api/initialize-http-api.ts(0 hunks)packages/backend/src/core/environment.ts(0 hunks)packages/backend/src/core/logger.ts(0 hunks)packages/backend/src/index.ts(0 hunks)packages/backend/src/modules/auth/better-auth.ts(0 hunks)packages/backend/src/modules/auth/initialize.ts(0 hunks)packages/backend/src/modules/auth/middleware.ts(0 hunks)packages/backend/src/modules/containers/api/create-page.ts(0 hunks)packages/backend/src/modules/containers/api/get-page-details.ts(0 hunks)packages/backend/src/modules/containers/api/get-pages-tree.ts(0 hunks)packages/backend/src/modules/database/entities/container.ts(0 hunks)packages/backend/src/modules/database/entities/index.ts(0 hunks)packages/backend/src/modules/database/entities/workspace.ts(0 hunks)packages/backend/src/modules/database/index.ts(0 hunks)packages/backend/tsconfig.json(0 hunks)packages/nextjs/eslint.config.mjs(1 hunks)packages/nextjs/package.json(1 hunks)packages/nextjs/src/app/pages/[id]/create/page.tsx(1 hunks)packages/nextjs/src/app/pages/create/page.tsx(1 hunks)packages/nextjs/src/components/layout.tsx(1 hunks)packages/nextjs/src/components/molecules/create-page-form.tsx(1 hunks)packages/nextjs/src/lib/hooks/use-cud-api.ts(1 hunks)packages/types/README.md(0 hunks)packages/types/package.json(0 hunks)packages/types/src/api/endpoints/get-page-details.ts(0 hunks)packages/types/src/api/endpoints/get-pages-tree.ts(0 hunks)packages/types/src/api/endpoints/index.ts(0 hunks)packages/types/src/api/index.ts(0 hunks)packages/types/src/api/utils.ts(0 hunks)packages/types/src/schemas/entities/container.ts(0 hunks)packages/types/src/schemas/entities/workspace.ts(0 hunks)packages/types/tsconfig.json(0 hunks)packages/web/.gitignore(0 hunks)packages/web/README.md(0 hunks)packages/web/index.html(0 hunks)packages/web/package.json(0 hunks)packages/web/postcss.config.cjs(0 hunks)packages/web/src/components/app.tsx(0 hunks)packages/web/src/components/atoms/tree-item.tsx(0 hunks)packages/web/src/components/atoms/tree-toggle.tsx(0 hunks)packages/web/src/components/layout.tsx(0 hunks)packages/web/src/components/molecules/pages-tree.tsx(0 hunks)packages/web/src/components/molecules/sidebar/logged-in-container.tsx(0 hunks)packages/web/src/components/molecules/tree-node.tsx(0 hunks)packages/web/src/components/router.tsx(0 hunks)packages/web/src/lib/api/client.ts(0 hunks)packages/web/src/lib/auth/client.ts(0 hunks)packages/web/src/lib/hooks/use-cud-api.ts(0 hunks)packages/web/src/lib/store/fetcher.ts(0 hunks)packages/web/src/lib/store/query/get-page-details.ts(0 hunks)packages/web/src/lib/store/query/get-pages-tree.ts(0 hunks)packages/web/src/lib/store/tree-expanded-state.ts(0 hunks)packages/web/src/main.tsx(0 hunks)packages/web/src/routes/create-page.tsx(0 hunks)packages/web/src/routes/home.tsx(0 hunks)packages/web/src/routes/login.tsx(0 hunks)packages/web/src/routes/logout.tsx(0 hunks)packages/web/src/routes/page.tsx(0 hunks)packages/web/src/theme.ts(0 hunks)packages/web/src/vite-env.d.ts(0 hunks)packages/web/tsconfig.json(0 hunks)packages/web/tsconfig.node.json(0 hunks)packages/web/vite.config.ts(0 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (62)
- .vscode/settings.json
- packages/backend/src/modules/auth/better-auth.ts
- packages/types/README.md
- packages/backend/src/modules/auth/initialize.ts
- packages/backend/src/core/api/errors.ts
- packages/backend/src/core/api/initialize-http-api.ts
- packages/web/src/routes/home.tsx
- packages/types/src/api/index.ts
- packages/web/src/vite-env.d.ts
- packages/types/src/api/utils.ts
- packages/types/src/api/endpoints/index.ts
- packages/web/src/routes/page.tsx
- packages/web/src/routes/create-page.tsx
- packages/web/src/lib/store/tree-expanded-state.ts
- packages/backend/src/modules/database/index.ts
- packages/web/src/lib/store/query/get-page-details.ts
- packages/web/src/routes/login.tsx
- packages/types/src/schemas/entities/container.ts
- packages/types/src/api/endpoints/get-pages-tree.ts
- packages/backend/src/core/logger.ts
- packages/web/README.md
- packages/backend/package.json
- packages/backend/src/core/api/api-route.ts
- packages/backend/src/modules/containers/api/get-page-details.ts
- packages/web/src/components/molecules/pages-tree.tsx
- packages/web/src/components/layout.tsx
- packages/backend/src/modules/database/entities/workspace.ts
- packages/types/src/api/endpoints/get-page-details.ts
- packages/web/src/components/atoms/tree-item.tsx
- packages/web/src/components/app.tsx
- packages/web/src/components/molecules/tree-node.tsx
- packages/web/src/lib/api/client.ts
- packages/web/tsconfig.json
- packages/backend/src/modules/database/entities/index.ts
- packages/web/src/components/router.tsx
- packages/web/.gitignore
- packages/web/src/components/molecules/sidebar/logged-in-container.tsx
- packages/backend/scripts/clear-auth.ts
- packages/backend/src/modules/containers/api/get-pages-tree.ts
- packages/backend/src/core/environment.ts
- packages/backend/src/modules/containers/api/create-page.ts
- packages/web/tsconfig.node.json
- packages/backend/src/index.ts
- packages/web/postcss.config.cjs
- packages/backend/tsconfig.json
- packages/web/src/lib/auth/client.ts
- packages/web/src/routes/logout.tsx
- packages/types/tsconfig.json
- packages/web/index.html
- packages/web/src/main.tsx
- packages/web/src/lib/store/fetcher.ts
- packages/types/src/schemas/entities/workspace.ts
- packages/backend/src/core/api/get-session.ts
- packages/web/package.json
- packages/web/src/components/atoms/tree-toggle.tsx
- packages/backend/src/modules/auth/middleware.ts
- packages/web/src/lib/store/query/get-pages-tree.ts
- packages/types/package.json
- packages/web/vite.config.ts
- packages/web/src/theme.ts
- packages/web/src/lib/hooks/use-cud-api.ts
- packages/backend/src/modules/database/entities/container.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nextjs/eslint.config.mjs
🧰 Additional context used
🧬 Code graph analysis (4)
packages/nextjs/src/components/layout.tsx (2)
packages/nextjs/src/app/(auth)/layout.tsx (1)
Layout(4-6)packages/nextjs/src/lib/auth/provider.tsx (1)
useAuth(50-56)
packages/nextjs/src/components/molecules/create-page-form.tsx (2)
packages/nextjs/src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)packages/nextjs/src/types/api/endpoints/create-page.ts (2)
CreatePageResponse(12-12)CreatePageBody(22-22)
packages/nextjs/src/app/pages/create/page.tsx (1)
packages/nextjs/src/components/molecules/create-page-form.tsx (1)
CreatePageForm(15-71)
packages/nextjs/src/app/pages/[id]/create/page.tsx (1)
packages/nextjs/src/components/molecules/create-page-form.tsx (1)
CreatePageForm(15-71)
🔇 Additional comments (15)
packages/nextjs/src/components/layout.tsx (3)
11-13: LGTM!The type definition is clear and correctly combines
PropsWithChildrenwith the sidebar prop.
37-37: Review header height - may be too small.A header height of 30 pixels seems very small for content that includes a Burger menu, Image (21x21), Title, and Logout link. This might cause layout issues or cramped appearance.
Please verify that this height is sufficient for all the header content across different screen sizes.
51-61: Verify intentional use of<a>tag instead of Next.jsLink.The logout link uses a plain
<a>tag instead of Next.jsLink, which will trigger a full page reload rather than client-side navigation. While this might be intentional for logout (to ensure clean session clearing), it's worth confirming.If client-side navigation is preferred, consider using Next.js
Link:+ <Link + href="/logout" - <a - href="/logout" style={{ textDecoration: 'none', color: 'inherit', fontWeight: 500, fontSize: '0.95rem', }} > Logout + </Link> - </a>If the full page reload is intentional for security reasons, consider adding a comment to clarify this decision.
packages/nextjs/package.json (5)
12-12: Verify format script path aligns with Next.js structure.The
formatscript targetssrcdirectory, but Next.js typically usesapp/orsrc/app/depending on configuration. Confirm this path exists and contains the files to be formatted.
12-12: Verify format script path aligns with Next.js directory structure.The
formatscript targetssrcdirectory, but Next.js typically usesapp/orsrc/app/depending on configuration. Confirm this path exists and correctly covers all source files to be formatted.
22-22: ✓ Verified: axios version 1.12.2 is current.The specified version 1.12.2 is the latest release of axios, so no change is needed here.
22-22: No action required — axios 1.12.2 is valid and current.axios v1.12.2 is the latest published release as of October 23, 2025, and was released on September 14, 2025. The dependency specification is correct.
22-22: No action required—axios 1.12.2 is valid.Version 1.12.2 was published to npm on Sep 14, 2025 and is a legitimate release. The version number follows standard semantic versioning and is not unusually high.
Likely an incorrect or invalid review comment.
packages/nextjs/src/app/pages/create/page.tsx (1)
3-4: LGTM!Clean implementation - the component correctly renders
CreatePageFormfor top-level page creation.packages/nextjs/src/components/molecules/create-page-form.tsx (1)
15-70: LGTM!The form implementation is well-structured with proper validation, error handling via
useCudApi, and clear user feedback through loading and error states.packages/nextjs/src/lib/hooks/use-cud-api.ts (3)
1-3: LGTM!The imports are correctly structured and appropriate for the hook's functionality.
37-43: LGTM - clean API surface.The return object provides a clear and type-safe API for the hook consumers.
1-44: No vulnerabilities detected in axios 1.12.2.The codebase uses axios 1.12.2, which has been verified against current security advisories. The DoS vulnerability affecting "< 1.12.0" was patched in 1.12.0, meaning version 1.12.2 is outside all known HIGH severity vulnerability ranges. The version is current and secure.
pnpm-workspace.yaml (2)
4-4:minimumReleaseAgeis a valid pnpm configuration—no action required.The setting
minimumReleaseAgedefines the minimum number of minutes that must pass after a version is published before pnpm will install it. pnpm 10.16 added this feature to filter out versions of packages newer than a certain age, to combat supply chain attacks. Your configuration of 14400 minutes (4 hours) is a legitimate security measure. No changes needed.
6-7: Clarify why@types/nodeoverride specifies v24 whennextjspackage requests v22.The workspace override forces
@types/node@24.9.1globally, butpackages/nextjs/package.jsonspecifies^22.13.14. While Next.js 15 is compatible with both v22 and v24 types, this major version mismatch should be intentional.
- Confirm the override aligns with your project's Node.js runtime version (check CI/build environment, deployment target,
.nvmrc, orengines.node)- If v24 is correct (Node 24 runtime), update
packages/nextjs/package.jsonto^24.0.0for consistency- If v22 is correct, align the override to match
The patch bump (24.8.0 → 24.9.1) itself is safe.
| import { useRouter } from 'next/navigation'; | ||
| import { type PropsWithChildren, type ReactNode, useEffect } from 'react'; | ||
| import { useAuth } from '@/lib/auth/provider'; | ||
| import appIcon from '@/app/icons/favicon-32x32.png' |
There was a problem hiding this comment.
Add missing semicolon.
The import statement is missing a trailing semicolon, which is inconsistent with the other imports and may cause issues.
Apply this diff:
-import appIcon from '@/app/icons/favicon-32x32.png'
+import appIcon from '@/app/icons/favicon-32x32.png';📝 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.
| import appIcon from '@/app/icons/favicon-32x32.png' | |
| import appIcon from '@/app/icons/favicon-32x32.png'; |
🤖 Prompt for AI Agents
In packages/nextjs/src/components/layout.tsx around line 8, the import statement
"import appIcon from '@/app/icons/favicon-32x32.png'" is missing a trailing
semicolon; update that line to add a semicolon at the end of the import so it
matches project style and prevents potential lint/parse issues.
| if (!user) { | ||
| return undefined; // Will redirect to login | ||
| } |
There was a problem hiding this comment.
Return null instead of undefined.
Returning undefined from a React component is discouraged in React 19. Use null for explicit "no render" semantics.
Apply this diff:
if (!user) {
- return undefined; // Will redirect to login
+ return null; // Will redirect to login
}📝 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.
| if (!user) { | |
| return undefined; // Will redirect to login | |
| } | |
| if (!user) { | |
| return null; // Will redirect to login | |
| } |
🤖 Prompt for AI Agents
In packages/nextjs/src/components/layout.tsx around lines 30 to 32, the
component currently returns undefined when no user is present; update that
return to null so the component explicitly renders nothing (React 19 recommends
returning null for "no render" semantics). Replace "return undefined;" with
"return null;" and run a quick typecheck to ensure no TS complaints.
| if (result.status === 200 && 'data' in result.data) { | ||
| return result.data.data; | ||
| } | ||
|
|
||
| // Endpoints that return a 204/201/200 with no content will return null | ||
| return null as unknown as T; |
There was a problem hiding this comment.
Fix status code handling and unsafe type assertion.
Two critical issues:
-
Status code mismatch: Line 23 only checks for status 200, but the comment on line 27 mentions "204/201/200". This means:
- POST requests returning 201 (Created) with a response body will incorrectly return
null - DELETE requests returning 204 (No Content) will correctly return
null
- POST requests returning 201 (Created) with a response body will incorrectly return
-
Unsafe type assertion:
return null as unknown as Ton line 28 bypasses TypeScript's type safety. If callers expect a non-nullableT, this will cause runtime errors.
Apply this diff to fix both issues:
- if (result.status === 200 && 'data' in result.data) {
+ if ((result.status === 200 || result.status === 201) && 'data' in result.data) {
return result.data.data;
}
- // Endpoints that return a 204/201/200 with no content will return null
- return null as unknown as T;
+ // Endpoints that return a 204 (No Content) or have no data will return null
+ return null as T;Additionally, consider updating the return type to be explicit about nullability:
): Promise<T | null> {Then update the wrapper functions on lines 39-41 to reflect this change.
🤖 Prompt for AI Agents
In packages/nextjs/src/lib/hooks/use-cud-api.ts around lines 23 to 28, the code
only treats status 200 as a successful body response and uses an unsafe "null as
unknown as T" assertion; change the status check to accept 200 and 201 (e.g.
result.status === 200 || result.status === 201) and only return result.data.data
when a body exists, return null otherwise without casting through unknown; also
update the function signature to Promise<T | null> and adjust the wrapper
functions referenced on lines 39-41 to expect and propagate T | null instead of
T so callers handle nullable responses safely.
| } catch (error) { | ||
| setError(error instanceof Error ? error.message : 'Unknown error'); | ||
| throw error; | ||
| } finally { | ||
| setInProgress(false); | ||
| } |
There was a problem hiding this comment.
Add cleanup to prevent state updates on unmounted components.
The finally block (line 33) calls setInProgress(false) which will trigger a React warning if the component unmounts before the request completes. This is a common issue with async operations in React hooks.
Consider using an AbortController or a mounted flag:
export const useCudApi = () => {
const [inProgress, setInProgress] = useState<boolean>(false);
const [error, setError] = useState<string | null>(null);
+ const mountedRef = useRef(true);
+
+ useEffect(() => {
+ return () => {
+ mountedRef.current = false;
+ };
+ }, []);
async function request<T = unknown, D = unknown>(
method: 'post' | 'patch' | 'delete',
path: string,
data?: D
): Promise<T> {
setInProgress(true);
setError(null);
try {
// ... request logic
} catch (error) {
- setError(error instanceof Error ? error.message : 'Unknown error');
+ if (mountedRef.current) {
+ setError(error instanceof Error ? error.message : 'Unknown error');
+ }
throw error;
} finally {
- setInProgress(false);
+ if (mountedRef.current) {
+ setInProgress(false);
+ }
}
}Don't forget to import useRef and useEffect from React.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/nextjs/src/lib/hooks/use-cud-api.ts around lines 29 to 34, the
finally block unconditionally calls setInProgress(false) which can update state
after the component has unmounted; add a mounted flag (useRef) or use an
AbortController and check it before calling state setters and before rethrowing:
create a ref like isMountedRef initialized to true, set it to false in a
useEffect cleanup, replace direct setError/setInProgress calls with guarded
calls that only run when isMountedRef.current is true (or abort the async
operation via AbortController and skip state updates when aborted), and add the
required imports for useRef and useEffect.
There was a problem hiding this comment.
Actionable comments posted: 14
♻️ Duplicate comments (11)
src/lib/store/fetcher.ts (1)
4-6: Fragile URL construction with keys.join('').The past review comment correctly identifies that joining keys without delimiters risks malformed paths and ignores query parameter objects. Consider implementing the suggested fix to handle the first key as the base path and merge remaining keys as query parameters.
src/lib/auth/provider.tsx (1)
20-41: WraprefreshSessioninuseCallbackto prevent React Hook warnings.The
refreshSessionfunction is called in theuseEffectbut not included in its dependency array, which will trigger React Hook exhaustive-deps warnings.src/lib/hooks/use-cud-api.ts (2)
23-28: Fix status code handling and unsafe type assertion.The code only checks for status 200, but POST requests typically return 201 (Created). Additionally,
null as unknown as Tbypasses TypeScript's type safety.
29-34: Add cleanup to prevent state updates on unmounted components.The
finallyblock callssetInProgress(false)which can trigger React warnings if the component unmounts before the request completes.src/components/molecules/tree-node.tsx (1)
26-35: Computed store created on every render.Line 27 creates a new
computedstore on every render, causing unnecessary re-computations and potential memory issues with nanostores.src/app/(auth)/logout/page.tsx (1)
12-15: Await signOut before navigation to prevent race condition.The
signOut()call is not awaited beforerouter.push('/')executes, creating a race condition where navigation may occur before sign-out completes. This issue was previously identified in an earlier review.Apply this diff to properly await the sign-out:
useEffect(() => { - signOut(); - router.push('/'); + const handleLogout = async () => { + await signOut(); + router.push('/'); + }; + handleLogout(); }, [router, signOut]);src/components/layout.tsx (1)
29-31: Returnnull, notundefined, when not rendering.React expects
nullfor “render nothing”; keep behavior explicit.- if (!user) { - return undefined; // Will redirect to login - } + if (!user) { + return null; // Will redirect to login + }src/app/pages/[id]/page.tsx (1)
17-24: Inconsistent null vs empty string handling.Line 21 sets
$currentPageIdto an empty string whenpageIdis falsy, while the cleanup (Line 23) sets it tonull. This inconsistency could cause confusion.src/app/api/v1/pages/[id]/route.ts (1)
18-20: Generic error throws result in 500 instead of 404.Throwing a generic
Errorwhen a page is not found results in a 500 status code. The wrapper should support typed errors (e.g.,ApiError) to return proper HTTP status codes like 404.src/app/api/v1/pages/route.ts (1)
21-23: Generic errors mapped to 500 instead of proper status codes.Throwing generic
Errorinstances results in 500 responses. Domain errors like "Workspace not found" (404), "Parent page not found" (404), and workspace mismatch (400/403) should return appropriate HTTP status codes.src/lib/api/route-wrapper.ts (1)
96-118: Missing typed error handling for proper HTTP status codes.The wrapper maps all errors (except session errors) to 500. Domain errors like "not found" or "invalid input" should return appropriate status codes (404, 400, etc.). An
ApiErrorclass and Response passthrough would enable this.
🧹 Nitpick comments (17)
src/lib/store/tree-expanded-state.ts (1)
6-14: Consider extracting shared mutation logic.The mutation functions are correct and properly maintain immutability. However,
expandPage,collapsePage, and parts oftogglePageExpandedshare a common pattern that could be extracted to reduce duplication.Consider introducing a private helper:
+const updatePageState = (pageId: string, state: boolean) => { + const current = $expandedPages.get(); + const newMap = new Map(current); + newMap.set(pageId, state); + $expandedPages.set(newMap); +}; + export const togglePageExpanded = (pageId: string) => { const current = $expandedPages.get(); - const newMap = new Map(current); - - const isCurrentlyExpanded = newMap.get(pageId) ?? false; - newMap.set(pageId, !isCurrentlyExpanded); - - $expandedPages.set(newMap); + const isCurrentlyExpanded = current.get(pageId) ?? false; + updatePageState(pageId, !isCurrentlyExpanded); }; export const expandPage = (pageId: string) => { - const current = $expandedPages.get(); - const newMap = new Map(current); - newMap.set(pageId, true); - $expandedPages.set(newMap); + updatePageState(pageId, true); }; export const collapsePage = (pageId: string) => { - const current = $expandedPages.get(); - const newMap = new Map(current); - newMap.set(pageId, false); - $expandedPages.set(newMap); + updatePageState(pageId, false); };Also applies to: 20-32
src/components/atoms/tree-toggle.tsx (1)
11-13: Consider using CSS for the spacer.The inline spacer div works but could be replaced with CSS margin or padding on the parent container for cleaner markup.
src/components/atoms/tree-item.tsx (1)
28-44: Consider using Mantine's styling system for hover states.The current approach of manipulating inline styles via
onMouseEnter/onMouseLeavehandlers can cause issues with SSR hydration and doesn't scale well. Mantine provides better alternatives through itssxprop or style objects that handle pseudo-selectors.Example using Mantine's styling approach:
<Text size="sm" sx={{ cursor: 'pointer', display: 'flex', alignItems: 'center', gap: 8, padding: '4px 8px', borderRadius: 4, transition: 'background-color 0.1s', '&:hover': { backgroundColor: 'var(--mantine-color-gray-1)', }, }} > {content} </Text>This eliminates the need for manual event handlers and works correctly with SSR.
src/lib/auth/config.ts (1)
28-28: Remove empty hooks object.The empty
hooks: {}object serves no purpose and can be removed for cleaner configuration.Apply this diff:
trustedOrigins: environment.NODE_ENV === 'development' ? ['http://localhost:3000'] : [], secret: environment.BETTER_AUTH_SECRET, - hooks: {}, databaseHooks: {src/components/molecules/sidebar/logged-in-container.tsx (1)
9-22: LGTM with a minor suggestion for defensive coding.The component correctly subscribes to the store and handles loading state. The
PagesTreecomponent already handles empty branches gracefully (see src/components/molecules/pages-tree.tsx lines 8-24).For extra safety, you could add optional chaining when passing branches:
- {!loading && rootPagesTree && <PagesTree branches={rootPagesTree.branches} />} + {!loading && rootPagesTree && <PagesTree branches={rootPagesTree?.branches ?? []} />}src/lib/api/client.ts (1)
13-16: Consider simplifying parameter handling.The conditional params object can be simplified since Axios automatically omits undefined values.
Apply this diff to simplify:
- getTree: (parentId?: string) => - apiClient.get('/pages/tree', { - params: parentId ? { parentId } : {}, - }), + getTree: (parentId?: string) => + apiClient.get('/pages/tree', { + params: { parentId } + }),eslint.config.mjs (1)
31-40: Remove duplicate ignore patterns.The ignore list contains redundant entries for
node_modulesand.nextdirectories.Apply this diff to clean up:
{ ignores: [ - 'node_modules/**', - '.next/**', 'out/**', 'build/**', 'next-env.d.ts', '**/.next/**', '**/node_modules/**', 'commitlint.config.js', ], },src/app/page.tsx (1)
1-1: Consider removing 'use client' directive.This page has no interactive elements or client-side state, so it could be a server component. Server components improve performance and reduce client bundle size.
Unless you plan to add interactivity, remove the 'use client' directive:
-'use client'; - import { Container, Stack, Title } from '@mantine/core';src/lib/logger.ts (1)
12-16: Consider adding file transport for production logging.Currently, logs are only written to the console. In production environments, console logs may be ephemeral or difficult to access. Consider adding a file transport or integrating with a cloud logging service to ensure log persistence and searchability.
Example file transport addition:
transports: [ new winston.transports.Console({ format: environment.NODE_ENV === 'development' ? winston.format.combine(winston.format.colorize(), winston.format.simple()) : winston.format.json(), }), ...(environment.NODE_ENV === 'production' ? [ new winston.transports.File({ filename: 'logs/error.log', level: 'error' }), new winston.transports.File({ filename: 'logs/combined.log' }), ] : []), ],src/app/(auth)/login/page.tsx (1)
21-32: Consider adding error handling and loading state.The sign-in button lacks error handling and loading feedback. Users won't know if the authentication request fails or is in progress, which could lead to confusion or repeated clicks.
Consider adding a try-catch block with error notifications and a loading state to improve UX.
Apply this diff to add error handling and loading state:
export default function LoginPage() { + const [loading, setLoading] = useState(false); + const [error, setError] = useState<string | null>(null); + + const handleSignIn = async () => { + try { + setLoading(true); + setError(null); + await authClient.signIn.social({ + provider: 'oidc', + callbackURL: `${globalThis.location.origin}/`, + }); + } catch (err) { + setError(err instanceof Error ? err.message : 'Sign in failed'); + } finally { + setLoading(false); + } + }; + return ( <Center style={{ minHeight: '100vh' }}> <Container size="xs" w="100%"> <Paper shadow="md" p="xl" radius="md" withBorder> <Stack gap="lg" align="center"> <div style={{ textAlign: 'center' }}> <Title order={2} c="var(--mantine-color-blue-6)"> Welcome to Thoth </Title> <Text c="dimmed" size="sm" mt="xs"> Sign in to access your account </Text> </div> + {error && ( + <Text c="red" size="sm"> + {error} + </Text> + )} + <Button size="md" fullWidth - onClick={() => - authClient.signIn.social({ - provider: 'oidc', - callbackURL: `${globalThis.location.origin}/`, - }) - } + onClick={handleSignIn} + loading={loading} > Sign In </Button>Add the useState import:
-import { Button, Center, Container, Paper, Stack, Text, Title } from '@mantine/core'; +import { useState } from 'react'; +import { Button, Center, Container, Paper, Stack, Text, Title } from '@mantine/core';src/app/api/v1/pages/tree/route.ts (2)
30-35: Sort children for stable ordering.Children aren’t sorted; add
lastUpdated descto keep parity with parents.- ? await containerRepository.getByQuery( - addUserIdToQuery(containerRepository.createQuery(), session.user.id) - .eq('type', 'page') - .in('parentId', parentIds) - ) + ? await containerRepository.getByQuery( + addUserIdToQuery(containerRepository.createQuery(), session.user.id) + .eq('type', 'page') + .in('parentId', parentIds) + .sort('lastUpdated', 'desc') + )
22-26: Root fetch could be large; consider DB-side null handling or a cap.Client-side filtering of roots (
!container.parentId) means fetching all user containers. If SuperSave can supportisNull('parentId')or similar in future, switch to that; else add a sanelimitor pagination for root requests.src/lib/database/index.ts (1)
6-18: Guard DB initialization with a promise and persist across HMR.Avoid duplicate connections under concurrency/HMR by memoizing the creation.
-let database: SuperSave; +let database: SuperSave; +let databasePromise: Promise<SuperSave> | undefined; + +// Persist across dev HMR +const globalForDb = globalThis as unknown as { __supersave_db__?: Promise<SuperSave> }; +if (!databasePromise) { + databasePromise = globalForDb.__supersave_db__; +} export async function getDatabase() { - if (database) { - return database; - } - - database = await SuperSave.create(environment.DB); - - await database.addEntity(entities.Container); - await database.addEntity(entities.Workspace); - return database; + if (database) return database; + if (!databasePromise) { + databasePromise = (async () => { + const db = await SuperSave.create(environment.DB); + await db.addEntity(entities.Container); + await db.addEntity(entities.Workspace); + return db; + })(); + if (process.env.NODE_ENV !== 'production') { + globalForDb.__supersave_db__ = databasePromise; + } + } + database = await databasePromise; + return database; }Also applies to: 20-28
src/components/molecules/create-page-form.tsx (1)
23-24: Apply trim validation; response shape confirmed as plain page object.The POST handler returns the page object directly (not wrapped), so
router.push(/pages/${page.id})is correct. The trim validation is valid and should be applied—the current validation only checks.length === 0and allows whitespace-only names like " ". The try-catch is optional but adds defensive error handling.- validate: { - name: (value) => (value.length === 0 ? 'Page name is required' : null), - }, + validate: { + name: (value) => (value.trim().length === 0 ? 'Page name is required' : null), + },Also apply trim to submission to prevent whitespace padding:
const handleSubmit = async (values: typeof form.values) => { const page = await post<CreatePageResponse, CreatePageBody>('/pages', { - name: values.name, + name: values.name.trim(), emoji: null, - parentId: parentId, + parentId, }); };src/app/pages/[id]/page.tsx (2)
64-65: Remove redundant optional chaining and fallback.After the null check at Line 52,
pageis guaranteed to exist. The optional chainingpage?.page.emojiand the<Loader />fallback forpage?.page.nameare unnecessary.Apply this diff:
- <Text size="xl">{page?.page.emoji}</Text> - <Title order={1}>{page?.page.name ?? <Loader />}</Title> + <Text size="xl">{page.page.emoji}</Text> + <Title order={1}>{page.page.name}</Title>
72-86: Remove redundant optional chaining throughout metadata rendering.After the null check at Line 52,
pageis guaranteed to exist. The optional chaining on Lines 72, 76, 80, and 82 is unnecessary.Apply this diff:
- <Text>{page?.page.type}</Text> + <Text>{page.page.type}</Text> </Group> <Group gap="xs"> <Badge variant="light">Created</Badge> - <Text>{page?.page.createdAt ? new Date(page.page.createdAt).toLocaleString() : ''}</Text> + <Text>{page.page.createdAt ? new Date(page.page.createdAt).toLocaleString() : ''}</Text> </Group> <Group gap="xs"> <Badge variant="light">Last Updated</Badge> - <Text>{page?.page.lastUpdated ? new Date(page.page.lastUpdated).toLocaleString() : ''}</Text> + <Text>{page.page.lastUpdated ? new Date(page.page.lastUpdated).toLocaleString() : ''}</Text> </Group> - {page?.page.parentId && ( + {page.page.parentId && ( <Group gap="xs"> <Badge variant="light">Parent</Badge> <Text>{page.page.parentId}</Text>src/app/api/v1/pages/route.ts (1)
12-14: Redundant body check.The
expectedBodySchemavalidation at Line 9 ensures thatbodyis present and valid. This explicit check is unnecessary.Apply this diff:
async ({ body }, session) => { - if (!body) { - throw new Error('Body is required'); - } - const workspaceRepository = await getWorkspaceRepository();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (26)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/icons/android-icon-144x144.pngis excluded by!**/*.pngpublic/icons/android-icon-192x192.pngis excluded by!**/*.pngpublic/icons/android-icon-36x36.pngis excluded by!**/*.pngpublic/icons/android-icon-48x48.pngis excluded by!**/*.pngpublic/icons/android-icon-72x72.pngis excluded by!**/*.pngpublic/icons/android-icon-96x96.pngis excluded by!**/*.pngpublic/icons/apple-icon-114x114.pngis excluded by!**/*.pngpublic/icons/apple-icon-120x120.pngis excluded by!**/*.pngpublic/icons/apple-icon-144x144.pngis excluded by!**/*.pngpublic/icons/apple-icon-152x152.pngis excluded by!**/*.pngpublic/icons/apple-icon-180x180.pngis excluded by!**/*.pngpublic/icons/apple-icon-57x57.pngis excluded by!**/*.pngpublic/icons/apple-icon-60x60.pngis excluded by!**/*.pngpublic/icons/apple-icon-72x72.pngis excluded by!**/*.pngpublic/icons/apple-icon-76x76.pngis excluded by!**/*.pngpublic/icons/apple-icon-precomposed.pngis excluded by!**/*.pngpublic/icons/apple-icon.pngis excluded by!**/*.pngpublic/icons/favicon-16x16.pngis excluded by!**/*.pngpublic/icons/favicon-32x32.pngis excluded by!**/*.pngpublic/icons/favicon-96x96.pngis excluded by!**/*.pngpublic/icons/ms-icon-144x144.pngis excluded by!**/*.pngpublic/icons/ms-icon-150x150.pngis excluded by!**/*.pngpublic/icons/ms-icon-310x310.pngis excluded by!**/*.pngpublic/icons/ms-icon-70x70.pngis excluded by!**/*.pngsrc/app/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (62)
.gitignore(1 hunks).prettierrc(1 hunks).vscode/extensions.json(1 hunks)eslint.config.mjs(1 hunks)next.config.ts(1 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)src/app/(auth)/layout.tsx(1 hunks)src/app/(auth)/login/page.tsx(1 hunks)src/app/(auth)/logout/page.tsx(1 hunks)src/app/@sidebar/default.tsx(1 hunks)src/app/api/auth/[...auth]/route.ts(1 hunks)src/app/api/v1/pages/[id]/route.ts(1 hunks)src/app/api/v1/pages/route.ts(1 hunks)src/app/api/v1/pages/tree/route.ts(1 hunks)src/app/globals.css(1 hunks)src/app/layout-client.tsx(1 hunks)src/app/layout.tsx(1 hunks)src/app/manifest.json(1 hunks)src/app/page.tsx(1 hunks)src/app/pages/[id]/create/page.tsx(1 hunks)src/app/pages/[id]/page.tsx(1 hunks)src/app/pages/create/page.tsx(1 hunks)src/components/atoms/tree-item.tsx(1 hunks)src/components/atoms/tree-toggle.tsx(1 hunks)src/components/layout.tsx(1 hunks)src/components/molecules/create-page-form.tsx(1 hunks)src/components/molecules/pages-tree.tsx(1 hunks)src/components/molecules/sidebar/logged-in-container.tsx(1 hunks)src/components/molecules/tree-node.tsx(1 hunks)src/lib/api/client.ts(1 hunks)src/lib/api/route-wrapper.ts(1 hunks)src/lib/auth/client.ts(1 hunks)src/lib/auth/config.ts(1 hunks)src/lib/auth/provider.tsx(1 hunks)src/lib/auth/session.ts(1 hunks)src/lib/database/entities/container.ts(1 hunks)src/lib/database/entities/index.ts(1 hunks)src/lib/database/entities/workspace.ts(1 hunks)src/lib/database/helpers.ts(1 hunks)src/lib/database/index.ts(1 hunks)src/lib/environment.ts(1 hunks)src/lib/hooks/use-cud-api.ts(1 hunks)src/lib/logger.ts(1 hunks)src/lib/store/fetcher.ts(1 hunks)src/lib/store/query/get-page-details.ts(1 hunks)src/lib/store/query/get-pages-tree.ts(1 hunks)src/lib/store/tree-expanded-state.ts(1 hunks)src/lib/theme.ts(1 hunks)src/types/api/endpoints/create-page.ts(2 hunks)src/types/api/endpoints/get-page-details.ts(1 hunks)src/types/api/endpoints/get-pages-tree.ts(1 hunks)src/types/api/endpoints/index.ts(1 hunks)src/types/api/entities.ts(1 hunks)src/types/api/index.ts(1 hunks)src/types/api/utilities.ts(1 hunks)src/types/database/index.ts(2 hunks)src/types/schemas/entities/container.ts(1 hunks)src/types/schemas/entities/index.ts(1 hunks)src/types/schemas/entities/workspace.ts(1 hunks)src/types/schemas/utilities.ts(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/app/globals.css
- .vscode/extensions.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .prettierrc
🧰 Additional context used
🧬 Code graph analysis (34)
src/app/(auth)/layout.tsx (1)
src/components/layout.tsx (1)
Layout(14-71)
src/lib/logger.ts (1)
src/lib/environment.ts (1)
environment(3-15)
src/app/@sidebar/default.tsx (1)
src/components/molecules/sidebar/logged-in-container.tsx (1)
LoggedInContainer(9-24)
src/types/schemas/entities/workspace.ts (2)
src/types/database/index.ts (1)
workspaceSchema(20-20)src/types/schemas/utilities.ts (3)
withTrackUpdatesSchema(11-14)withUserIdSchema(16-18)withIdSchema(3-5)
src/app/api/auth/[...auth]/route.ts (1)
src/lib/auth/config.ts (1)
auth(10-56)
src/components/molecules/tree-node.tsx (3)
src/lib/store/tree-expanded-state.ts (2)
$expandedPages(3-3)togglePageExpanded(6-14)src/components/atoms/tree-toggle.tsx (1)
TreeToggle(10-20)src/components/atoms/tree-item.tsx (1)
TreeItem(11-79)
src/components/molecules/pages-tree.tsx (2)
src/types/api/endpoints/get-pages-tree.ts (1)
GetPagesTreeResponse(23-23)src/components/molecules/tree-node.tsx (1)
TreeNode(26-83)
src/lib/auth/config.ts (3)
src/lib/database/index.ts (3)
getDatabase(8-18)getWorkspaceRepository(25-28)getContainerRepository(20-23)src/lib/environment.ts (1)
environment(3-15)src/types/database/index.ts (2)
WorkspaceCreate(25-25)ContainerCreate(16-16)
src/app/(auth)/login/page.tsx (1)
src/lib/auth/client.ts (1)
authClient(4-6)
src/lib/database/index.ts (5)
src/lib/environment.ts (1)
environment(3-15)src/lib/database/entities/container.ts (1)
Container(5-18)src/lib/database/entities/index.ts (2)
Container(1-1)Workspace(2-2)src/types/database/index.ts (2)
Container(15-15)Workspace(24-24)src/lib/database/entities/workspace.ts (1)
Workspace(5-15)
src/components/molecules/create-page-form.tsx (2)
src/lib/hooks/use-cud-api.ts (1)
useCudApi(5-44)src/types/api/endpoints/create-page.ts (2)
CreatePageResponse(12-12)CreatePageBody(22-22)
src/app/api/v1/pages/tree/route.ts (5)
src/app/api/v1/pages/[id]/route.ts (1)
GET(7-34)src/lib/api/route-wrapper.ts (1)
apiRoute(14-119)src/types/api/endpoints/get-pages-tree.ts (3)
GetPagesTreeResponse(23-23)GetPagesTreeQueryVariables(29-29)getPagesTreeQueryVariablesSchema(26-28)src/lib/database/index.ts (1)
getContainerRepository(20-23)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)
src/lib/database/entities/workspace.ts (3)
src/lib/database/entities/container.ts (1)
NAME(3-3)src/lib/database/entities/index.ts (3)
NAME(1-1)NAME(2-2)Workspace(2-2)src/types/database/index.ts (1)
Workspace(24-24)
src/types/schemas/entities/container.ts (2)
src/types/database/index.ts (2)
containerSchema(13-13)pageContainerSchema(6-6)src/types/schemas/utilities.ts (5)
withTrackUpdatesSchema(11-14)withWorkspaceIdSchema(20-22)withUserIdSchema(16-18)withIdSchema(3-5)withParentIdSchema(7-9)
src/lib/store/query/get-pages-tree.ts (2)
src/lib/store/fetcher.ts (1)
createFetcherStore(4-6)src/types/api/endpoints/get-pages-tree.ts (2)
GetPagesTreeResponse(23-23)GET_PAGES_TREE_ENDPOINT(6-6)
src/app/pages/create/page.tsx (1)
src/components/molecules/create-page-form.tsx (1)
CreatePageForm(14-62)
src/lib/store/fetcher.ts (1)
src/lib/api/client.ts (1)
apiClient(3-8)
src/lib/database/entities/container.ts (3)
src/lib/database/entities/index.ts (3)
NAME(1-1)NAME(2-2)Container(1-1)src/lib/database/entities/workspace.ts (1)
NAME(3-3)src/types/database/index.ts (1)
Container(15-15)
src/app/pages/[id]/create/page.tsx (1)
src/components/molecules/create-page-form.tsx (1)
CreatePageForm(14-62)
src/lib/store/query/get-page-details.ts (2)
src/lib/store/fetcher.ts (1)
createFetcherStore(4-6)src/types/api/endpoints/get-page-details.ts (1)
GetPageDetailsResponse(11-11)
src/types/api/endpoints/get-page-details.ts (2)
src/types/api/entities.ts (1)
pageSchema(3-6)src/types/api/utilities.ts (1)
DataWrapper(2-4)
src/components/molecules/sidebar/logged-in-container.tsx (2)
src/lib/store/query/get-pages-tree.ts (1)
$rootPagesTree(4-4)src/components/molecules/pages-tree.tsx (1)
PagesTree(9-25)
src/app/api/v1/pages/route.ts (4)
src/lib/api/route-wrapper.ts (1)
apiRoute(14-119)src/types/api/endpoints/create-page.ts (3)
CreatePageResponse(12-12)CreatePageBody(22-22)createPageBodySchema(16-20)src/lib/database/index.ts (2)
getWorkspaceRepository(25-28)getContainerRepository(20-23)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)
src/app/pages/[id]/page.tsx (2)
src/lib/auth/provider.tsx (1)
useAuth(55-61)src/lib/store/query/get-page-details.ts (2)
$currentPage(7-7)$currentPageId(5-5)
src/types/api/endpoints/get-pages-tree.ts (3)
src/types/api/entities.ts (1)
pageSchema(3-6)src/types/schemas/utilities.ts (1)
withIdSchema(3-5)src/types/api/utilities.ts (1)
DataWrapper(2-4)
src/types/api/entities.ts (1)
src/types/schemas/entities/container.ts (1)
pageContainerSchema(19-24)
src/app/layout.tsx (3)
src/lib/auth/config.ts (1)
auth(10-56)src/app/layout-client.tsx (1)
RootClientLayout(13-36)src/components/layout.tsx (1)
Layout(14-71)
src/app/(auth)/logout/page.tsx (1)
src/lib/auth/provider.tsx (1)
useAuth(55-61)
src/app/api/v1/pages/[id]/route.ts (5)
src/app/api/v1/pages/tree/route.ts (1)
GET(7-64)src/lib/api/route-wrapper.ts (1)
apiRoute(14-119)src/types/api/endpoints/get-page-details.ts (3)
GetPageDetailsResponse(11-11)GetPageDetailsParameters(17-17)getPageDetailsParametersSchema(14-16)src/lib/database/index.ts (1)
getContainerRepository(20-23)src/lib/database/helpers.ts (1)
addUserIdToQuery(3-5)
src/app/layout-client.tsx (2)
src/lib/theme.ts (1)
theme(3-5)src/lib/auth/provider.tsx (1)
AuthProvider(16-53)
src/lib/auth/session.ts (1)
src/lib/auth/config.ts (1)
auth(10-56)
src/components/layout.tsx (1)
src/lib/auth/provider.tsx (1)
useAuth(55-61)
src/lib/api/route-wrapper.ts (2)
src/lib/auth/session.ts (1)
getSession(4-15)src/lib/logger.ts (1)
logger(4-17)
src/lib/auth/provider.tsx (1)
src/lib/auth/client.ts (1)
authClient(4-6)
🔇 Additional comments (46)
tsconfig.json (4)
8-8: Question:noEmitwithincrementalcompilation.Setting both
noEmit: trueandincremental: trueis unusual. Incremental compilation is typically beneficial when emitting output to cache build artifacts. WithnoEmit: true, Next.js handles all transpilation via SWC, so incremental caching may not provide meaningful benefit.Verify this is intentional or consider removing
incrementalto simplify the configuration.Also applies to: 15-15
21-23: Verify the path alias matches your project structure.The
@/*alias maps to./src/*, which assumes asrcdirectory as the root for application source code. Since this PR consolidates multiple packages into a Next.js monorepo, confirm:
- A
src/directory exists at the project root- If this is a multi-package setup, whether each package has its own
tsconfig.jsonor if they all share this one- Whether the path alias is correct for all packages (e.g., if
packages/webhas its own structure, this alias may not work as expected)
16-20: Verify the Next.js plugin is necessary.The
"plugins": [{ "name": "next" }]entry is included incompilerOptions. In recent versions of Next.js (v13+), the framework automatically configures this plugin when needed. Confirm this explicit declaration is not redundant or conflicting with Next.js's automatic setup.
3-14: TypeScript compiler options look well-aligned with Next.js.The configuration appropriately targets ES2022, includes DOM types alongside ESNext, enables strict mode, uses SWC-compatible
jsx: "preserve", and setsmoduleResolution: "bundler"as recommended for modern tooling.src/lib/database/helpers.ts (1)
1-9: LGTM! Formatting change aligns with tooling migration.The quote style change from double to single quotes is consistent with the PR's objective to consolidate tooling from Biome to ESLint/Prettier. No functional impact.
pnpm-workspace.yaml (2)
1-1: Clarify the purpose ofminimumReleaseAgein the context of this migration.The addition of
minimumReleaseAge: 1440(24 hours) is a pnpm release/publish-related setting. Given that this PR consolidates to a single Next.js app, please clarify:
- Does the project still publish packages, or is this a leftover configuration that should be removed?
- If this is being introduced now, what is the rationale relative to the Next.js migration?
This will help determine if this is an accidental change or intentional.
1-2: Monorepo consolidation in pnpm-workspace.yaml is complete and correct.The verification confirms the changes are consistent with the stated migration objective. The
packages/*glob has been properly removed (nopackages/directory remains), the rootpackage.jsoncontains no workspace configuration, and there are no orphaned references to the old monorepo structure anywhere in the codebase. The addition ofminimumReleaseAge: 1440is a valid pnpm configuration setting (24-hour minimum release age).src/lib/store/tree-expanded-state.ts (2)
1-3: LGTM: Clean atom setup.The atom declaration is clear and follows nanostore conventions. The
Map<string, boolean>type is appropriate for tracking page expansion state.
16-18: LGTM: Clean getter implementation.The function correctly returns the expanded state with an appropriate default value for missing entries.
src/components/atoms/tree-toggle.tsx (2)
1-2: LGTM!The imports are correctly structured and appropriate for the component's functionality.
4-8: LGTM!The interface is well-defined with appropriate types for all properties.
src/components/atoms/tree-item.tsx (1)
11-17: LGTM!The content setup correctly handles the optional emoji with a sensible default and follows DRY principles by being reusable in both rendering paths.
next.config.ts (1)
1-7: LGTM!The configuration correctly marks
mysql2as an external package, which is necessary since it's used in the auth config (line 11 of src/lib/auth/config.ts) and cannot be bundled by webpack..gitignore (1)
1-41: LGTM!Standard Next.js ignore patterns are properly configured, including dependencies, build artifacts, environment files, and TypeScript outputs.
src/lib/auth/config.ts (1)
8-8: Top-level await may delay app initialization.The top-level
await getDatabase()blocks the entire module from loading until the database connection is established. If the database is slow or unavailable, this will prevent the app from starting.Consider verifying startup behavior under degraded database conditions. If this causes issues, lazy-initialize the database on first auth request instead.
src/lib/auth/session.ts (1)
4-15: LGTM!The session helper correctly retrieves headers and throws when no session is found, which is appropriate for protecting server-side routes and components.
src/app/@sidebar/default.tsx (1)
1-5: LGTM! Clean parallel route implementation.The component correctly implements a Next.js parallel route slot for the sidebar, cleanly wrapping the LoggedInContainer.
src/lib/environment.ts (1)
3-15: LGTM! Comprehensive environment validation.The environment configuration properly validates all required variables with appropriate types. The use of envalid ensures type safety and provides clear error messages for missing or invalid environment variables.
src/lib/auth/provider.tsx (1)
43-50: LGTM! Proper sign-out implementation.The sign-out logic correctly calls the auth client, clears the user state, and handles errors appropriately.
src/lib/api/client.ts (2)
3-8: LGTM! Proper Axios client configuration.The API client is correctly configured with appropriate baseURL and headers for Next.js API routes.
18-21: LGTM! Clean API method implementations.The
getDetailsandcreatemethods follow REST conventions and have appropriate signatures.eslint.config.mjs (1)
13-29: LGTM! Sensible ESLint configuration.The rule configuration appropriately balances strictness with pragmatism, disabling overly restrictive unicorn rules while maintaining good practices like kebab-case filenames.
src/components/molecules/tree-node.tsx (1)
37-82: LGTM! Well-structured tree rendering.The rendering logic is clean with proper:
- Level-based indentation
- Event handling (stopPropagation for the add button)
- Recursive child rendering with correct keys
- Accessibility attributes
src/components/molecules/pages-tree.tsx (1)
9-25: LGTM! Clean and correct implementation.The component properly handles:
- Empty state with a user-friendly message
- Correct mapping of branches to TreeNode components
- Proper key usage for list items
- Appropriate prop passing
package.json (1)
7-8: Verify Turbopack stability for production builds.Turbopack is still experimental in Next.js 15. Using it in the build script may lead to unexpected issues or instability in production deployments.
Consider removing
--turbopackfrom the build script until it reaches stable status, or verify that your deployment environment supports experimental features:- "build": "next build --turbopack", + "build": "next build",src/types/api/utilities.ts (1)
1-4: LGTM!Clean implementation of a standard API response wrapper pattern. The generic type provides type safety for wrapped data.
src/lib/theme.ts (1)
3-5: LGTM! Consider adding theme customizations.The theme configuration is properly set up. The placeholder comment indicates this is ready for future customizations when needed.
src/types/api/endpoints/create-page.ts (1)
1-22: LGTM!The import updates and formatting changes improve code consistency. The schema definitions remain semantically correct and properly typed.
src/app/api/auth/[...auth]/route.ts (1)
1-4: LGTM!Proper implementation of better-auth integration with Next.js. The catch-all route pattern correctly handles all authentication endpoints, and the configuration includes appropriate database hooks for user onboarding.
src/app/pages/create/page.tsx (1)
3-5: LGTM! Clean component composition.Good separation of concerns—the page component remains a server component while delegating client-side form logic to
CreatePageForm. This is the recommended Next.js pattern for keeping the component tree as server-rendered as possible.src/types/api/endpoints/index.ts (1)
1-3: LGTM!Clean barrel export pattern consolidating endpoint type definitions into a single public API surface.
src/types/schemas/entities/index.ts (1)
1-2: LGTM!Clean barrel export consolidating entity schema modules.
src/lib/database/entities/workspace.ts (1)
1-15: LGTM!The Workspace entity definition follows the standard supersave pattern with appropriate filter/sort fields for the workspace entity.
src/types/api/index.ts (1)
1-3: LGTM!Clean barrel export providing a unified API type surface for the application.
src/lib/database/entities/index.ts (1)
1-2: LGTM!Clean barrel export with appropriate aliases for NAME constants to avoid naming conflicts.
src/types/api/entities.ts (2)
1-1: LGTM!The import path update removes the
.jsextension, aligning with TypeScript/Next.js conventions and consistent with the broader migration in this PR.
3-6: LGTM!The schema definitions appropriately omit internal fields (
workspaceId,userId) from the public API surface, following best practices for API boundaries.src/lib/store/query/get-pages-tree.ts (1)
1-4: LGTM!The store creation is clean and properly typed. The use of the fetcher store pattern with typed responses follows good practices for state management.
src/types/schemas/entities/workspace.ts (1)
4-10: LGTM!The workspace schema definition is clean and follows good composition patterns by extending utility schemas. The required fields (name, id, userId, timestamps) are appropriately defined.
src/types/database/index.ts (1)
1-3: LGTM!The changes to use type-only imports and remove
.jsextensions from module paths are appropriate for the Next.js migration. These updates align with TypeScript and Next.js conventions.Also applies to: 6-6, 20-20
src/types/schemas/entities/container.ts (1)
10-24: LGTM!Both schema definitions are well-structured. The use of type literal
'page'for the page container provides good type safety, and the composition pattern with utility schemas promotes code reuse and consistency.src/app/layout.tsx (1)
11-24: LGTM!The session-based conditional rendering is implemented correctly. The layout appropriately renders unauthenticated pages without the sidebar/app shell, while authenticated users get the full layout with navigation. The use of
await headers()is correct for Next.js 15+.src/lib/database/entities/container.ts (1)
5-18: LGTM — entity metadata is coherent with filters/sorts used elsewhere.Fields cover name/timestamps/relations expected by queries.
src/types/api/endpoints/get-page-details.ts (1)
5-16: LGTM — endpoint constant, response/params schemas, and types are consistent.Aligned with usage in the
[id]route.src/types/schemas/utilities.ts (1)
11-14: The review comment is incorrect; no datetime validation was removed.The schema has never included
.datetime()validation in the tracked git history. The timestamps are guaranteed to be ISO 8601 formatted strings because they're always generated vianew Date().toISOString()at creation points (auth/config.ts and api/v1/pages/route.ts). The schema design intentionally validates at the application layer (where timestamps are created) rather than at deserialization, which is a valid pattern that ensures all data flowing through the system is already compliant.Likely an incorrect or invalid review comment.
src/types/api/endpoints/get-pages-tree.ts (1)
8-17: Review comment is incorrect. Schema is consistent for both parent and child pages.The child pages do include IDs in the schema. The
pageSchemaalready contains theidfield (inherited through the schema composition chain:containerSchema.extend(withIdSchema.shape)), so child pages defined aspageSchemaare identical to parent pages in terms of ID inclusion. The parent's.extend(withIdSchema.shape)is redundant but harmless. The API implementation confirms both parent and child pages returnidfields.Likely an incorrect or invalid review comment.
| "build": "next build --turbopack", | ||
| "start": "next start", | ||
| "lint": "concurrently \"pnpm:lint:*\"", | ||
| "lint:eslint": "eslint --ext .ts,.tsx", |
There was a problem hiding this comment.
Fix missing path in eslint command.
The lint:eslint script is missing a path argument, which may cause it to lint nothing or behave unpredictably.
Apply this diff to specify the source directory:
- "lint:eslint": "eslint --ext .ts,.tsx",
+ "lint:eslint": "eslint --ext .ts,.tsx src",📝 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.
| "lint:eslint": "eslint --ext .ts,.tsx", | |
| "lint:eslint": "eslint --ext .ts,.tsx src", |
🤖 Prompt for AI Agents
In package.json around line 11, the "lint:eslint" script lacks a target path so
ESLint may run on nothing; update the script to include the source directory
(for example change "eslint --ext .ts,.tsx" to "eslint --ext .ts,.tsx src") so
ESLint actually lints project files.
| "eslint-config-next": "15.5.6", | ||
| "eslint-plugin-react-hooks": "^7.0.0", | ||
| "eslint-plugin-unicorn": "^61.0.2", | ||
| "express": "^5.1.0", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused Express dependency.
Express is listed in devDependencies but shouldn't be needed in a Next.js application, as Next.js provides its own server. This appears to be a leftover from the Express backend that was removed during the migration.
Remove the unused dependency:
- "express": "^5.1.0",Verify that no code references Express:
#!/bin/bash
# Search for any Express imports or usage in the codebase
rg -n "from ['\"]express['\"]" --type ts --type tsx
rg -n "require\(['\"]express['\"]\)" --type ts --type tsx🤖 Prompt for AI Agents
In package.json around line 52, the express dependency is unused and should be
removed; delete the "express": "^5.1.0" entry from package.json, then run
dependency cleanup (npm/yarn/pnpm) to update lockfile and node_modules, and
finally verify there are no imports/usages of express across the codebase (e.g.,
search for "from 'express'", "require('express')" and relevant TS/JS files)
before committing the change.
| export default function Layout({ children }: PropsWithChildren) { | ||
| return <Container>{children}</Container>; | ||
| } |
There was a problem hiding this comment.
Misleading folder name: (auth) layout provides no authentication.
This layout is in the (auth) route group but only wraps children in a Container without any authentication logic or route protection. This creates confusion about which routes are actually protected.
Based on the relevant snippet from src/components/layout.tsx, authentication checks exist elsewhere. The (auth) folder naming convention in Next.js typically indicates routes that are public auth pages (login, signup) or routes requiring authentication, but this layout doesn't enforce either.
Consider one of these approaches:
- If this is meant for public auth pages (login/logout), rename to
(public)or(unprotected) - If routes under this group should require authentication, add session validation
- If this is just a styling wrapper, move it out of a route group and rename appropriately
Which pattern were you intending to implement?
🤖 Prompt for AI Agents
In src/app/(auth)/layout.tsx lines 4-6, the layout only wraps children in a
Container but the (auth) route group name implies authentication behavior;
either rename or implement auth: choose one approach and apply the corresponding
change — if these routes are meant to be public auth pages, rename the folder to
(public) or (unprotected) and update any imports/route references; if routes
under this group should require authentication, add session validation in this
layout (import your session/auth utilities, check session/server-side auth, and
redirect to sign-in if unauthenticated) and keep the Container wrapper; if this
file is only a styling wrapper, move the component out of the route group into
src/components (rename to e.g., AuthContainer or PageContainer) and update all
route/layout usages accordingly.
| const databaseQuery = addUserIdToQuery(containerRepository.createQuery(), session.user.id).sort( | ||
| 'lastUpdated', | ||
| 'desc' | ||
| ); |
There was a problem hiding this comment.
Filter by type instead of casting; avoid mismatched payloads.
Currently casting container.type as 'page' can mask non-page items. Enforce type === 'page' at the query level and return the literal without a cast. Also apply the same filter for children.
- const databaseQuery = addUserIdToQuery(containerRepository.createQuery(), session.user.id).sort(
- 'lastUpdated',
- 'desc'
- );
+ const databaseQuery = addUserIdToQuery(containerRepository.createQuery(), session.user.id)
+ .eq('type', 'page')
+ .sort('lastUpdated', 'desc');- ? await containerRepository.getByQuery(
- addUserIdToQuery(containerRepository.createQuery(), session.user.id).in('parentId', parentIds)
- )
+ ? await containerRepository.getByQuery(
+ addUserIdToQuery(containerRepository.createQuery(), session.user.id)
+ .eq('type', 'page')
+ .in('parentId', parentIds)
+ )- type: container.type as 'page',
+ type: 'page',- type: child.type as 'page',
+ type: 'page',Also applies to: 33-34, 43-47, 55-59
🤖 Prompt for AI Agents
In src/app/api/v1/pages/tree/route.ts around lines 13-16 (and similarly at
33-34, 43-47, 55-59), you're currently casting container.type as 'page' which
can hide non-page items; instead add a query filter enforcing type === 'page' on
the repository queries (both the parent query and the children query), remove
the type cast from returned payloads and return the literal 'page' string or the
existing type field directly; ensure children queries mirror the same filter so
only page-typed items are fetched and the response payloads contain the correct
literal type without casting.
| return ( | ||
| <html lang="en"> | ||
| <body> | ||
| <MantineProvider theme={theme} defaultColorScheme={colorScheme}> | ||
| <AuthProvider>{children}</AuthProvider> | ||
| </MantineProvider> | ||
| </body> | ||
| </html> | ||
| ); |
There was a problem hiding this comment.
Client component should not render html/body tags.
In Next.js, html and body tags should be rendered by server components (typically in layout.tsx), not client components. Rendering them here may cause hydration mismatches and unexpected behavior. The client layout should only wrap its children with providers.
Apply this diff:
return (
- <html lang="en">
- <body>
- <MantineProvider theme={theme} defaultColorScheme={colorScheme}>
- <AuthProvider>{children}</AuthProvider>
- </MantineProvider>
- </body>
- </html>
+ <MantineProvider theme={theme} defaultColorScheme={colorScheme}>
+ <AuthProvider>{children}</AuthProvider>
+ </MantineProvider>
);
}The parent server layout should handle the html and body tags.
📝 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 ( | |
| <html lang="en"> | |
| <body> | |
| <MantineProvider theme={theme} defaultColorScheme={colorScheme}> | |
| <AuthProvider>{children}</AuthProvider> | |
| </MantineProvider> | |
| </body> | |
| </html> | |
| ); | |
| return ( | |
| <MantineProvider theme={theme} defaultColorScheme={colorScheme}> | |
| <AuthProvider>{children}</AuthProvider> | |
| </MantineProvider> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/app/layout-client.tsx around lines 27 to 35, this client component is
rendering html and body tags which should be provided by the parent server
layout; remove the <html> and <body> elements and return only the provider
wrappers so the component simply wraps children with MantineProvider and
AuthProvider. Ensure the server layout (layout.tsx) is responsible for the
html/lang and body tags, and keep this file as a pure client component exporting
the providers around {children}.
| return ( | ||
| <ActionIcon variant="subtle" size="sm" onClick={onToggle} style={{ flexShrink: 0 }}> | ||
| {isExpanded ? <IconChevronDown size={16} /> : <IconChevronRight size={16} />} | ||
| </ActionIcon> | ||
| ); |
There was a problem hiding this comment.
Add aria-label for screen reader accessibility.
The toggle button lacks an accessible label, preventing screen reader users from understanding its purpose.
Apply this diff to add proper accessibility:
- <ActionIcon variant="subtle" size="sm" onClick={onToggle} style={{ flexShrink: 0 }}>
+ <ActionIcon
+ variant="subtle"
+ size="sm"
+ onClick={onToggle}
+ style={{ flexShrink: 0 }}
+ aria-label={isExpanded ? "Collapse" : "Expand"}
+ >
{isExpanded ? <IconChevronDown size={16} /> : <IconChevronRight size={16} />}
</ActionIcon>🤖 Prompt for AI Agents
In src/components/atoms/tree-toggle.tsx around lines 15 to 19, the ActionIcon
toggle lacks an accessible label for screen readers; add an aria-label and
aria-expanded to the button that reflects the current state (e.g.,
aria-label={`${isExpanded ? 'Collapse' : 'Expand'} tree node`} and
aria-expanded={isExpanded}) so assistive tech conveys purpose and state; ensure
the label text is concise and matches your i18n approach if applicable.
| export const authClient = createAuthClient({ | ||
| baseURL: process.env.NODE_ENV === 'development' ? 'http://localhost:3000' : process.env.NEXT_PUBLIC_API_URL || '', | ||
| }); |
There was a problem hiding this comment.
Empty string fallback for baseURL will cause auth failures.
In production, if NEXT_PUBLIC_API_URL is not set, the baseURL defaults to an empty string, which will cause all auth requests to fail. This failure mode is difficult to debug.
Apply this diff to fail fast with a clear error message:
export const authClient = createAuthClient({
- baseURL: process.env.NODE_ENV === 'development' ? 'http://localhost:3000' : process.env.NEXT_PUBLIC_API_URL || '',
+ baseURL: process.env.NODE_ENV === 'development'
+ ? 'http://localhost:3000'
+ : process.env.NEXT_PUBLIC_API_URL || (() => { throw new Error('NEXT_PUBLIC_API_URL is required in 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.
| export const authClient = createAuthClient({ | |
| baseURL: process.env.NODE_ENV === 'development' ? 'http://localhost:3000' : process.env.NEXT_PUBLIC_API_URL || '', | |
| }); | |
| export const authClient = createAuthClient({ | |
| baseURL: process.env.NODE_ENV === 'development' | |
| ? 'http://localhost:3000' | |
| : process.env.NEXT_PUBLIC_API_URL || (() => { throw new Error('NEXT_PUBLIC_API_URL is required in production') })(), | |
| }); |
🤖 Prompt for AI Agents
In src/lib/auth/client.ts around lines 4-6, the production fallback sets baseURL
to an empty string which breaks auth; change the logic to fail fast: when
process.env.NODE_ENV !== 'development' and process.env.NEXT_PUBLIC_API_URL is
missing or empty, throw a clear Error (e.g. "NEXT_PUBLIC_API_URL must be set in
production") so the app fails at startup; otherwise keep the dev localhost URL
or use NEXT_PUBLIC_API_URL as the baseURL.
| after: async (user) => { | ||
| const workspaceRepository = await getWorkspaceRepository(); | ||
| const workspace = await workspaceRepository.create({ | ||
| name: 'Default Workspace', | ||
| userId: user.id, | ||
| createdAt: new Date().toISOString(), | ||
| lastUpdated: new Date().toISOString(), | ||
| } satisfies WorkspaceCreate); | ||
|
|
||
| const containerRepository = await getContainerRepository(); | ||
| await containerRepository.create({ | ||
| name: 'Welcome', | ||
| type: 'page', | ||
| userId: user.id, | ||
| createdAt: new Date().toISOString(), | ||
| lastUpdated: new Date().toISOString(), | ||
| workspaceId: workspace.id, | ||
| emoji: '👋', | ||
| parentId: null, | ||
| } satisfies ContainerCreate); | ||
| }, |
There was a problem hiding this comment.
Database hook lacks error handling and transaction safety.
The user.create hook performs multiple database operations without error handling or transactional guarantees. If workspace or container creation fails, the user will be left in an incomplete state, which could break the application for that user.
Consider these improvements:
- Wrap operations in try-catch with logging
- Consider using database transactions if SuperSave supports them
- Add recovery logic or mark the user as needing setup
Additionally, create the timestamp once and reuse it:
after: async (user) => {
+ const now = new Date().toISOString();
+ try {
const workspaceRepository = await getWorkspaceRepository();
const workspace = await workspaceRepository.create({
name: 'Default Workspace',
userId: user.id,
- createdAt: new Date().toISOString(),
- lastUpdated: new Date().toISOString(),
+ createdAt: now,
+ lastUpdated: now,
} satisfies WorkspaceCreate);
const containerRepository = await getContainerRepository();
await containerRepository.create({
name: 'Welcome',
type: 'page',
userId: user.id,
- createdAt: new Date().toISOString(),
- lastUpdated: new Date().toISOString(),
+ createdAt: now,
+ lastUpdated: now,
workspaceId: workspace.id,
emoji: '👋',
parentId: null,
} satisfies ContainerCreate);
+ } catch (error) {
+ // Log error and consider marking user as needing setup
+ console.error('Failed to create default workspace for user', user.id, error);
+ throw error; // Re-throw to prevent user creation from completing
+ }
},📝 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.
| after: async (user) => { | |
| const workspaceRepository = await getWorkspaceRepository(); | |
| const workspace = await workspaceRepository.create({ | |
| name: 'Default Workspace', | |
| userId: user.id, | |
| createdAt: new Date().toISOString(), | |
| lastUpdated: new Date().toISOString(), | |
| } satisfies WorkspaceCreate); | |
| const containerRepository = await getContainerRepository(); | |
| await containerRepository.create({ | |
| name: 'Welcome', | |
| type: 'page', | |
| userId: user.id, | |
| createdAt: new Date().toISOString(), | |
| lastUpdated: new Date().toISOString(), | |
| workspaceId: workspace.id, | |
| emoji: '👋', | |
| parentId: null, | |
| } satisfies ContainerCreate); | |
| }, | |
| after: async (user) => { | |
| const now = new Date().toISOString(); | |
| try { | |
| const workspaceRepository = await getWorkspaceRepository(); | |
| const workspace = await workspaceRepository.create({ | |
| name: 'Default Workspace', | |
| userId: user.id, | |
| createdAt: now, | |
| lastUpdated: now, | |
| } satisfies WorkspaceCreate); | |
| const containerRepository = await getContainerRepository(); | |
| await containerRepository.create({ | |
| name: 'Welcome', | |
| type: 'page', | |
| userId: user.id, | |
| createdAt: now, | |
| lastUpdated: now, | |
| workspaceId: workspace.id, | |
| emoji: '👋', | |
| parentId: null, | |
| } satisfies ContainerCreate); | |
| } catch (error) { | |
| // Log error and consider marking user as needing setup | |
| console.error('Failed to create default workspace for user', user.id, error); | |
| throw error; // Re-throw to prevent user creation from completing | |
| } | |
| }, |
🤖 Prompt for AI Agents
In src/lib/auth/config.ts around lines 32 to 52, the after hook creates a
workspace and container without error handling, transaction safety, or a single
timestamp; wrap the multi-step DB operations in a transaction if the ORM
(SuperSave) supports it (begin transaction, create workspace and container,
commit), otherwise perform them inside a try-catch and on failure log the error
and perform recovery (rollback if in a transaction, or delete the partially
created workspace/container and mark the user as needing-setup); also compute a
single timestamp variable once and reuse it for createdAt/lastUpdated, and
ensure any thrown errors are rethrown or bubbled so the caller can react.
| format: winston.format.combine( | ||
| winston.format.timestamp(), | ||
| winston.format.errors({ stack: true }), | ||
| winston.format.json() | ||
| ), | ||
| defaultMeta: { service: 'thoth' }, | ||
| transports: [ | ||
| new winston.transports.Console({ | ||
| format: winston.format.combine(winston.format.colorize(), winston.format.simple()), | ||
| }), |
There was a problem hiding this comment.
Format configuration conflict: JSON format is overridden by console transport.
The logger configures winston.format.json() at the top level (line 9), but the Console transport overrides it with winston.format.simple() (line 14). This means the JSON format never takes effect—all logs are output in simple format instead of structured JSON.
For production environments, structured JSON logging is essential for log aggregation and analysis tools (e.g., CloudWatch, ELK, Datadog).
Apply this diff to separate development and production formats:
export const logger = winston.createLogger({
level: environment.LOG_LEVEL,
format: winston.format.combine(
winston.format.timestamp(),
winston.format.errors({ stack: true }),
winston.format.json()
),
defaultMeta: { service: 'thoth' },
transports: [
new winston.transports.Console({
- format: winston.format.combine(winston.format.colorize(), winston.format.simple()),
+ format: environment.NODE_ENV === 'development'
+ ? winston.format.combine(winston.format.colorize(), winston.format.simple())
+ : winston.format.json(),
}),
],
});📝 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.
| format: winston.format.combine( | |
| winston.format.timestamp(), | |
| winston.format.errors({ stack: true }), | |
| winston.format.json() | |
| ), | |
| defaultMeta: { service: 'thoth' }, | |
| transports: [ | |
| new winston.transports.Console({ | |
| format: winston.format.combine(winston.format.colorize(), winston.format.simple()), | |
| }), | |
| format: winston.format.combine( | |
| winston.format.timestamp(), | |
| winston.format.errors({ stack: true }), | |
| winston.format.json() | |
| ), | |
| defaultMeta: { service: 'thoth' }, | |
| transports: [ | |
| new winston.transports.Console({ | |
| format: environment.NODE_ENV === 'development' | |
| ? winston.format.combine(winston.format.colorize(), winston.format.simple()) | |
| : winston.format.json(), | |
| }), |
| export const $currentPageId = atom<string | null>(null); | ||
|
|
||
| export const $currentPage = createFetcherStore<GetPageDetailsResponse>(['/pages/', $currentPageId]); |
There was a problem hiding this comment.
Handle null pageId to prevent invalid API calls.
When $currentPageId is null, the fetcher will construct the URL /pages/null (string concatenation), which will trigger an API call to a non-existent endpoint. This could result in unnecessary 404 errors and wasted requests.
Consider adding conditional logic to prevent fetching when the ID is null, or update the fetcher to handle null values in the key array.
One approach is to conditionally create the store or use a computed value that returns undefined when the ID is null:
import { atom, computed } from 'nanostores';
import type { GetPageDetailsResponse } from '@/types/api';
import { createFetcherStore } from '../fetcher';
export const $currentPageId = atom<string | null>(null);
// Create a computed atom that returns a valid key or a sentinel value
const $pageKey = computed($currentPageId, (id) =>
id ? ['/pages/', id] : null
);
export const $currentPage = createFetcherStore<GetPageDetailsResponse>($pageKey);Alternatively, you may need to update the fetcher utility to skip fetching when keys contain null values.
1 participant
Summary by CodeRabbit