fix: address missing hook dependencies to remove exhaustive-deps disables#447
fix: address missing hook dependencies to remove exhaustive-deps disables#447narutamaaurum wants to merge 0 commit into
Conversation
WalkthroughThis PR addresses missing React hook dependencies across three components by removing ChangesHook Dependencies Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
👋 Thanks for opening a PR, @narutamaaurum!Your PR has entered the 🚦 PR Review Pipeline.
What happens next
A pipeline status comment will appear below and update automatically as your PR progresses. While you wait
This comment is posted only once. |
Review Summary by QodoFix missing React hook dependencies and remove eslint-disable comments
WalkthroughsDescription• Remove eslint-disable comments by fixing missing hook dependencies • Wrap updateEditorLanguage in React.useCallback with proper dependencies • Move highlight and highlightCode functions to module scope • Ensure hook dependency arrays are exhaustive and accurate Diagramflowchart LR
A["Identify missing dependencies"] --> B["playground-editor.tsx: useCallback wrapper"]
A --> C["code-line.tsx: Move functions outside"]
A --> D["terminal.tsx: Remove unnecessary disables"]
B --> E["Remove eslint-disable comments"]
C --> E
D --> E
E --> F["Exhaustive-deps compliance"]
File Changes1. modules/home/code-line.tsx
|
Code Review by Qodo
Context used✅ Tickets:
🎫 refactor(react): address missing hook dependencies to remove exhaustive-deps eslint disables✅ Compliance rules (platform):
22 rules 1. CodeLine uses dangerouslySetInnerHTML
|
| if (highlighted.includes('//')) { | ||
| const parts = highlighted.split('//'); | ||
| return <><span dangerouslySetInnerHTML={{ __html: highlightCode(parts[0]) }} /><span className="text-slate-500 italic">{'//' + parts[1]}</span></>; | ||
| } | ||
| return <span dangerouslySetInnerHTML={{ __html: highlightCode(highlighted) }} />; |
There was a problem hiding this comment.
1. codeline uses dangerouslysetinnerhtml 📘 Rule violation ⛨ Security
CodeLine renders line via dangerouslySetInnerHTML using highlightCode() output that is not HTML-escaped or sanitized, enabling XSS if line can contain attacker-controlled markup. This can lead to script execution in the browser.
Agent Prompt
## Issue description
`CodeLine` uses `dangerouslySetInnerHTML` with a string produced from raw `line` content without escaping/sanitization, which can enable XSS.
## Issue Context
The component builds HTML strings via `.replace(...)` and injects them with `dangerouslySetInnerHTML`. If `line` ever contains `<`, `>`, event handlers, or tags, they can be interpreted as HTML.
## Fix Focus Areas
- modules/home/code-line.tsx[5-20]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| writePrompt(); | ||
|
|
||
| return terminal; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [theme, handleTerminalInput, writePrompt]); | ||
|
|
There was a problem hiding this comment.
2. terminal.tsx exceeds 500 lines 📘 Rule violation ⚙ Maintainability
modules/webcontainers/components/terminal.tsx is over the 500-line maximum allowed for source files. Oversized files reduce maintainability and make review/auditing harder.
Agent Prompt
## Issue description
`modules/webcontainers/components/terminal.tsx` exceeds the 500-line limit and must be refactored into smaller cohesive modules.
## Issue Context
The current file length reaches at least line 550, violating the 500-line cap.
## Fix Focus Areas
- modules/webcontainers/components/terminal.tsx[1-550]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const updateEditorLanguage = React.useCallback(() => { | ||
| if (!activeFile || !monacoRef.current || !editorRef.current) return; | ||
| const model = editorRef.current.getModel(); | ||
| if (!model) return; |
There was a problem hiding this comment.
3. Undefined react reference 🐞 Bug ≡ Correctness
modules/playground/components/playground-editor.tsx now calls React.useCallback but the file does
not import React (nor declares a global React), causing a compile/runtime failure ("React is not
defined"). This breaks PlaygroundEditor rendering/bundling.
Agent Prompt
### Issue description
`React.useCallback` is referenced, but `React` is not in scope in this module (it only imports named hooks). This will fail compilation/bundling.
### Issue Context
The file currently imports hooks via `import { useRef, useEffect, useState } from "react";` and later uses `React.useCallback`.
### Fix Focus Areas
- modules/playground/components/playground-editor.tsx[3-6]
- modules/playground/components/playground-editor.tsx[256-271]
### Suggested fix
Use one of the following patterns:
1) Prefer named hook import:
- Change the import to `import { useRef, useEffect, useState, useCallback } from "react";`
- Change `React.useCallback(...)` to `useCallback(...)`
2) Or import React namespace:
- Add `import React from "react";` (or `import * as React from "react";`) and keep `React.useCallback(...)`
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/playground/components/playground-editor.tsx (1)
256-267:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
React.useCallbackused without importingReact
Inmodules/playground/components/playground-editor.tsx(around lines 256-267),updateEditorLanguagecallsReact.useCallback, but React is only imported as named hooks (import { useRef, useEffect, useState } from "react";). This makesReactundefined (runtimeReferenceError/ TS error).🔧 Proposed fix (choose one option)
Option 1: Import React default
-import { useRef, useEffect, useState } from "react"; +import React, { useRef, useEffect, useState } from "react";Option 2: Import
useCallbackdirectly (recommended)-import { useRef, useEffect, useState } from "react"; +import { useRef, useEffect, useState, useCallback } from "react";Then update the callback:
- const updateEditorLanguage = React.useCallback(() => { + const updateEditorLanguage = useCallback(() => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/playground/components/playground-editor.tsx` around lines 256 - 267, The file uses React.useCallback inside updateEditorLanguage but React isn't imported as a default, causing a runtime/TS error; fix by importing the hook directly (add useCallback to the existing named imports) or alternatively import default React and leave React.useCallback; update the import that currently has useRef/useEffect/useState to include useCallback so updateEditorLanguage can call useCallback without errors.
🧹 Nitpick comments (2)
modules/home/code-line.tsx (2)
14-14: 💤 Low valueOptional: Remove redundant variable assignment.
The assignment
const highlighted = text;is redundant sincehighlightedis simply an alias fortext. You can usetextdirectly in the subsequent logic.♻️ Proposed simplification
const highlight = (text: string) => { - const highlighted = text; - if (highlighted.includes('//')) { - const parts = highlighted.split('//'); + if (text.includes('//')) { + const parts = text.split('//'); return <><span dangerouslySetInnerHTML={{ __html: highlightCode(parts[0]) }} /><span className="text-slate-500 italic">{'//' + parts[1]}</span></>; } - return <span dangerouslySetInnerHTML={{ __html: highlightCode(highlighted) }} />; + return <span dangerouslySetInnerHTML={{ __html: highlightCode(text) }} />; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/home/code-line.tsx` at line 14, The variable assignment const highlighted = text; in the CodeLine component is redundant; remove that declaration and replace uses of highlighted with text directly (e.g., in any map/JSX where highlighted is referenced) so the component uses the original prop/variable name; ensure no other logic relies on a separate highlighted variable and adjust any references in modules/home/code-line.tsx accordingly.
13-20: ⚡ Quick winConsider using a proper syntax highlighting library to avoid XSS risks.
The static analysis correctly flags the use of
dangerouslySetInnerHTMLwith unsanitized content. While the simple regex replacements appear safe for controlled code strings, this pattern bypasses React's XSS protection. If thelineprop could ever contain untrusted user input, this creates a security vulnerability.Consider using a dedicated syntax highlighting library (e.g., Prism, highlight.js) or sanitizing HTML with DOMPurify before injection. As per coding guidelines, CWE-79: Improper Neutralization of Input During Web Page Generation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/home/code-line.tsx` around lines 13 - 20, The highlight function uses dangerouslySetInnerHTML with highlightCode output (in highlight and the JSX spans), which risks XSS; replace this by removing dangerouslySetInnerHTML and either (a) integrate a proper syntax-highlighting library (e.g., Prism or highlight.js) to render tokens safely in the highlight function (call the library on the code string and render tokens as React elements), or (b) if you must keep HTML output from highlightCode, sanitize it before injection using DOMPurify and then use dangerouslySetInnerHTML on the sanitized result; update the highlight function and any calls to highlightCode to return/supply safe content and ensure comment-splitting logic (the parts = highlighted.split('//')) still renders the comment span without raw HTML injection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@modules/playground/components/playground-editor.tsx`:
- Around line 256-267: The file uses React.useCallback inside
updateEditorLanguage but React isn't imported as a default, causing a runtime/TS
error; fix by importing the hook directly (add useCallback to the existing named
imports) or alternatively import default React and leave React.useCallback;
update the import that currently has useRef/useEffect/useState to include
useCallback so updateEditorLanguage can call useCallback without errors.
---
Nitpick comments:
In `@modules/home/code-line.tsx`:
- Line 14: The variable assignment const highlighted = text; in the CodeLine
component is redundant; remove that declaration and replace uses of highlighted
with text directly (e.g., in any map/JSX where highlighted is referenced) so the
component uses the original prop/variable name; ensure no other logic relies on
a separate highlighted variable and adjust any references in
modules/home/code-line.tsx accordingly.
- Around line 13-20: The highlight function uses dangerouslySetInnerHTML with
highlightCode output (in highlight and the JSX spans), which risks XSS; replace
this by removing dangerouslySetInnerHTML and either (a) integrate a proper
syntax-highlighting library (e.g., Prism or highlight.js) to render tokens
safely in the highlight function (call the library on the code string and render
tokens as React elements), or (b) if you must keep HTML output from
highlightCode, sanitize it before injection using DOMPurify and then use
dangerouslySetInnerHTML on the sanitized result; update the highlight function
and any calls to highlightCode to return/supply safe content and ensure
comment-splitting logic (the parts = highlighted.split('//')) still renders the
comment span without raw HTML injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b98fc01a-4297-40a0-9052-b7f34481e153
📒 Files selected for processing (3)
modules/home/code-line.tsxmodules/playground/components/playground-editor.tsxmodules/webcontainers/components/terminal.tsx
💤 Files with no reviewable changes (1)
- modules/webcontainers/components/terminal.tsx
|
I've addressed the review comments in a new PR: #480 Fixes Applied
Build passes successfully. Ready for review. |
71a9d1b to
45f9eb3
Compare
Changes
Resolves missing hook dependencies as described in #442.
1.
modules/webcontainers/components/terminal.tsxeslint-disable-next-line react-hooks/exhaustive-depscommentsterminalRef,term) don't need to be listed2.
modules/playground/components/playground-editor.tsxupdateEditorLanguageinReact.useCallbackwith[activeFile]dependencyupdateEditorLanguageto the useEffect dependency array3.
modules/home/code-line.tsxhighlightandhighlightCodefunctions outside the component bodyCloses #442
Summary by CodeRabbit
Refactor
Chores