Skip to content

fix: address review comments on PR #447 #480

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.

Sign up for GitHub

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

Closed
narutamaaurum wants to merge 3 commits into from
+28 −37
Closed

fix: address review comments on PR #447 #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71a9d1b
fix: address missing hook dependencies to remove exhaustive-deps disa…
narutamaaurum Jun 1, 2026
5473a53
fix: address review comments on PR #447
narutamaaurum Jun 8, 2026
af940f8
fix: use useMemo instead of useState+useEffect for derived state in C…
narutamaaurum Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 24 additions & 30 deletions modules/home/code-line.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,33 @@
"use client";
import React from 'react';

export const CodeLine = ({ line }: { line: string }) => {
// Basic replacements to simulate syntax highlighting
// Note: This is a very simplistic implementation and should be replaced with a proper library like prismjs or shiki in production

const highlight = (text: string) => {
// We use a series of replacements. Order matters to avoid replacing inside already replaced spans.
// A better approach for robust highlighting is tokenization.

const highlighted = text;

// Comments (simple // for now)
if (highlighted.includes('//')) {
const parts = highlighted.split('//');
return <><span dangerouslySetInnerHTML={{ __html: highlightCode(parts[0]) }} /><span className="text-slate-500 italic">{'//' + parts[1]}</span></>;
}
const escapeHtml = (text: string) => {
return text
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#039;");
Comment on lines +5 to +11

@coderabbitai coderabbitai Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the file in question
FILE="modules/home/code-line.tsx"
echo "=== File exists? ==="
ls -la "$FILE" || true

echo "=== Lines 1-120 (for context) ==="
nl -ba "$FILE" | sed -n '1,160p'

echo "=== Find escapeHtml / highlightCode / highlight / dangerouslySetInnerHTML usages ==="
rg -n "escapeHtml|highlightCode|highlight\(|dangerouslySetInnerHTML|split\\(.*//|useState|useEffect" "$FILE" || true

echo "=== Extract suspected string/token regexes (around where comment references lines 18-19) ==="
# Print a small window around line numbers referenced by the review comment.
nl -ba "$FILE" | sed -n '10,60p'

echo "=== If present, inspect any string regex patterns ==="
rg -n --hidden --no-ignore -S "['\"]|string|literal|regex|match|replace\\(\\/.*\\/" "$FILE" || true

Repository: piyushdotcomm/Editron

Length of output: 258

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="modules/home/code-line.tsx"
echo "=== File exists? ==="
ls -la "$FILE" || true

echo "=== Lines 1-180 (context) ==="
cat -n "$FILE" | sed -n '1,180p'

echo "=== Find key symbols/usages ==="
rg -n "escapeHtml|highlightCode|highlight\\(|dangerouslySetInnerHTML|split\\(|useState|useEffect" "$FILE" || true

echo "=== Extract escapeHtml function body (best-effort) ==="
# Print around the first occurrence of escapeHtml
LINE=$(rg -n "escapeHtml" "$FILE" | head -n 1 | cut -d: -f1 || true)
if [[ -n "${LINE:-}" ]]; then
  START=$((LINE-10))
  END=$((LINE+40))
  (( START<1 )) && START=1
  cat -n "$FILE" | sed -n "${START},${END}p"
fi

echo "=== Extract highlightCode/highlight usage sections ==="
rg -n "highlightCode|highlight\\(" "$FILE" || true

for pat in "highlightCode" "highlight(" "dangerouslySetInnerHTML" "split\\(" "useEffect" "useState"; do
  echo "=== Around first match of: $pat ==="
  LINE=$(rg -n "$pat" "$FILE" | head -n 1 | cut -d: -f1 || true)
  if [[ -z "${LINE:-}" ]]; then
    echo "No match for $pat"
    continue
  fi
  START=$((LINE-15))
  END=$((LINE+60))
  (( START<1 )) && START=1
  (( END>9999 )) && END=9999
  cat -n "$FILE" | sed -n "${START},${END}p"
done

Repository: piyushdotcomm/Editron

Length of output: 11559

Quote escaping breaks string token highlighting.

highlightCode escapes the input via escapeHtml (turning " into &quot; and ' into &#39;) before applying the string regexes (/'[^']*'/g and /"[^"]*"/g`), so quoted literals never match and don’t get highlighted.

Proposed fix 
 const escapeHtml = (text: string) => {
     return text
         .replace(/&/g, "&amp;")
         .replace(/</g, "&lt;")
-        .replace(/>/g, "&gt;")
-        .replace(/"/g, "&quot;")
-        .replace(/'/g, "&`#039`;");
+        .replace(/>/g, "&gt;");
 };

Also applies to: 18-19

🤖 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 5 - 11, The string token
highlighting fails because escapeHtml transforms quotes into HTML entities
before highlightCode runs its string regexes (/'[^']*'/g and /"[^"]*"/g), so
those patterns never match; fix by running the tokenizer/highlighter against the
raw input (call highlightCode on the original text before calling escapeHtml) or
by applying the string regexes to the unescaped text and only escaping content
when wrapping tokens for output; update the flow in functions escapeHtml and
highlightCode so string matching happens on unescaped text and ensure any
inserted HTML is escaped when you build the final markup.

};

return <span dangerouslySetInnerHTML={{ __html: highlightCode(highlighted) }} />;
};
const highlightCode = (code: string) => {
const escaped = escapeHtml(code);
return escaped
.replace(/import|from|export|default|return|const|new/g, '<span class="text-red-500 dark:text-red-400 font-semibold">$&</span>')
.replace(/'[^']*'/g, '<span class="text-amber-600 dark:text-amber-400">$&</span>')
.replace(/"[^"]*"/g, '<span class="text-amber-600 dark:text-amber-400">$&</span>')
.replace(/Editron|console|editor/g, '<span class="text-rose-600 dark:text-rose-400">$&</span>');
};

const highlightCode = (code: string) => {
return code
.replace(/import|from|export|default|return|const|new/g, '<span class="text-red-500 dark:text-red-400 font-semibold">$&</span>')
.replace(/'[^']*'/g, '<span class="text-amber-600 dark:text-amber-400">$&</span>')
.replace(/"[^"]*"/g, '<span class="text-amber-600 dark:text-amber-400">$&</span>')
.replace(/Editron|console|editor/g, '<span class="text-rose-600 dark:text-rose-400">$&</span>');
const highlight = (text: string) => {
if (text.includes('//')) {
const parts = text.split('//');
return <><span dangerouslySetInnerHTML={{ __html: highlightCode(parts[0]) }} /><span className="text-slate-500 italic">{'//' + parts[1]}</span></>;
Comment on lines +24 to +26

@coderabbitai coderabbitai Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and display the relevant portion of the file
FILE="modules/home/code-line.tsx"
if [ ! -f "$FILE" ]; then
  echo "Missing file: $FILE"
  exit 1
fi

echo "---- $FILE (lines 1-120) ----"
nl -ba "$FILE" | sed -n '1,120p'

echo "---- $FILE (lines 18-40) ----"
nl -ba "$FILE" | sed -n '18,40p'

echo "---- $FILE: find split('//') usage ----"
rg -n "split\(['\"]//['\"]\)" "$FILE" || true

Repository: piyushdotcomm/Editron

Length of output: 178

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="modules/home/code-line.tsx"
if [ ! -f "$FILE" ]; then
  echo "Missing file: $FILE"
  exit 1
fi

echo "---- $FILE (lines 1-140) ----"
awk 'NR>=1 && NR<=140 { printf "%6d\t%s\n", NR, $0 }' "$FILE"

echo "---- $FILE: find split('//') usage ----"
rg -n "split\(['\"]//['\"]\)" "$FILE" || true

echo "---- $FILE: find text.split usages ----"
rg -n "text\.split\(" "$FILE" || true

Repository: piyushdotcomm/Editron

Length of output: 1994

Fix highlight() dropping content after additional //

text.split('//') is used, but only parts[0] and parts[1] are rendered ('//' + parts[1]), so any subsequent // segments (e.g., parts[2+]) are lost.

Proposed fix 
 const highlight = (text: string) => {
     if (text.includes('//')) {
-        const parts = text.split('//');
-        return <><span dangerouslySetInnerHTML={{ __html: highlightCode(parts[0]) }} /><span className="text-slate-500 italic">{'//' + parts[1]}</span></>;
+        const commentStart = text.indexOf('//');
+        const codePart = text.slice(0, commentStart);
+        const commentPart = text.slice(commentStart);
+        return (
+            <>
+                <span dangerouslySetInnerHTML={{ __html: highlightCode(codePart) }} />
+                <span className="text-slate-500 italic">{commentPart}</span>
+            </>
+        );
     }
📝 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.

Suggested change
if (text.includes('//')) {
const parts = text.split('//');
return <><span dangerouslySetInnerHTML={{ __html: highlightCode(parts[0]) }} /><span className="text-slate-500 italic">{'//' + parts[1]}</span></>;
if (text.includes('//')) {
const commentStart = text.indexOf('//');
const codePart = text.slice(0, commentStart);
const commentPart = text.slice(commentStart);
return (
<>
<span dangerouslySetInnerHTML={{ __html: highlightCode(codePart) }} />
<span className="text-slate-500 italic">{commentPart}</span>
</>
);
}
🧰 Tools
🪛 ast-grep (0.43.0)

[warning] 25-25: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation

(react-unsafe-html-injection)

🤖 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 24 - 26, The comment parsing in the
code-line.tsx branch uses text.split('//') but only renders parts[0] and
parts[1], dropping any additional segments; update the rendering in the if
(text.includes('//')) branch so that after splitting you reconstruct the full
comment by joining parts.slice(1).join('//') (prefixing with '//' as before) and
render highlightCode(parts[0]) for the code portion and the reconstructed
comment string for the comment span; reference the variables text, parts and the
function highlightCode to locate the change.

}
return <span dangerouslySetInnerHTML={{ __html: highlightCode(text) }} />;
};

const [highlighted, setHighlighted] = React.useState<React.ReactNode>(line);

React.useEffect(() => {
setHighlighted(highlight(line));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [line]);

export const CodeLine = ({ line }: { line: string }) => {
const highlighted = React.useMemo(() => highlight(line), [line]);
return highlighted;
};
9 changes: 4 additions & 5 deletions modules/playground/components/playground-editor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"use client";

import { EDITOR_CONFIG } from "@/lib/constants/config";
import { useRef, useEffect, useState } from "react";
import { useRef, useEffect, useState, useCallback } from "react";
import Editor, { type Monaco } from "@monaco-editor/react";
import type { editor as MonacoEditor } from "monaco-editor";
import {
Expand Down Expand Up @@ -253,7 +253,7 @@ const PlaygroundEditor = ({
);
};

const updateEditorLanguage = () => {
const updateEditorLanguage = useCallback(() => {
if (!activeFile || !monacoRef.current || !editorRef.current) return;
const model = editorRef.current.getModel();
if (!model) return;
Expand All @@ -264,12 +264,11 @@ const PlaygroundEditor = ({
} catch (error) {
console.warn("Failed to set editor language:", error);
}
};
}, [activeFile]);

useEffect(() => {
updateEditorLanguage();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [activeFile]);
}, [activeFile, updateEditorLanguage]);

// Bind Yjs to Monaco
useEffect(() => {
Expand Down
2 changes: 0 additions & 2 deletions modules/webcontainers/components/terminal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,6 @@ const
writePrompt();

return terminal;
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [theme, handleTerminalInput, writePrompt]);

const connectToWebContainer = useCallback(async () => {
Expand Down Expand Up @@ -420,7 +419,6 @@ const
currentProcess.current.kill();
}
if (shellProcess.current) {
// eslint-disable-next-line react-hooks/exhaustive-deps
shellProcess.current.kill();
}
if (term.current) {
Expand Down