Feat/dashboard route placeholder#86
Conversation
|
@Obiajulu-gif Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
There was a problem hiding this comment.
Pull request overview
This PR scaffolds a pnpm-workspaces monorepo with an initial Next.js (apps/web) app and registers placeholder routes/layouts (notably /dashboard) to unblock future UI integration.
Changes:
- Added pnpm workspace + lockfile, root build tooling, and Turborepo task config.
- Bootstrapped
apps/web(Next.js/React/TS) with root layout, global CSS, and initial pages. - Implemented
/dashboardlayout/page stubs (plus a stub/hotel/[id]detail page) with TODO references tofrontend-SafeTrustcomponents.
Reviewed changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Adds Turborepo task configuration for build caching. |
| scripts/build.mjs | Adds cross-platform build script that builds @safetrust/web. |
| pnpm-workspace.yaml | Defines workspace packages under apps/*. |
| pnpm-lock.yaml | Locks dependencies for reproducible installs. |
| package.json | Adds root package metadata and build script. |
| .gitignore | Ignores node/Next/Turbo build artifacts. |
| apps/web/package.json | Defines Next/React deps and app scripts. |
| apps/web/next.config.mjs | Configures Next build behavior (lint/TS handling). |
| apps/web/tsconfig.json | TypeScript config for Next App Router. |
| apps/web/next-env.d.ts | Next.js TypeScript environment references. |
| apps/web/src/app/layout.tsx | Root layout + basic metadata. |
| apps/web/src/app/globals.css | Base styles + responsive class toggles. |
| apps/web/src/app/page.tsx | Home page linking to placeholder detail route. |
| apps/web/src/app/dashboard/layout.tsx | /dashboard layout with sidebar placeholder. |
| apps/web/src/app/dashboard/page.tsx | /dashboard page with placeholder sections. |
| apps/web/src/app/hotel/[id]/page.tsx | Stubbed apartment detail page UI placeholders. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| throw result.error; | ||
| } | ||
|
|
||
| process.exit(result.status ?? 0); |
There was a problem hiding this comment.
If the child process is terminated by a signal, spawnSync sets status to null and signal to the signal name; the current code exits 0 in that case (result.status ?? 0). Handle result.signal/status === null explicitly (exit non-zero and/or rethrow) so interrupted/terminated builds fail correctly.
| process.exit(result.status ?? 0); | |
| if (result.status === null) { | |
| // Child process was terminated by a signal; fail the build. | |
| if (result.signal) { | |
| console.error(`Build process terminated by signal: ${result.signal}`); | |
| } | |
| process.exit(1); | |
| } | |
| process.exit(result.status); |
| "private": true, | ||
| "packageManager": "pnpm@10.11.0", | ||
| "scripts": { | ||
| "build": "node scripts/build.mjs" |
There was a problem hiding this comment.
The root package.json only defines build, but the repo’s README documents pnpm dev, pnpm test, and other root-level commands and implies Turborepo usage. Either add the missing root scripts (and turbo as a dev dependency if that’s the intended runner) or update the documentation so the documented commands don’t fail.
| "build": "node scripts/build.mjs" | |
| "build": "node scripts/build.mjs", | |
| "dev": "turbo dev", | |
| "test": "turbo test", | |
| "lint": "turbo lint", | |
| "clean": "turbo clean" | |
| }, | |
| "devDependencies": { | |
| "turbo": "^2.0.0" |
| "start": "next start" | ||
| }, | ||
| "dependencies": { | ||
| "next": "14.2.26", |
There was a problem hiding this comment.
next@14.2.26 is flagged in the lockfile as having a known security vulnerability (see the deprecation notice in pnpm-lock.yaml). Please upgrade next to a patched version and regenerate the lockfile so installs/builds don’t rely on a vulnerable release.
| "next": "14.2.26", | |
| "next": "14.2.27", |
| const nextConfig = { | ||
| eslint: { | ||
| ignoreDuringBuilds: true, | ||
| }, | ||
| typescript: { | ||
| ignoreBuildErrors: true, |
There was a problem hiding this comment.
ignoreDuringBuilds and ignoreBuildErrors can allow CI/production builds to succeed even with lint/type errors, which makes failures harder to detect and can ship broken code. Consider removing these, or gating them behind an explicit env flag (e.g., only ignore in local dev) so pnpm build remains a meaningful signal.
| const nextConfig = { | |
| eslint: { | |
| ignoreDuringBuilds: true, | |
| }, | |
| typescript: { | |
| ignoreBuildErrors: true, | |
| const shouldIgnoreBuildIssues = process.env.NEXT_IGNORE_BUILD_ERRORS === 'true'; | |
| const nextConfig = { | |
| eslint: { | |
| ignoreDuringBuilds: shouldIgnoreBuildIssues, | |
| }, | |
| typescript: { | |
| ignoreBuildErrors: shouldIgnoreBuildIssues, |
📝 WalkthroughWalkthroughThis PR establishes a monorepo-based Next.js web application for SafeTrust. It adds configuration files for Next.js, TypeScript, and PNPM monorepo structure, plus a complete app shell with global styling, root layout, home page, dashboard route stubs, and a dynamic apartment detail page with navigation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (4)
.gitignore (1)
3-4: Redundant.gitignoreentry.The pattern
.next/on line 3 already matches.nextdirectories at any depth in the repository, making line 4 (apps/web/.next/) redundant.🧹 Proposed cleanup
node_modules/ .turbo/ .next/ -apps/web/.next/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore around lines 3 - 4, Remove the redundant gitignore entry by deleting the specific pattern "apps/web/.next/" since the existing ".next/" pattern already ignores .next directories at any depth; update the .gitignore so only the global ".next/" entry remains (leave ".next/" and remove "apps/web/.next/").turbo.json (1)
1-9: Turbo configuration is not used by the build script.The
turbo.jsondeclares build task configuration with caching outputs, but the rootpackage.jsoninvokesnode scripts/build.mjswhich directly callspnpm --filter@safetrust/webbuild, completely bypassing Turbo.Consider either:
- Using Turbo for builds: change root build script to
turbo run build- Remove
turbo.jsonif Turbo orchestration is not intendedIf this is intentional scaffolding to integrate Turbo later, a TODO comment would clarify intent.
♻️ Option 1: Wire up Turbo in root package.json
In
package.json:"scripts": { - "build": "node scripts/build.mjs" + "build": "turbo run build" }This would leverage Turbo's caching and task orchestration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@turbo.json` around lines 1 - 9, The turbo.json declares a Turbo "build" task with outputs but the root build flow bypasses Turbo—root package.json's build script runs node scripts/build.mjs which shells out to pnpm --filter `@safetrust/web` build—so either update the root build script to invoke Turbo (e.g., replace the script so it runs turbo run build) to enable Turbo caching/orchestration, or remove turbo.json (or add a TODO comment) if Turbo is not intended; locate the root package.json build script, scripts/build.mjs, and turbo.json to implement the chosen change.apps/web/src/app/globals.css (1)
19-29: Consider avoiding!importantfor responsive visibility.Using
!importantto override inline styles creates specificity issues that become harder to manage as the codebase grows. When integrating the real sidebar/suggestions components, consider using CSS classes or CSS variables for visibility state instead of inline styles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/globals.css` around lines 19 - 29, Remove the use of !important in the media queries for .desktop-suggestions and .dashboard-sidebar and instead implement a visibility state class or CSS variable that can be toggled by the component logic; update the media queries to set the default visibility (e.g., display: block) for larger viewports and rely on a stateful class like .is-visible or a CSS variable (e.g., --sidebar-visible) on the root/component element to control show/hide behavior so inline styles aren't overridden by specificity issues, and ensure the sidebar/suggestions components add/remove that class or update the variable in their rendering logic.apps/web/package.json (1)
4-8: Add explicitlintandtypecheckscripts for CI clarity.The scripts section currently exposes only runtime commands. Adding
lintandtypecheckhere makes quality gates explicit and easier to wire into CI.♻️ Proposed update
"scripts": { "build": "next build", "dev": "next dev", - "start": "next start" + "start": "next start", + "lint": "next lint", + "typecheck": "tsc --noEmit" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/package.json` around lines 4 - 8, Update the "scripts" object in apps/web/package.json to add explicit CI-targeted commands: add a "lint" script that runs ESLint across the repo (e.g., linting .ts/.tsx files) and a "typecheck" script that runs the TypeScript compiler in --noEmit mode (tsc --noEmit). Ensure the new scripts are named "lint" and "typecheck" and that required devDependencies (eslint and typescript/tsconfig) are present so CI can invoke npm run lint and npm run typecheck reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/next.config.mjs`:
- Around line 3-8: The Next.js build config currently silences lint/type errors
via the eslint.ignoreDuringBuilds and typescript.ignoreBuildErrors flags; remove
or set these flags to false in next.config.mjs (or delete those properties) so
build-time ESLint and TypeScript checks run, and add a TODO comment referencing
strict TS in tsconfig.json to track re-enabling if you temporarily need to relax
checks for scaffolding; ensure you update any CI/build scripts accordingly so
they fail on lint/type errors.
In `@apps/web/src/app/hotel/`[id]/page.tsx:
- Around line 21-25: The owner object in the page component contains
real-looking PII; replace the values in the owner object (owner.name,
owner.email, owner.phone) with clearly synthetic placeholders (e.g., "Owner
Name", "owner@example.com", "+1-000-000-0000") or remove the contact fields
entirely so no real personal data remains in the exported placeholder data used
by the page component (look for the owner object in the hotel/[id] page module).
In `@scripts/build.mjs`:
- Line 16: The current process.exit(result.status ?? 0) can mask failures when
result.status is null (e.g., terminated by a signal); change the exit logic to
treat a null status as a failure: if result.status !== null call
process.exit(result.status) else log the termination (include result.signal) and
call process.exit(1); update the code around process.exit and the variables
result.status/result.signal accordingly so builds that were killed or signaled
do not return a success code.
---
Nitpick comments:
In @.gitignore:
- Around line 3-4: Remove the redundant gitignore entry by deleting the specific
pattern "apps/web/.next/" since the existing ".next/" pattern already ignores
.next directories at any depth; update the .gitignore so only the global
".next/" entry remains (leave ".next/" and remove "apps/web/.next/").
In `@apps/web/package.json`:
- Around line 4-8: Update the "scripts" object in apps/web/package.json to add
explicit CI-targeted commands: add a "lint" script that runs ESLint across the
repo (e.g., linting .ts/.tsx files) and a "typecheck" script that runs the
TypeScript compiler in --noEmit mode (tsc --noEmit). Ensure the new scripts are
named "lint" and "typecheck" and that required devDependencies (eslint and
typescript/tsconfig) are present so CI can invoke npm run lint and npm run
typecheck reliably.
In `@apps/web/src/app/globals.css`:
- Around line 19-29: Remove the use of !important in the media queries for
.desktop-suggestions and .dashboard-sidebar and instead implement a visibility
state class or CSS variable that can be toggled by the component logic; update
the media queries to set the default visibility (e.g., display: block) for
larger viewports and rely on a stateful class like .is-visible or a CSS variable
(e.g., --sidebar-visible) on the root/component element to control show/hide
behavior so inline styles aren't overridden by specificity issues, and ensure
the sidebar/suggestions components add/remove that class or update the variable
in their rendering logic.
In `@turbo.json`:
- Around line 1-9: The turbo.json declares a Turbo "build" task with outputs but
the root build flow bypasses Turbo—root package.json's build script runs node
scripts/build.mjs which shells out to pnpm --filter `@safetrust/web` build—so
either update the root build script to invoke Turbo (e.g., replace the script so
it runs turbo run build) to enable Turbo caching/orchestration, or remove
turbo.json (or add a TODO comment) if Turbo is not intended; locate the root
package.json build script, scripts/build.mjs, and turbo.json to implement the
chosen change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f437f022-89b4-49c9-97a6-7ea100599b81
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
pnpm-workspace.yaml.gitignoreapps/web/next-env.d.tsapps/web/next.config.mjsapps/web/package.jsonapps/web/src/app/dashboard/layout.tsxapps/web/src/app/dashboard/page.tsxapps/web/src/app/globals.cssapps/web/src/app/hotel/[id]/page.tsxapps/web/src/app/layout.tsxapps/web/src/app/page.tsxapps/web/tsconfig.jsonpackage.jsonpnpm-workspace.yamlscripts/build.mjsturbo.json
| eslint: { | ||
| ignoreDuringBuilds: true, | ||
| }, | ||
| typescript: { | ||
| ignoreBuildErrors: true, | ||
| }, |
There was a problem hiding this comment.
Ignoring ESLint and TypeScript errors during builds is risky.
Disabling build-time checks allows broken code to pass CI. This contradicts the strict TypeScript configuration in tsconfig.json (which enables strict: true).
Acceptable for initial scaffolding, but should be removed before production to ensure type safety. Consider adding a TODO comment to track this.
📝 Suggested documentation
/** `@type` {import('next').NextConfig} */
const nextConfig = {
+ // TODO: Remove these overrides before production deployment
+ // Currently ignoring errors to allow scaffold to build
eslint: {
ignoreDuringBuilds: true,
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/next.config.mjs` around lines 3 - 8, The Next.js build config
currently silences lint/type errors via the eslint.ignoreDuringBuilds and
typescript.ignoreBuildErrors flags; remove or set these flags to false in
next.config.mjs (or delete those properties) so build-time ESLint and TypeScript
checks run, and add a TODO comment referencing strict TS in tsconfig.json to
track re-enabling if you temporarily need to relax checks for scaffolding;
ensure you update any CI/build scripts accordingly so they fail on lint/type
errors.
| owner: { | ||
| name: 'Alberto Casas', | ||
| email: 'albertoCasas100@gmail.com', | ||
| phone: '+506 64852179', | ||
| }, |
There was a problem hiding this comment.
Remove real-looking personal contact details from placeholder data.
Lines 21–25 include concrete-looking PII in a shipped page module. Replace with clearly synthetic placeholders (or omit contact fields) to avoid privacy/compliance risk.
🛡️ Proposed update
owner: {
- name: 'Alberto Casas',
- email: 'albertoCasas100@gmail.com',
- phone: '+506 64852179',
+ name: 'Sample Owner',
+ email: 'owner@example.com',
+ phone: '+0000000000',
},📝 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.
| owner: { | |
| name: 'Alberto Casas', | |
| email: 'albertoCasas100@gmail.com', | |
| phone: '+506 64852179', | |
| }, | |
| owner: { | |
| name: 'Sample Owner', | |
| email: 'owner@example.com', | |
| phone: '+0000000000', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/app/hotel/`[id]/page.tsx around lines 21 - 25, The owner object
in the page component contains real-looking PII; replace the values in the owner
object (owner.name, owner.email, owner.phone) with clearly synthetic
placeholders (e.g., "Owner Name", "owner@example.com", "+1-000-000-0000") or
remove the contact fields entirely so no real personal data remains in the
exported placeholder data used by the page component (look for the owner object
in the hotel/[id] page module).
| throw result.error; | ||
| } | ||
|
|
||
| process.exit(result.status ?? 0); |
There was a problem hiding this comment.
Do not default to success when the build process has no exit status.
Line 16 can return exit code 0 when result.status is null (for example, signal termination), which can falsely mark failed builds as successful.
🐛 Proposed fix
-process.exit(result.status ?? 0);
+if (result.signal) {
+ process.exit(1);
+}
+
+process.exit(result.status ?? 1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build.mjs` at line 16, The current process.exit(result.status ?? 0)
can mask failures when result.status is null (e.g., terminated by a signal);
change the exit logic to treat a null status as a failure: if result.status !==
null call process.exit(result.status) else log the termination (include
result.signal) and call process.exit(1); update the code around process.exit and
the variables result.status/result.signal accordingly so builds that were killed
or signaled do not return a success code.
This pull request sets up a new Next.js monorepo project for SafeTrust using pnpm workspaces, including initial configuration, package management, and placeholder UI for the web application. The changes establish the project structure, dependencies, and basic application pages with stubs for future development.
Monorepo and Package Management Setup:
pnpm-workspace.yamlto manage packages under theapps/directory.package.jsonwith a build script delegating to a custom build script, and set up pnpm as the package manager.pnpm-lock.yamlfile to lock dependencies and ensure reproducible installs across environments.Web App Bootstrapping (apps/web):
apps/web/package.jsonwith Next.js, React, and TypeScript dependencies, and standard scripts for building and running the app.next.config.mjs) to ignore ESLint and TypeScript errors during builds for easier development.tsconfig.json) tailored for Next.js apps.Build and Development Tooling:
scripts/build.mjs) that runs the Next.js build for the web app using pnpm filters.Initial UI and Placeholder Pages:
globals.css) with base styles and responsive sidebar/suggestions visibility.layout.tsx) and metadata for the web app.page.tsx) with a link to the apartment detail placeholder.hotel/[id]/page.tsx) with static data, layout, and placeholders for suggestions, image gallery, amenities, and booking actions. (apps/web/src/app/hotel/[id]/page.tsxR1-R189)These changes provide a solid foundation for further SafeTrust web development, with monorepo management, basic app structure, and clear TODOs for future component integration.
Closes #67
Summary by CodeRabbit
New Features
Chores