fix: stabilize project with tests, linting, and bug fixes#10
fix: stabilize project with tests, linting, and bug fixes#10ViVaLaDaniel wants to merge 1 commit intomainfrom
Conversation
Co-authored-by: ViVaLaDaniel <110051738+ViVaLaDaniel@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe pull request establishes testing and linting infrastructure by adding ESLint and Vitest configurations with necessary dependencies. It also updates Firebase configuration to use environment variables, refactors Firebase document reference construction patterns, adds unit tests, and makes minor UI adjustments. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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: 1
🤖 Fix all issues with AI agents
In @.eslintrc.json:
- Around line 1-3: The ESLint config currently extends only
"next/core-web-vitals"; for a TypeScript Next.js project update the "extends"
entry in .eslintrc.json to include both "next/core-web-vitals" and
"next/typescript" (i.e., change the extends value to an array containing those
two presets) so TypeScript-specific lint rules are applied by Next's presets.
🧹 Nitpick comments (2)
src/firebase/config.ts (1)
1-8: Fail fast when required Firebase env vars are missing.
This avoids obscure runtime errors when any value is undefined.♻️ Suggested fail-fast helper
+const requiredEnv = (name: string) => { + const value = process.env[name]; + if (!value) { + throw new Error(`Missing environment variable: ${name}`); + } + return value; +}; + export const firebaseConfig = { - projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID, - appId: process.env.NEXT_PUBLIC_FIREBASE_APP_ID, - apiKey: process.env.NEXT_PUBLIC_FIREBASE_API_KEY, - authDomain: process.env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN, - measurementId: process.env.NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID, - messagingSenderId: process.env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID, + projectId: requiredEnv("NEXT_PUBLIC_FIREBASE_PROJECT_ID"), + appId: requiredEnv("NEXT_PUBLIC_FIREBASE_APP_ID"), + apiKey: requiredEnv("NEXT_PUBLIC_FIREBASE_API_KEY"), + authDomain: requiredEnv("NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN"), + measurementId: requiredEnv("NEXT_PUBLIC_FIREBASE_MEASUREMENT_ID"), + messagingSenderId: requiredEnv("NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID"), };src/lib/seed-db.ts (1)
1-37: Use the directdoc(firestore, path, id)pattern instead ofdoc(collection(), id)for better clarity.While
doc(collection(firestore, path), id)is valid, the Firebase v11+ modular API recommends the more direct pattern:doc(firestore, path, id). For the municipalities collection, usedoc(firestore, "municipalities", municipality.id). For the user-specific permits subcollection, usedoc(firestore, "users", userId, "permits", permit.id).
| { | ||
| "extends": "next/core-web-vitals" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Next.js 15 eslint next/core-web-vitals preset configuration requirements
💡 Result:
Next.js 15: next/core-web-vitals ESLint preset — what you need to configure
1) Install the required packages
You need eslint and eslint-config-next in your project (Next can also add them for you when you first run next lint). [1]
2) Choose the config format you’re using
A) Legacy .eslintrc* (extends string)
Use this if you’re on the classic ESLint config format:
{
"extends": "next/core-web-vitals"
}This enables the Core Web Vitals ruleset (it promotes some Next.js lint rules from warnings to errors). [2]
B) ESLint “flat config” (eslint.config.mjs)
In Next.js 15 docs, the flat-config way is to import the preset from eslint-config-next/core-web-vitals and export it:
// eslint.config.mjs
import { defineConfig, globalIgnores } from 'eslint/config'
import nextVitals from 'eslint-config-next/core-web-vitals'
export default defineConfig([
...nextVitals,
// (optional) override ignores
globalIgnores(['.next/**', 'out/**', 'build/**', 'next-env.d.ts']),
])This is the documented setup flow for using the core-web-vitals preset with flat config. [1]
3) (Optional) Combine with TypeScript preset
If you want the Next.js TypeScript ESLint add-on, extend/include next/typescript alongside next/core-web-vitals. [2]
🌐 Web query:
ESLint TypeScript parser configuration for Next.js projects
💡 Result:
Recommended (Next.js-managed parser; most projects)
In Next.js projects you typically do not set parser yourself—eslint-config-next already wires up the right parser/plugins/settings. Use the Next presets and (if you want TS rules) add the TS preset too. [1]
Flat config (eslint.config.mjs)
import { defineConfig } from "eslint/config";
import nextVitals from "eslint-config-next/core-web-vitals";
import nextTs from "eslint-config-next/typescript";
export default defineConfig([
...nextVitals,
...nextTs,
]);This is the “Next + TypeScript” setup Create Next App uses conceptually (Core Web Vitals + TS rules). [1]
Legacy config (.eslintrc.*)
{
"extends": ["next/core-web-vitals", "next/typescript"]
}next/typescript is based on plugin:@typescript-eslint/recommended``. [2]
If you specifically need typed linting (rules that require type info)
Configure @typescript-eslint/parser with type-aware parser options (modern recommendation: parserOptions.projectService: true). [3]
import tsParser from "@typescript-eslint/parser";
export default [
{
files: ["**/*.{ts,tsx}"],
languageOptions: {
parser: tsParser,
parserOptions: {
projectService: true
}
}
}
];Tip: Only do this when you actually enable type-aware rulesets; otherwise it can slow linting and complicate config. [3]
Sources: Next.js ESLint config docs [1], Next.js “With TypeScript” (next/typescript) [2], typescript-eslint parser typed-linting options [3].
Solid base ESLint preset for Next.js, but incomplete for TypeScript projects.
The "next/core-web-vitals" preset includes Next.js specific rules for Core Web Vitals compliance (promoting certain warnings to errors). However, if this is a TypeScript project, extend both presets:
{
"extends": ["next/core-web-vitals", "next/typescript"]
}No additional parserOptions or root fields are required—the Next.js presets handle parser configuration automatically. Add custom parser options only if you need type-aware linting (which requires projectService: true on @typescript-eslint/parser).
🤖 Prompt for AI Agents
In @.eslintrc.json around lines 1 - 3, The ESLint config currently extends only
"next/core-web-vitals"; for a TypeScript Next.js project update the "extends"
entry in .eslintrc.json to include both "next/core-web-vitals" and
"next/typescript" (i.e., change the extends value to an array containing those
two presets) so TypeScript-specific lint rules are applied by Next's presets.
1 participant
This pull request addresses several critical issues and improves the project infrastructure:
Project Purpose & Status:
Changes Implemented:
src/firebase/config.tsto environment variables (.env.local.examplecreated).src/lib/seed-db.tsto correctly usedoc()from the Firebase Modular SDK.src/components/ui/calendar.tsxtype errors by removing incompatible overrides forreact-day-pickerv9.src/app/(app)/dashboard/page.tsx.vitestand configuredvitest.config.mts.testscript and a sample test (src/lib/utils.test.ts).eslintproperly.Remaining Work (What else needs to be done):
src/app/(app)/code-validator/validator-form.tsx) currently mocks building codes. It needs real RAG implementation to fetch actual local codes.PR created automatically by Jules for task 13903886953341354066 started by @ViVaLaDaniel
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.