Skip to content

Conversation

@levtechs
Copy link
Owner

Ran this prompt:

fix lint errors such as no unused variables

@vercel
Copy link

vercel bot commented Oct 27, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
coilearn Ready Ready Preview Comment Oct 27, 2025 7:46pm

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @levtechs, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the codebase's quality and performance by systematically eliminating lint warnings, primarily focusing on unused variables and imports. Beyond mere cleanup, it introduces performance optimizations through React hook refactoring and a crucial upgrade to Next.js's <Image> component for improved image loading across key UI elements. The changes contribute to a more efficient and maintainable application.

Highlights

  • Lint Warning Resolution: The primary focus of this pull request is to address and resolve various lint warnings, particularly those related to unused variables and imports across the codebase. This improves code cleanliness and reduces potential for errors.
  • Code Optimization: Several React components have been optimized by wrapping functions in useCallback and updating useEffect dependency arrays, enhancing performance and preventing unnecessary re-renders.
  • Image Optimization: Standard <img> tags have been migrated to Next.js's <Image> component in card_popup.tsx, detail_card.tsx, and study_card.tsx. This change leverages Next.js's built-in image optimization features for better loading performance and user experience.
  • Unused Code Removal: Unused imports (e.g., arrayUnion, deleteDoc, useState, various Fi icons) and unused variables/props (e.g., setIsCreatingQuiz, result, setProject, isOwner, handleCardClick) have been removed from multiple files, streamlining the codebase.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request, aimed at fixing lint warnings, successfully removes many unused variables and imports, which cleans up the code. The migration from <img> tags to Next.js's <Image> component is also a significant improvement for performance. However, in a couple of instances, the automated removal of type imports has weakened type safety for code that relies on implicit type inference. I've added comments to restore these imports. I also found an opportunity to simplify the code by replacing an unnecessary state variable with a constant.

