-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Implement consistent loading states #44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRemoved analytics usage and related error handling from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant NextPage as page.tsx
participant Data as Data Source
participant Home as HomePageContainer
User->>NextPage: Request /
NextPage->>Data: Fetch result
Data-->>NextPage: result.data
Note right of NextPage: Derive initialProjects from result.data
NextPage->>Home: Render with { initialProjects, userId }
Home-->>User: Home view
%% Removed (previous flow)
rect rgba(255, 235, 235, 0.5)
Note over NextPage: Analytics fetching, derivation,<br/>and error handling removed
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
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 ignored due to path filters (1)
📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/jwt.ts (1)
8-17: Separate JWT mock from loading-state PR and fix production middleware
- src/lib/jwt.ts’s
verifyJWTthrows whenAPP_ENV==='production', but src/middleware.ts calls it unconditionally—this will break production. Provide a real JWT verifier for production or guard the middleware call withif (APP_ENV !== 'production').- This JWT change is unrelated to the loading-state work; move it into a dedicated PR for clarity.
package.json (1)
14-48: Fix ESLint plugin placement and remove invalid entry
- Remove
"eslint-plugin-next": "^0.0.0"; rely on@next/eslint-plugin-next@15.5.4.- Move all ESLint tooling into devDependencies:
•@eslint/js
•@typescript-eslint/eslint-plugin&@typescript-eslint/parser
•eslint-plugin-react
•eslint-plugin-react-hooks(v7.0.0)
•@next/eslint-plugin-next- Update the lint script to
"lint": "eslint ."
🧹 Nitpick comments (7)
src/app/(general)/page.tsx (1)
26-29: Gate loading skeleton when server-fetched data exists
ProjectsTable currently flashes a skeleton on mount becauseloading={isLoading}even withinitialProjectspresent—change toloading={isLoading && projects.length === 0}.src/actions/projects/createProject/schema.ts (1)
13-13: LGTM! Consider adding validation constraints.The optional
templateIdfield is correctly added to the schema. However, consider whether template IDs should have format constraints (e.g., length limits, allowed characters) to ensure data integrity.Apply this diff if you want to add basic constraints:
- templateId: z.string().optional(), + templateId: z.string().trim().min(1).max(50).optional(),src/components/models/projects/ProjectsTable.tsx (1)
17-19: Skeleton columns mismatch with table headersBasicTable renders 2 columns ("Title", "Type"), but TableSkeleton uses 3. Align to 2 for visual consistency.
- if (loading) { - return <TableSkeleton columns={3} />; - } + if (loading) { + return <TableSkeleton columns={2} />; + }src/hooks/use-action.ts (1)
7-18: Global loading boolean is not concurrency-safeMultiple concurrent actions will toggle setLoading(true/false) racing to false prematurely. Use a ref-count or token-based API in LoadingContext (increment on start, decrement on finish; expose derived isLoading = count > 0).
src/lib/templates.ts (1)
8-27: Prefer readonly to prevent accidental mutationMake the templates array readonly for safety.
-export const projectTemplates: ProjectTemplate[] = [ +export const projectTemplates: Readonly<ProjectTemplate[]> = [src/components/models/templates/TemplateCard.tsx (1)
13-21: Add keyboard and ARIA semanticsClickable Card needs keyboard support and role semantics.
<Card className={cn( 'cursor-pointer hover:border-primary transition-colors', isSelected && 'border-primary ring-2 ring-primary ring-offset-2', className )} - onClick={() => onSelect(template.id)} + role="button" + tabIndex={0} + aria-pressed={isSelected} + onClick={() => onSelect(template.id)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onSelect(template.id); + } + }} {...props}src/components/models/projects/CreateProjectForm.tsx (1)
54-56: Optional: prefer disabling form during submit instead of replacing itReturning a skeleton unmounts the form and loses context. Consider an overlay/disabled state instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
eslint.config.mjs(2 hunks)package.json(3 hunks)src/actions/projects/createProject/action.ts(1 hunks)src/actions/projects/createProject/logic.ts(1 hunks)src/actions/projects/createProject/schema.ts(1 hunks)src/actions/user.ts(1 hunks)src/app/(general)/layout.tsx(2 hunks)src/app/(general)/page.tsx(1 hunks)src/components/common/Skeleton/FormSkeleton.tsx(1 hunks)src/components/common/Skeleton/TableSkeleton.tsx(1 hunks)src/components/models/auth/SignupForm.tsx(0 hunks)src/components/models/projects/CreateProjectDialog.tsx(2 hunks)src/components/models/projects/CreateProjectForm.tsx(2 hunks)src/components/models/projects/ProjectsTable.tsx(1 hunks)src/components/models/templates/TemplateCard.tsx(1 hunks)src/components/models/templates/TemplateSelector.tsx(1 hunks)src/components/pages/home/HomePageContainer.tsx(1 hunks)src/components/ui/sidebar.tsx(1 hunks)src/contexts/LoadingContext.tsx(1 hunks)src/hooks/use-action.ts(1 hunks)src/lib/jwt.ts(1 hunks)src/lib/templates.ts(1 hunks)src/lib/validations/projects.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/models/auth/SignupForm.tsx
🧰 Additional context used
🧬 Code graph analysis (14)
src/lib/jwt.ts (1)
src/middleware.ts (1)
middleware(4-26)
src/lib/templates.ts (1)
src/app/(general)/create/page.tsx (1)
CreateProjectPage(4-10)
src/app/(general)/layout.tsx (1)
src/contexts/LoadingContext.tsx (1)
LoadingProvider(10-18)
src/actions/projects/createProject/logic.ts (4)
src/actions/projects/createProject/schema.ts (1)
CreateProjectInput(16-16)src/lib/result.ts (1)
Result(18-18)src/lib/prisma.ts (1)
prisma(7-7)src/actions/projects/updateProject/logic.ts (1)
updateProject(8-19)
src/components/models/templates/TemplateCard.tsx (3)
src/lib/templates.ts (1)
ProjectTemplate(1-6)src/components/ui/card.tsx (10)
Card(85-85)CardHeader(86-86)CardTitle(88-88)CardDescription(90-90)CardContent(91-91)Card(5-16)CardTitle(31-39)CardDescription(41-49)CardHeader(18-29)CardContent(64-72)src/lib/utils.ts (1)
cn(4-6)
src/components/models/projects/CreateProjectForm.tsx (4)
src/lib/validations/projects.ts (2)
CreateProjectFormValues(10-10)CreateProjectSchema(4-8)src/hooks/use-action.ts (1)
useAction(6-22)src/actions/projects/createProject/action.ts (1)
createProjectAction(10-38)src/components/common/Skeleton/FormSkeleton.tsx (1)
FormSkeleton(7-18)
src/app/(general)/page.tsx (1)
src/components/pages/home/HomePageContainer.tsx (1)
HomePageContainer(15-40)
src/components/common/Skeleton/TableSkeleton.tsx (2)
src/components/ui/table.tsx (6)
Table(108-108)TableHeader(109-109)TableRow(113-113)TableHead(112-112)TableBody(110-110)TableCell(114-114)src/components/ui/skeleton.tsx (1)
Skeleton(13-13)
src/components/models/projects/ProjectsTable.tsx (1)
src/components/common/Skeleton/TableSkeleton.tsx (1)
TableSkeleton(9-34)
src/components/models/templates/TemplateSelector.tsx (2)
src/lib/templates.ts (1)
projectTemplates(8-27)src/components/models/templates/TemplateCard.tsx (1)
TemplateCard(11-31)
src/components/models/projects/CreateProjectDialog.tsx (2)
src/components/models/templates/TemplateSelector.tsx (1)
TemplateSelector(9-22)src/components/models/projects/CreateProjectForm.tsx (1)
CreateProjectForm(21-93)
src/hooks/use-action.ts (1)
src/contexts/LoadingContext.tsx (1)
useLoading(20-26)
src/actions/projects/createProject/action.ts (3)
src/actions/projects/createProject/schema.ts (1)
createProjectSchema(4-14)src/lib/templates.ts (1)
projectTemplates(8-27)src/actions/projects/createProject/logic.ts (1)
createProject(8-21)
src/components/pages/home/HomePageContainer.tsx (3)
src/hooks/use-action.ts (1)
useAction(6-22)src/actions/projects/getProjects.ts (1)
getProjects(11-30)src/components/models/projects/ProjectsTable.tsx (1)
ProjectsTable(14-68)
🪛 GitHub Actions: Lint and Build
src/contexts/LoadingContext.tsx
[error] 1-1: Next.js build failed due to server/client component mismatch: the context file imports createContext/useState but is used in a server component; mark the consuming components with 'use client' or convert LoadingContext.tsx to a client component.
🪛 GitHub Check: lint-and-build
src/components/models/templates/TemplateSelector.tsx
[warning] 1-1:
'ProjectTemplate' is defined but never used
🔇 Additional comments (14)
src/actions/user.ts (1)
2-16: Run a broad search for usages of this file and its exports#!/bin/bash echo "=== Checking for imports of src/actions/user.ts ===" rg -n --no-ignore --max-filesize 1M "@/actions/user" . echo -e "\n=== Checking for any path-based imports ===" rg -n --no-ignore --max-filesize 1M "actions/user" . echo -e "\n=== Checking for references to updateProfile or changePassword ===" rg -n --no-ignore --max-filesize 1M "\b(updateProfile|changePassword)\b" .If no matches are found, this file (and its commented-out exports) is unused and can be removed. Otherwise, restore or replace the functionality.
src/components/ui/sidebar.tsx (1)
610-613: Good switch to one-time width initializationUsing useState initializer for random width avoids recomputation and keeps the skeleton stable across re-renders. LGTM.
src/lib/validations/projects.ts (1)
7-7: Watch for schema drift between validation layers.The
templateIdfield is now defined in bothsrc/lib/validations/projects.tsandsrc/actions/projects/createProject/schema.ts. While this separation may be intentional (client vs. server validation), ensure these schemas remain synchronized when validation rules change.Consider consolidating or establishing a single source of truth for shared validation rules to prevent drift.
src/components/common/Skeleton/FormSkeleton.tsx (1)
1-18: LGTM! Clean skeleton implementation.The FormSkeleton component is well-structured with configurable fields and proper key usage in the mapped elements. The default value of 3 fields is reasonable for most forms.
src/components/models/templates/TemplateSelector.tsx (1)
9-22: LGTM! Clean template selection implementation.The component correctly maps over templates and delegates rendering to
TemplateCard, properly computing theisSelectedstate for each template.src/components/common/Skeleton/TableSkeleton.tsx (1)
1-34: LGTM! Well-structured table skeleton.The TableSkeleton component correctly renders a skeleton structure with configurable columns and rows, using proper key props in the nested iterations. The default of 5 rows is reasonable.
src/components/models/projects/CreateProjectDialog.tsx (2)
10-10: Verify default template ID exists.The
selectedTemplateIdis initialized to'blank', which should correspond to a template inprojectTemplates. Based on the relevant code snippet fromsrc/lib/templates.ts, the 'blank' template does exist, so this is correct.If you add or rename templates in the future, ensure this default value is updated accordingly.
20-25: LGTM! Clean dialog layout with template selection.The dialog correctly integrates template selection before project details, passing the selected template ID to the form. The layout structure is clear and follows a logical flow.
src/actions/projects/createProject/logic.ts (2)
8-9: LGTM! Content field properly integrated.The function signature correctly accepts an optional
contentfield, and the destructuring includes it for use in project creation.
16-16: Appropriate default for missing content.The fallback to an empty string (
content || '') ensures the database always receives a valid string value, which is appropriate for optional template content.src/contexts/LoadingContext.tsx (2)
10-18: LGTM! Standard context provider pattern.The
LoadingProvidercomponent follows React best practices for context providers, maintaining state and exposing it to consumers.
20-26: LGTM! Proper error handling in custom hook.The
useLoadinghook correctly validates that it's used within aLoadingProviderand throws a descriptive error otherwise. This prevents runtime issues from improper usage.src/actions/projects/createProject/action.ts (2)
15-23: LGTM on template content initializationCleanly derives initialContent from templateId and falls back safely.
26-26: LGTM on create invocationPassing content into createProject maintains compatibility and revalidates cache.
| export default tseslint.config( | ||
| js.configs.recommended, | ||
| { | ||
| files: ["**/*.{js,jsx}"], | ||
| languageOptions: { | ||
| globals: { | ||
| ...globals.browser, | ||
| ...globals.node, | ||
| }, | ||
| }, | ||
| rules: { | ||
| // Add any specific JavaScript rules here if needed | ||
| }, | ||
| }, | ||
| ...tseslint.configs.recommended, | ||
| { | ||
| files: ["src/**/*.{ts,tsx}"], | ||
| languageOptions: { | ||
| parser: tseslint.parser, | ||
| parserOptions: { | ||
| ecmaFeatures: { | ||
| jsx: true, | ||
| }, | ||
| ecmaVersion: "latest", | ||
| sourceType: "module", | ||
| project: ["./tsconfig.json"], | ||
| }, | ||
| globals: { | ||
| ...globals.browser, | ||
| ...globals.node, | ||
| }, | ||
| }, | ||
| plugins: { | ||
| react, | ||
| "react-hooks": reactHooks, | ||
| "@next/next": nextPlugin, | ||
| }, | ||
| rules: { | ||
| ...react.configs.recommended.rules, | ||
| ...reactHooks.configs.recommended.rules, | ||
| ...nextPlugin.configs.recommended.rules, | ||
| "react/react-in-jsx-scope": "off", | ||
| "@typescript-eslint/no-unused-vars": [ | ||
| "warn", | ||
| { argsIgnorePattern: "^_" }, | ||
| ], | ||
| }, | ||
| settings: { | ||
| react: { | ||
| version: "detect", | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Scope @eslint/js to JS files to avoid rule overlap on TS
Passing js.configs.recommended as a top-level arg applies it to all files (including TS), potentially duplicating/conflicting rules. Prefer scoping it with files.
Apply:
-export default tseslint.config(
- js.configs.recommended,
- {
- files: ["**/*.{js,jsx}"],
- languageOptions: { globals: { ...globals.browser, ...globals.node } },
- rules: { },
- },
+export default tseslint.config(
+ {
+ ...js.configs.recommended,
+ files: ["**/*.{js,jsx}"],
+ languageOptions: { globals: { ...globals.browser, ...globals.node } },
+ },
...tseslint.configs.recommended,
{
files: ["src/**/*.{ts,tsx}"],
languageOptions: {
parser: tseslint.parser,
parserOptions: {
ecmaFeatures: { jsx: true },
ecmaVersion: "latest",
sourceType: "module",
project: ["./tsconfig.json"],
},
globals: { ...globals.browser, ...globals.node },
},
plugins: { react, "react-hooks": reactHooks, "@next/next": nextPlugin },
- rules: {
- ...react.configs.recommended.rules,
- ...reactHooks.configs.recommended.rules,
- ...nextPlugin.configs.recommended.rules,
+ rules: {
+ ...react.configs.recommended.rules,
+ ...reactHooks.configs.recommended.rules,
+ // Verify this export exists; see note below.
+ ...nextPlugin.configs.recommended?.rules,
"react/react-in-jsx-scope": "off",
"@typescript-eslint/no-unused-vars": ["warn", { argsIgnorePattern: "^_" }],
},
settings: { react: { version: "detect" } },
},
{ ignores: ["node_modules/**", ".next/**", "out/**", "build/**", "next-env.d.ts", "src/generated"] },
);Also verify Next plugin config availability. If recommended is not exported by @next/eslint-plugin-next in flat config, consider using eslint-config-next as a base and layering TS/React rules on top.
🌐 Web query:
In ESLint flat config, does @next/eslint-plugin-next export a "configs.recommended" (or "core-web-vitals") usable via plugin.configs.recommended.rules, or should eslint-config-next be used instead?
💡 Result:
Short answer: @next/eslint-plugin-next does export a configs.recommended (and—after the linked change—core-web-vitals), so with ESLint flat config you can import the plugin and use plugin.configs.recommended (or plugin.configs['core-web-vitals']) directly. If you prefer the traditional shareable config names (next / next/core-web-vitals) or need the exact behavior of eslint-config-next, you can still use eslint-config-next (or use FlatCompat to load it). (nextjs.org)
Citations:
Scope js.configs.recommended to JS files to avoid duplicate rule application
Spread js.configs.recommended inside the JS-only config block rather than passing it top-level:
-export default tseslint.config(
- js.configs.recommended,
- {
- files: ["**/*.{js,jsx}"],
- languageOptions: { globals: { ...globals.browser, ...globals.node } },
- },
+export default tseslint.config(
+ {
+ ...js.configs.recommended,
+ files: ["**/*.{js,jsx}"],
+ languageOptions: { globals: { ...globals.browser, ...globals.node } },
+ },
...tseslint.configs.recommended,@next/eslint-plugin-next exports configs.recommended, so ...nextPlugin.configs.recommended.rules is valid.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In eslint.config.mjs around lines 8 to 60, the top-level spread of
js.configs.recommended causes its rules to apply globally and duplicate with the
JS-specific config; move or spread js.configs.recommended into the JS-only
config block (the one with files ["**/*.{js,jsx}"]) so its rules are scoped to
JS files only, i.e., remove the top-level spread and instead include
...js.configs.recommended.rules inside that JS config object to avoid duplicate
rule application.
| import { AppSidebar } from '@/components/common/Sidebar/AppSidebar'; | ||
| import { GeneralLayout } from '@/components/layouts/GeneralLayout'; | ||
| import { SidebarProvider } from '@/components/ui/sidebar'; | ||
| import { LoadingProvider } from '@/contexts/LoadingContext'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
LoadingProvider placement looks good; confirm client boundary and scope
- Ensure src/contexts/LoadingContext.tsx has "use client" at top; it uses useState.
- Only GeneralLayout subtree gets the context. If Sidebar or other shells need loading state, wrap higher (around SidebarProvider).
Also applies to: 14-16
🏁 Script executed:
#!/bin/bash
# Verify LoadingContext is a client component and check other usage sites
rg -n --no-ignore -C2 '^["'\'']use client["'\'']' src/contexts/LoadingContext.tsx
rg -n -C2 'useLoading\\(|LoadingProvider' src | sed -n '1,200p'Length of output: 92
🏁 Script executed:
#!/bin/bash
# Verify LoadingContext is a client component
rg -n "^['\"]use client['\"]" src/contexts/LoadingContext.tsx
# Show top of LoadingContext.tsx for confirmation
sed -n '1,20p' src/contexts/LoadingContext.tsx
# Search for useLoading hook usage
rg -n -C2 '\buseLoading\s*\(' src
# Search for LoadingProvider usage
rg -n -C2 '\bLoadingProvider\b' srcLength of output: 2760
Missing client directive in LoadingContext.tsx
- Add
"use client";as the very first line of src/contexts/LoadingContext.tsx so it can useuseStateand context hooks. - Note the provider currently wraps only GeneralLayout; if Sidebar (or other shells) need loading state, lift LoadingProvider to include them.
🤖 Prompt for AI Agents
In src/app/(general)/layout.tsx around line 4, the LoadingContext consumer
requires its provider file to be a client component; open
src/contexts/LoadingContext.tsx and add the directive "use client" as the very
first line so useState and context hooks are allowed, then ensure the provider
export remains unchanged; additionally, if Sidebar or other shell components
need access to loading state, move the LoadingProvider higher in the tree (e.g.,
wrap Sidebar and GeneralLayout) in layout.tsx so those components are enclosed
by the provider.
| type: ProjectType.article, | ||
| templateId: selectedTemplateId, | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Ensure enum values align with Prisma ProjectType
defaultValues.type = ProjectType.article and FormSelect item values ('article' | 'ebook' | 'newsletter') must exactly match z.nativeEnum(ProjectType). If your Prisma enum uses uppercase (e.g., 'ARTICLE'), this will fail validation.
Also applies to: 62-76
🏁 Script executed:
#!/bin/bash
# Inspect generated ProjectType to confirm allowed values.
rg -n -C2 "export (enum|const) ProjectType|nativeEnum\\(ProjectType\\)" src | cat
rg -n -C1 "ProjectType\\." src | catLength of output: 618
🏁 Script executed:
#!/bin/bash
# Locate Prisma enum definition for ProjectType
rg -g '*.prisma' -n -C2 "enum ProjectType" .Length of output: 169
🏁 Script executed:
#!/bin/bash
rg -n "newsletter" -C2 src/components/models/projects/CreateProjectForm.tsxLength of output: 124
Ensure FormSelect values match Prisma ProjectType
CreateProjectForm.tsx (lines 73-76) includes 'newsletter', but Prisma’s ProjectType (schema.prisma 36-38) only defines 'article' and 'ebook'; z.nativeEnum(ProjectType) will reject 'newsletter'—remove it or update the enum.
🤖 Prompt for AI Agents
In src/components/models/projects/CreateProjectForm.tsx around lines 28 to 30,
the FormSelect includes a 'newsletter' value that doesn't exist on the Prisma
ProjectType enum (which only has 'article' and 'ebook'), causing
z.nativeEnum(ProjectType) validation to fail; remove the 'newsletter' option (or
alternatively add it to the Prisma enum/schema if intended) and ensure all
places that set or default the project type (including initial form values and
any templateId → type mapping) are updated to only use 'article' or 'ebook' so
runtime validation passes.
| // Update default value for templateId when selectedTemplateId changes | ||
| // This ensures the form reflects the currently selected template in the dialog | ||
| form.setValue('templateId', selectedTemplateId); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: setValue called on every render causes re-renders and dirty-state bugs
Wrap in useEffect keyed on selectedTemplateId.
-// Update default value for templateId when selectedTemplateId changes
-// This ensures the form reflects the currently selected template in the dialog
-form.setValue('templateId', selectedTemplateId);
+// Keep templateId in sync with the dialog selection
+useEffect(() => {
+ form.setValue('templateId', selectedTemplateId, { shouldDirty: false, shouldTouch: false });
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+}, [selectedTemplateId]);📝 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.
| // Update default value for templateId when selectedTemplateId changes | |
| // This ensures the form reflects the currently selected template in the dialog | |
| form.setValue('templateId', selectedTemplateId); | |
| // Keep templateId in sync with the dialog selection | |
| useEffect(() => { | |
| form.setValue('templateId', selectedTemplateId, { shouldDirty: false, shouldTouch: false }); | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, [selectedTemplateId]); |
🤖 Prompt for AI Agents
In src/components/models/projects/CreateProjectForm.tsx around lines 33 to 36,
the call form.setValue('templateId', selectedTemplateId) is executed on every
render causing extra re-renders and dirty-state issues; move this into a
useEffect that depends only on selectedTemplateId, guard for undefined/null
values, and call form.setValue with options to avoid marking the form
dirty/touched/triggering validation (e.g. { shouldDirty: false, shouldTouch:
false, shouldValidate: false }) so the templateId is updated only when
selectedTemplateId changes without altering form dirty state.
| @@ -0,0 +1,22 @@ | |||
| import { ProjectTemplate, projectTemplates } from '@/lib/templates'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused import.
The ProjectTemplate type is imported but never used in this component. Only projectTemplates (the data array) is referenced.
Apply this diff to remove the unused import:
-import { ProjectTemplate, projectTemplates } from '@/lib/templates';
+import { projectTemplates } from '@/lib/templates';📝 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 { ProjectTemplate, projectTemplates } from '@/lib/templates'; | |
| import { projectTemplates } from '@/lib/templates'; |
🧰 Tools
🪛 GitHub Check: lint-and-build
[warning] 1-1:
'ProjectTemplate' is defined but never used
🤖 Prompt for AI Agents
In src/components/models/templates/TemplateSelector.tsx around line 1, the
import includes the unused ProjectTemplate type which should be removed; update
the import to only import projectTemplates from '@/lib/templates' (i.e., remove
ProjectTemplate from the import list) and save so linters/TS no-unused-vars
warnings are resolved.
| "use client"; | ||
|
|
||
| import { ProjectsTable } from '@/components/models/projects/ProjectsTable'; | ||
| import { CreateProjectButton } from '@/components/models/projects/CreateProjectButton'; | ||
| import { Project } from '@/generated/prisma'; | ||
| import { getProjects } from '@/actions/projects/getProjects'; | ||
| import { useEffect, useState } from 'react'; | ||
| import { useAction } from '@/hooks/use-action'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify server/client boundary for getProjects
This is a client component ("use client"). Ensure getProjects is a server action (annotated 'use server') or a fetch to an API route; otherwise bundling will fail due to prisma usage on the client. Also relies on wrappedAction being memoized (see useAction fix) to avoid effect loops.
Also applies to: 19-30
🏁 Script executed:
#!/bin/bash
# Confirm getProjects is a server action and not importing prisma into the client bundle.
rg -n -C2 "^'use server'|\"use server\"" src/actions/projects/getProjects.ts || true
rg -n -C2 "from 'prisma'|from \"@/lib/prisma\"" src/actions/projects/getProjects.ts || trueLength of output: 0
🏁 Script executed:
#!/bin/bash
# Show top of getProjects.ts to confirm 'use server' and prisma imports
if [ -f src/actions/projects/getProjects.ts ]; then
head -n 50 src/actions/projects/getProjects.ts
else
echo "File not found: src/actions/projects/getProjects.ts"
fiLength of output: 659
Annotate getProjects as a server action or switch to an API route
getProjectsimportsprismabut lacks a'use server'directive; add it at the top ofsrc/actions/projects/getProjects.tsor convert this call to an API fetch to avoid bundling server code into the client.- Ensure the callback passed to
useActionis memoized to prevent effect loops.
🤖 Prompt for AI Agents
In src/components/pages/home/HomePageContainer.tsx lines 1-8: getProjects is a
server-only action that imports prisma and must not be bundled into client code
— either add "use server" at the top of src/actions/projects/getProjects.ts
(preferable) or replace the direct call with a client-side API fetch to a
Next.js API route; additionally, ensure the callback passed into useAction is
memoized (e.g., wrap it with useCallback in this file) so the effect that
depends on it does not re-run continuously.
|
|
||
| useEffect(() => { | ||
| const loadProjects = async () => { | ||
| const result = await fetchProjects(userId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: getProjects called with wrong input shape
getProjects expects an object, not a string. Pass { userId }.
- const result = await fetchProjects(userId);
+ const result = await fetchProjects({ userId });📝 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 result = await fetchProjects(userId); | |
| const result = await fetchProjects({ userId }); |
🤖 Prompt for AI Agents
In src/components/pages/home/HomePageContainer.tsx around line 21, the call uses
fetchProjects(userId) but the getProjects/fetchProjects API expects an object
shape; change the single string argument to an object by calling fetchProjects({
userId }) so the function receives the correct input shape and continue to await
the result as before.
| @@ -0,0 +1,26 @@ | |||
| import React, { createContext, useContext, useState, ReactNode } from 'react'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add 'use client' directive to fix build error.
The pipeline failure indicates this file must be marked as a client component because it uses React hooks (useState, createContext, useContext). These are client-only APIs in Next.js.
Apply this diff to fix the build error:
+'use client';
+
import React, { createContext, useContext, useState, ReactNode } from 'react';🧰 Tools
🪛 GitHub Actions: Lint and Build
[error] 1-1: Next.js build failed due to server/client component mismatch: the context file imports createContext/useState but is used in a server component; mark the consuming components with 'use client' or convert LoadingContext.tsx to a client component.
🤖 Prompt for AI Agents
In src/contexts/LoadingContext.tsx at or before line 1, this file uses React
hooks and must be a Next.js client component; add the "use client" directive as
the very first line of the file (a solitary "use client" string literal) before
any imports to mark it as a client component so hooks like useState,
createContext and useContext can be used without build errors.
| const wrappedAction = async (input: TInput) => { | ||
| setIsActionLoading(true); | ||
| setLoading(true); | ||
| try { | ||
| return await action(input); | ||
| } finally { | ||
| setIsActionLoading(false); | ||
| setLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unstable wrappedAction causes effects to re-run; memoize with useCallback
Without memoization, consumers putting wrappedAction in deps will loop on every render. Wrap it in useCallback.
-import { useState } from 'react';
+import { useCallback, useState } from 'react';
@@
- const wrappedAction = async (input: TInput) => {
+ const wrappedAction = useCallback(async (input: TInput) => {
setIsActionLoading(true);
setLoading(true);
try {
return await action(input);
} finally {
setIsActionLoading(false);
setLoading(false);
}
- };
+ }, [action, setLoading]);📝 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 wrappedAction = async (input: TInput) => { | |
| setIsActionLoading(true); | |
| setLoading(true); | |
| try { | |
| return await action(input); | |
| } finally { | |
| setIsActionLoading(false); | |
| setLoading(false); | |
| } | |
| }; | |
| import { useCallback, useState } from 'react'; | |
| const wrappedAction = useCallback(async (input: TInput) => { | |
| setIsActionLoading(true); | |
| setLoading(true); | |
| try { | |
| return await action(input); | |
| } finally { | |
| setIsActionLoading(false); | |
| setLoading(false); | |
| } | |
| }, [action, setLoading]); |
🤖 Prompt for AI Agents
In src/hooks/use-action.ts around lines 10 to 19, the wrappedAction function is
recreated every render which breaks consumers that use it in dependency arrays;
wrap wrappedAction in React.useCallback to memoize it, keeping the same async
behavior and body, and include the needed dependencies (at minimum the action
reference; state setters are stable but you can include them if you prefer) in
the dependency array so the callback only changes when the underlying action
changes.
This PR introduces consistent loading states across the application to improve user experience.
Changes include:
TableSkeleton.tsxandFormSkeleton.tsxinsrc/components/common/Skeleton/for reusable loading indicators.src/components/models/projects/ProjectsTable.tsxto displayTableSkeletonduring data loading.src/components/models/projects/CreateProjectForm.tsxto displayFormSkeletonduring form submission/loading.LoadingContext.tsxinsrc/contexts/to manage global loading states.Review Notes:
ProjectsTableandCreateProjectForm.Summary by CodeRabbit