import QuizSettingsComponent from "./quiz_settings";
import LessonComponent from "./edit_lesson";
import { CourseLesson, Card, NewCard, Course, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
import { Card, NewCard, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The updateCourse function expects its second argument to be of type Omit<Course, "id">. Although the courseData object is not explicitly typed, TypeScript relies on the imported Course and CourseLesson types for structural compatibility checking and type inference. Removing these imports can lead to subtle bugs if the Course or CourseLesson types change in the future, as type checking will be less strict. It's better to keep the imports for type safety.

Suggested change
import { Card, NewCard, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
import { CourseLesson, Card, NewCard, Course, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";

const [selectedCards, setSelectedCards] = useState<boolean[]>(lesson.cardsToUnlock.map(() => true));
const [lessonQuizzes, setLessonQuizzes] = useState<{id?: string, status: 'creating' | 'created', title?: string}[]>(lesson.quizIds?.map(id => ({id, status: 'created'})) || []);
const [isCreatingQuiz, setIsCreatingQuiz] = useState<boolean | string>(false);
const [isCreatingQuiz] = useState<boolean | string>(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The state variable isCreatingQuiz is initialized with useState but its value is never updated. It can be simplified to a constant. This improves readability and removes unnecessary state management.

Suggested change
const [isCreatingQuiz] = useState<boolean | string>(false);
const isCreatingQuiz = false;

import { getQuiz } from "../../views/quiz";
import { createCourseInvitation } from "../../views/invite";
import { Course, CourseLesson, Project, Quiz } from "@/lib/types";
import { Course, Project, Quiz } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The CourseLesson type is used implicitly. For example, when iterating over course.lessons, the type of each lesson is inferred as CourseLesson. Removing the import weakens type safety and can make the code harder to maintain, especially if the CourseLesson type definition changes. It's best to keep the import to ensure proper type checking.

Suggested change
import { Course, Project, Quiz } from "@/lib/types";
import { Course, CourseLesson, Project, Quiz } from "@/lib/types";

@levtechs
Copy link
Owner Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to fix lint warnings, and while it successfully removes many unused variables and imports, it also introduces some critical issues. Several files have had type imports removed (Course, CourseLesson) that are still required due to being part of other imported types' structures. This will likely cause the build to fail. Additionally, a change from <img> to <Image> in one component has altered the layout by introducing a fixed height for images, which may be an unintended side effect. I've left comments on the specific files with these issues.

import QuizSettingsComponent from "./quiz_settings";
import LessonComponent from "./edit_lesson";
import { CourseLesson, Card, NewCard, Course, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
import { Card, NewCard, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The imports for Course and CourseLesson have been removed. However, Course is used as part of the return type for the getCourse function, and its structure (which includes CourseLesson) is used when calling updateCourse. Removing these imports will likely cause a build error.

Suggested change
import { Card, NewCard, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
import { CourseLesson, Card, NewCard, Course, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";

import { getQuiz } from "../../views/quiz";
import { createCourseInvitation } from "../../views/invite";
import { Course, CourseLesson, Project, Quiz } from "@/lib/types";
import { Course, Project, Quiz } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The import for CourseLesson has been removed. This type is implicitly used as it's part of the Course type structure, specifically course.lessons which is an array of CourseLesson. Removing this import will likely cause a build error.

Suggested change
import { Course, Project, Quiz } from "@/lib/types";
import { Course, CourseLesson, Project, Quiz } from "@/lib/types";

import { auth } from "@/lib/firebase";

import { Course, CourseLesson, NewCard, Project, NewLesson, QuizSettings, CommentTree, CreateCommentData, UpdateCommentData } from "@/lib/types";
import { Course, Project, NewLesson, QuizSettings, CommentTree, CreateCommentData, UpdateCommentData } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The imports for CourseLesson and NewCard have been removed. However, CourseLesson is a dependency for the Course type (used in updateCourse), and NewCard is a dependency for the NewLesson type. Removing these will likely cause a build error. They should be re-added to the import statement.

Suggested change
import { Course, Project, NewLesson, QuizSettings, CommentTree, CreateCommentData, UpdateCommentData } from "@/lib/types";
import { Course, CourseLesson, NewCard, Project, NewLesson, QuizSettings, CommentTree, CreateCommentData, UpdateCommentData } from "@/lib/types";

import { db } from "@/lib/firebase";
import { getVerifiedUid } from "../../../../helpers";
import { doc, getDoc, updateDoc, deleteDoc, arrayUnion, arrayRemove, Timestamp } from "firebase/firestore";
import { doc, getDoc, updateDoc, deleteDoc, Timestamp } from "firebase/firestore";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The imports for arrayUnion and arrayRemove were removed. While they are not used in this file, the logic for updating votes in app/api/courses/[courseId]/comments/[commentId]/vote/route.ts was reimplemented manually using filter and push. Using arrayUnion and arrayRemove from Firestore is generally more idiomatic and can be more efficient for managing array membership in documents. Since this PR is about linting, this change is out of scope, but it's worth considering if this was an intentional refactor.

@@ -1,4 +1,4 @@
import { Course, CourseLesson, NewCard, NewCourse, NewLesson, QuizSettings } from "@/lib/types";
import { NewCard, NewCourse, NewLesson, QuizSettings } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The imports for Course and CourseLesson have been removed. While not directly used as type annotations in this file, they are used in the definition of NewCourse and NewLesson within lib/types.ts. Depending on the TypeScript configuration, their removal might lead to build errors related to exposing types from imported modules. It's safer to keep these imports if they are part of the public API of types used here.

Suggested change
import { NewCard, NewCourse, NewLesson, QuizSettings } from "@/lib/types";
import { Course, CourseLesson, NewCard, NewCourse, NewLesson, QuizSettings } from "@/lib/types";

const [selectedCards, setSelectedCards] = useState<boolean[]>(lesson.cardsToUnlock.map(() => true));
const [lessonQuizzes, setLessonQuizzes] = useState<{id?: string, status: 'creating' | 'created', title?: string}[]>(lesson.quizIds?.map(id => ({id, status: 'created'})) || []);
const [isCreatingQuiz, setIsCreatingQuiz] = useState<boolean | string>(false);
const [isCreatingQuiz] = useState<boolean | string>(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The state setter setIsCreatingQuiz has been removed, but the state isCreatingQuiz is still passed as a prop to QuizSettingsComponent. While setIsCreatingQuiz was unused in this component, it seems the intention was to manage a loading state for quiz creation. If this loading state is still desired, the setter should be used. If not, the isCreatingQuiz state and its prop passing could be removed entirely for cleaner code.

Comment on lines 83 to 93
<div key={index} className="relative w-full h-48">
<Image
src={url}
alt={`Reference image ${index + 1}`}
fill
className="rounded-lg object-scale-down"
onError={(e) => {
(e.target as HTMLImageElement).style.display = 'none';
}}
/>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While replacing <img> with <Image> is a good optimization, this change also replaces h-auto with a fixed height of h-48. This will alter the layout, as images will now be constrained to a fixed height container, which may not have been the original intent. The original h-auto allowed images to maintain their aspect ratio while filling the container's width. This change could lead to images being scaled down more than intended or having unexpected empty space.

Comment on lines 51 to 52

const handleCardClick = (card: Card, position: number) => {
if (position === 0) {
setSelectedCard(card);
} else if (position === -1) {
setCurrentSlot(currentSlot - 1);
} else if (position === 1) {
next();
}
};

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The handleCardClick function was removed. However, the onClick handlers for the study cards now contain duplicated logic that was previously in handleCardClick. It would be more maintainable to restore handleCardClick and call it from the onClick handlers.

@levtechs
Copy link
Owner Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a number of lint warnings, primarily by removing unused variables and imports across the codebase. The changes are generally correct and improve code cleanliness. Additionally, several <img> tags have been updated to use Next.js's <Image> component, which is a good performance enhancement.

I've left a few comments on specific areas:

  • A potential race condition in the voting logic that should be addressed with atomic operations.
  • A security concern with the wildcard hostname configuration for next/image.
  • A couple of places where removed type imports should be restored to improve code clarity and maintainability.

Overall, good work on cleaning up the code. Addressing the feedback will further improve the application's robustness and security.

import { NextRequest, NextResponse } from "next/server";
import { db } from "@/lib/firebase";
import { getVerifiedUid } from "../../../../../helpers";
import { doc, getDoc, updateDoc, arrayUnion, arrayRemove } from "firebase/firestore";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

These imports were correctly removed as they are unused. However, the current implementation for vote updates is susceptible to a race condition because it's not atomic. It reads the vote arrays, modifies them, and writes them back. This can lead to lost votes under concurrent requests.

To fix this, you should use Firestore's atomic arrayUnion and arrayRemove operations. This would require re-adding these imports and refactoring the logic.

remotePatterns: [
{
protocol: 'https',
hostname: '**',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using a wildcard ** for the hostname in remotePatterns allows images from any domain. This can be a security risk, as it makes your application vulnerable to image-based attacks from malicious sources. It's highly recommended to specify a list of trusted hostnames instead of using a wildcard.

import QuizSettingsComponent from "./quiz_settings";
import LessonComponent from "./edit_lesson";
import { CourseLesson, Card, NewCard, Course, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
import { Card, NewCard, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The types Course and CourseLesson were removed, but they seem to be used implicitly in this file. For example, the return type of getCourse uses Course, and its lessons property is an array of CourseLesson. While TypeScript might infer these types, explicitly importing them improves code clarity and maintainability. I suggest re-adding them.

Suggested change
import { Card, NewCard, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";
import { CourseLesson, Card, NewCard, Course, NewCourse, QuizSettings, CourseCategory, NewLesson } from "@/lib/types";

import { getQuiz } from "../../views/quiz";
import { createCourseInvitation } from "../../views/invite";
import { Course, CourseLesson, Project, Quiz } from "@/lib/types";
import { Course, Project, Quiz } from "@/lib/types";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type CourseLesson was removed, but it appears to be used implicitly when iterating over course.lessons. To improve code clarity and maintainability, it's better to keep the explicit import.

Suggested change
import { Course, Project, Quiz } from "@/lib/types";
import { Course, CourseLesson, Project, Quiz } from "@/lib/types";

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant