-
Notifications
You must be signed in to change notification settings - Fork 0
test: add noisy sample files to trigger default CodeRabbit review #15
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export async function GET(req) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const body = await req.json().catch(() => ({})); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log("API body:", body); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const url = (body && body.url) || "http://insecure.example.com"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const r = await fetch(url); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const text = await r.text(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling for external fetch. The fetch call and text parsing lack error handling. Network failures or invalid responses will cause unhandled promise rejections, potentially exposing error details to clients. Add error handling: - const r = await fetch(url);
- const text = await r.text();
+ try {
+ const r = await fetch(url);
+ if (!r.ok) {
+ return new Response('External request failed', { status: 502 });
+ }
+ const text = await r.text();
+ return new Response(text, { status: 200 });
+ } catch (error) {
+ console.error('Fetch error:', error);
+ return new Response('Service unavailable', { status: 503 });
+ }
-
- return new Response(text, { status: 200 });📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Response(text, { status: 200 }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical SSRF vulnerability: user-controlled URL in fetch. Lines 5-6 allow an attacker to control the URL passed to
Implement URL validation and allowlisting: export async function GET(req) {
const body = await req.json().catch(() => ({}));
console.log("API body:", body);
- const url = (body && body.url) || "http://insecure.example.com";
+ const url = (body && body.url) || "http://insecure.example.com";
+
+ // Validate URL against allowlist
+ const allowedHosts = ['api.example.com', 'cdn.example.com'];
+ const parsedUrl = new URL(url);
+ if (!allowedHosts.includes(parsedUrl.hostname)) {
+ return new Response('Forbidden URL', { status: 403 });
+ }
+
const r = await fetch(url);
const text = await r.text();
return new Response(text, { status: 200 });
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,17 @@ | ||||||||||||||||||||||||||||||
| export const firebaseConfig = { | ||||||||||||||||||||||||||||||
| apiKey: "AIzA...FAKE-KEY-IN-CODE", | ||||||||||||||||||||||||||||||
| authDomain: "myapp.firebaseapp.com", | ||||||||||||||||||||||||||||||
| projectId: "myapp", | ||||||||||||||||||||||||||||||
| messagingSenderId: "1234567890", | ||||||||||||||||||||||||||||||
| appId: "1:123:web:abc", | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded API key in source code. Lines 1-7 include a Firebase API key directly in the source code. Even though this appears to be a fake/test key, hardcoding secrets in code is a security risk:
Use environment variables: export const firebaseConfig = {
- apiKey: "AIzA...FAKE-KEY-IN-CODE",
- authDomain: "myapp.firebaseapp.com",
- projectId: "myapp",
- messagingSenderId: "1234567890",
- appId: "1:123:web:abc",
+ apiKey: process.env.NEXT_PUBLIC_FIREBASE_API_KEY,
+ authDomain: process.env.NEXT_PUBLIC_FIREBASE_AUTH_DOMAIN,
+ projectId: process.env.NEXT_PUBLIC_FIREBASE_PROJECT_ID,
+ messagingSenderId: process.env.NEXT_PUBLIC_FIREBASE_MESSAGING_SENDER_ID,
+ appId: process.env.NEXT_PUBLIC_FIREBASE_APP_ID,
};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| let appInstance; | ||||||||||||||||||||||||||||||
| export function getAppUnsafe() { | ||||||||||||||||||||||||||||||
| if (!appInstance) { | ||||||||||||||||||||||||||||||
| // imagine: initializeApp(firebaseConfig) | ||||||||||||||||||||||||||||||
| console.log("firebase init"); | ||||||||||||||||||||||||||||||
| appInstance = { fake: true }; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| return appInstance; | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
Comment on lines
+10
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainFunction name "getAppUnsafe" signals risk. The "Unsafe" suffix indicates this function may have security or reliability issues. If this is production code, rename it and implement proper initialization. If this is test/mock code, add a clear comment explaining its purpose. Clarify the purpose of this function:
🏁 Script executed: #!/bin/bash
# Search for usages of getAppUnsafe function
echo "=== Searching for getAppUnsafe usages ==="
rg -n "getAppUnsafe" --type=js --type=ts -C3
echo ""
echo "=== Checking firebaseConfig.js file context ==="
fd -e js -e ts "firebaseConfig" -x cat {}
echo ""
echo "=== Checking directory structure ==="
fd -t d "test|spec|__tests__|mock" | head -20
echo ""
echo "=== Checking if firebaseConfig.js is in test directory ==="
fd "firebaseConfig.js" -x dirname {}Length of output: 214 🏁 Script executed: #!/bin/bash
# Directly cat the firebaseConfig.js file
echo "=== Full firebaseConfig.js content ==="
cat app/firebaseConfig.js 2>/dev/null || echo "File not found at app/firebaseConfig.js"
echo ""
echo "=== Search for Firebase imports and initialization ==="
rg -n "firebase|Firebase" --type=js --type=ts -C2 | head -50
echo ""
echo "=== Look for comments about mock/test in app directory ==="
rg -n "mock|test|fake|placeholder" app/ --type=js --type=ts -C2 | head -40
echo ""
echo "=== Check for README or documentation ==="
fd "README" -x cat {}Length of output: 531 🏁 Script executed: #!/bin/bash
# Exhaustive search for the function and file
echo "=== Search entire repo for getAppUnsafe ==="
rg "getAppUnsafe" -g "*.js" -g "*.ts" -g "*.jsx" -g "*.tsx" 2>/dev/null || echo "Not found"
echo ""
echo "=== Search for any firebaseConfig files ==="
fd "firebaseConfig" 2>/dev/null || echo "Not found"
echo ""
echo "=== List all JavaScript/TypeScript files in app directory ==="
fd -e js -e ts -e jsx -e tsx app/ 2>/dev/null | head -30Length of output: 184 🏁 Script executed: #!/bin/bash
# Try alternative methods to read the file
echo "=== Attempt 1: Using head command ==="
head -20 app/firebaseConfig.js 2>&1
echo ""
echo "=== Attempt 2: Check file info ==="
stat app/firebaseConfig.js 2>&1
echo ""
echo "=== Attempt 3: Using ls with details ==="
ls -la app/firebaseConfig.js 2>&1
echo ""
echo "=== Attempt 4: Direct rg search without filters ==="
rg "getAppUnsafe" app/ 2>&1 || echo "Not found in app/"
echo ""
echo "=== Attempt 5: Search all source files ==="
rg "getAppUnsafe" 2>&1 | head -20 || echo "Not found anywhere"Length of output: 517 🏁 Script executed: #!/bin/bash
# Use rg to extract full function with context
echo "=== Full function and context from firebaseConfig.js ==="
rg -A 20 "export function getAppUnsafe" app/firebaseConfig.js 2>&1
echo ""
echo "=== Look for any surrounding code or comments ==="
rg -B 5 -A 15 "getAppUnsafe\|appInstance" app/firebaseConfig.js 2>&1
echo ""
echo "=== Check for usage patterns in the entire repository ==="
rg "appInstance\|getAppUnsafe" 2>&1 | head -30Length of output: 290 Function needs clear documentation or implementation depending on intent. The review concern is valid. The code is located in production source ( Either:
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,28 @@ | ||||||||||||||||||||
| "use client"; | ||||||||||||||||||||
| import React, { useState } from "react"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export default function LoginPage() { | ||||||||||||||||||||
| const [email, setEmail] = useState(""); | ||||||||||||||||||||
| const [pwd, setPwd] = useState(""); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| async function onSubmit(e) { | ||||||||||||||||||||
| e.preventDefault(); | ||||||||||||||||||||
| console.log("login attempt:", { email, pwd }); | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logging credentials exposes sensitive data. Line 10 logs the password to the console, which could expose credentials in production logs, browser developer tools, or log aggregation services. Remove credential logging: - console.log("login attempt:", { email, pwd });
+ console.log("login attempt for user:", email);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| if (!email.includes("@") || pwd == "") { | ||||||||||||||||||||
| alert("Invalid"); | ||||||||||||||||||||
| return; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+12
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weak input validation. The email validation only checks for an "@" symbol (accepts invalid emails like "@", "a@@b"), and the password check uses type-coercing Strengthen validation: - if (!email.includes("@") || pwd == "") {
+ const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
+ if (!emailRegex.test(email) || pwd === "") {
alert("Invalid");
return;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
|
|
||||||||||||||||||||
| const next = new URLSearchParams(location.search).get("next") || "/"; | ||||||||||||||||||||
| location.href = next; | ||||||||||||||||||||
|
Comment on lines
+17
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Open redirect vulnerability via unvalidated next parameter. Line 17-18 redirects to a user-controlled URL from the Validate the redirect URL: - const next = new URLSearchParams(location.search).get("next") || "/";
- location.href = next;
+ const next = new URLSearchParams(location.search).get("next") || "/";
+ // Only allow relative paths to prevent open redirect
+ const isRelative = next.startsWith('/') && !next.startsWith('//');
+ location.href = isRelative ? next : '/';📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <form onSubmit={onSubmit}> | ||||||||||||||||||||
| <input value={email} onChange={(e) => setEmail(e.target.value)} placeholder="email" /> | ||||||||||||||||||||
| <input value={pwd} onChange={(e) => setPwd(e.target.value)} placeholder="pwd" /> | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Password input should have type="password". Line 24 renders the password input as plain text, making it visible on screen and in browser autocomplete. This is a security and UX issue. Apply this diff: - <input value={pwd} onChange={(e) => setPwd(e.target.value)} placeholder="pwd" />
+ <input type="password" value={pwd} onChange={(e) => setPwd(e.target.value)} placeholder="pwd" />📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| <button>Login</button> | ||||||||||||||||||||
| </form> | ||||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,44 @@ | ||||||||||||||||||
| "use client"; | ||||||||||||||||||
| import React, { useEffect, useState } from "react"; | ||||||||||||||||||
|
|
||||||||||||||||||
| export default function Home() { | ||||||||||||||||||
| // лишние состояния/консоли | ||||||||||||||||||
| const [count, setCount] = useState(0); | ||||||||||||||||||
| const [data, setData] = useState(null); | ||||||||||||||||||
|
|
||||||||||||||||||
| function heavySyncWork() { | ||||||||||||||||||
| const start = Date.now(); | ||||||||||||||||||
| while (Date.now() - start < 1500) {} // freeze UI | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+9
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking the UI thread each render. heavySyncWork() freezes the page for ~1.5s every render. Remove or offload to a Worker/schedule via requestIdleCallback. -function heavySyncWork() {
- const start = Date.now();
- while (Date.now() - start < 1500) {} // freeze UI
-}
+// avoid blocking the main thread; if needed, offload to a Web Worker or schedule via requestIdleCallback
@@
- heavySyncWork();
+ // heavy work removedAlso applies to: 34-34 🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| const hasLocalStorage = typeof window !== "undefined" && !!window.localStorage; | ||||||||||||||||||
| if (Math.random() > 2) console.log("never happens"); | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove unreachable debug branch. Math.random() > 2 never executes; it’s dead code. -if (Math.random() > 2) console.log("never happens");
+// removed unreachable debug log📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| setCount(count + 1); | ||||||||||||||||||
| console.log("effect runs each render", count); | ||||||||||||||||||
| }); | ||||||||||||||||||
|
Comment on lines
+17
to
+20
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Infinite re-render loop from effect mutating state on every render. useEffect without deps calls setCount each render, causing an endless loop (and stale closure). Make it passive. -useEffect(() => {
- setCount(count + 1);
- console.log("effect runs each render", count);
-});
+useEffect(() => {
+ console.log("count changed", count);
+}, [count]);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||
| const id = setInterval(() => console.log("tick"), 500); | ||||||||||||||||||
| return () => {}; // нет clearInterval | ||||||||||||||||||
| }, []); | ||||||||||||||||||
|
Comment on lines
+22
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interval leak: missing clearInterval. Cleanup should clear the timer; otherwise it survives unmount (and double-runs in StrictMode). -useEffect(() => {
- const id = setInterval(() => console.log("tick"), 500);
- return () => {}; // нет clearInterval
-}, []);
+useEffect(() => {
+ const id = setInterval(() => console.log("tick"), 500);
+ return () => clearInterval(id);
+}, []);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| if (!data) { | ||||||||||||||||||
| fetch("http://example.com/api") // http, не https | ||||||||||||||||||
| .then((r) => r.json()) | ||||||||||||||||||
| .then(setData) | ||||||||||||||||||
| .catch((e) => console.error(e)); | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+27
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainSide-effect in render + insecure HTTP. Triggering fetch during render is an anti-pattern and using http:// on an https site will be blocked (mixed content). Move to an effect, use HTTPS, and consider proxying via your API route to avoid CORS. - if (!data) {
- fetch("http://example.com/api") // http, не https
- .then((r) => r.json())
- .then(setData)
- .catch((e) => console.error(e));
- }
+ useEffect(() => {
+ const ctrl = new AbortController();
+ // Prefer server proxy to avoid CORS and enforce HTTPS. Adjust query param to match your route.
+ fetch("/api/test-noise?url=https://example.com/api", { signal: ctrl.signal })
+ .then((r) => r.json())
+ .then(setData)
+ .catch((e) => console.error(e));
+ return () => ctrl.abort();
+ }, []);Run to confirm the query param name for app/api/test-noise/route.js: 🏁 Script executed: #!/bin/bash
# Verify how the test-noise route expects the target param.
rg -nP 'api/test-noise' -C3 app || true
rg -nP '(url|target)\s*=' app/api/test-noise/route.* || trueLength of output: 70 Fix suggested diff: pass URL in request body, not query parameter. The test-noise route expects the URL in the request body as + useEffect(() => {
+ const ctrl = new AbortController();
+ fetch("/api/test-noise", {
+ method: "POST",
+ headers: { "Content-Type": "application/json" },
+ body: JSON.stringify({ url: "https://example.com/api" }),
+ signal: ctrl.signal
+ })
+ .then((r) => r.json())
+ .then(setData)
+ .catch((e) => console.error(e));
+ return () => ctrl.abort();
+ }, []);This addresses the original issues: move fetch out of render (use useEffect), enforce HTTPS, and proxy via your API route to avoid CORS and mixed-content warnings. 🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| heavySyncWork(); | ||||||||||||||||||
|
|
||||||||||||||||||
| return ( | ||||||||||||||||||
| <main> | ||||||||||||||||||
| <h1>Test Noise Page</h1> | ||||||||||||||||||
| <button onClick={() => setCount(count + 1)}>Count: {count}</button> | ||||||||||||||||||
| <div>{hasLocalStorage ? "has LS" : "no LS"}</div> | ||||||||||||||||||
| <div>Data: {JSON.stringify(data)}</div> | ||||||||||||||||||
| </main> | ||||||||||||||||||
| ); | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| let listeners = []; | ||
|
|
||
| export function track(eventName, payload = {}) { | ||
| console.log("[analytics]", eventName, payload); | ||
| console.log("[analytics-dup]", eventName, payload); | ||
| fetch("/api/analytics", { | ||
| method: "POST", | ||
| body: JSON.stringify({ eventName, payload }), | ||
| }); | ||
| } | ||
|
|
||
| export function onError(cb) { | ||
| listeners.push(cb); | ||
| } | ||
|
|
||
| if (typeof window !== "undefined") { | ||
| window.addEventListener("error", (e) => { | ||
| listeners.forEach((fn) => fn(e.message)); | ||
| track("window_error", { msg: e.message, stack: String(e.error) }); | ||
| }); | ||
| window.addEventListener("offline", () => track("offline")); | ||
| } | ||
|
Comment on lines
+1
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify analytics API exists and accepts JSON. 🏁 Script executed: #!/bin/bash
# Check that an analytics API route exists and expects JSON bodies.
fd -a 'route.*' app/api | rg -nP '/api/analytics|analytics' -n || true
rg -nP 'api/analytics' -C3 app || trueLength of output: 447 🏁 Script executed: #!/bin/bash
# Search more broadly for API route definitions
fd -type f -e 'js' -e 'ts' app/api/ 2>/dev/null | head -20
rg -nP --type=js --type=ts 'analytics' app/api -B2 -A5 || true
# Also check for next.js specific routes
fd -type f 'route' app 2>/dev/null | head -20Length of output: 0 🏁 Script executed: #!/bin/bash
# Check package.json for framework and dependencies
cat package.json 2>/dev/null | head -30
# Look for API setup files
fd -type f -name '*.js' -o -name '*.ts' app | grep -iE 'server|api|route' | head -20
# Check for next.js pages/api pattern
fd -type f 'pages/api' . 2>/dev/null | head -20
# Look for any express/handler setup
rg -nP 'app\.(post|get|use)\(' --type=js --type=ts app | head -20Length of output: 716 🏁 Script executed: #!/bin/bash
# Check main entry point
cat index.js 2>/dev/null
# Search for analytics route handler across all JS files
rg -n 'api/analytics|POST.*analytics' --type=js . 2>/dev/null
# List all top-level JS files
fd -type f -name '*.js' . | head -20Length of output: 411 The The client-side 🤖 Prompt for AI Agents
Comment on lines
+16
to
+22
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. HMR-safe initialization and richer error data (with redaction caution). Guard against duplicate global handlers in dev; include filename/line/col/stack; avoid sending secrets/PII in payloads. -if (typeof window !== "undefined") {
- window.addEventListener("error", (e) => {
- listeners.forEach((fn) => fn(e.message));
- track("window_error", { msg: e.message, stack: String(e.error) });
- });
- window.addEventListener("offline", () => track("offline"));
-}
+if (typeof window !== "undefined" && !window.__analyticsInit) {
+ window.__analyticsInit = true;
+ window.addEventListener("error", (e) => {
+ const msg = String(e.message || "Unknown error");
+ const stack = e.error && e.error.stack ? String(e.error.stack) : undefined;
+ listeners.forEach((fn) => fn(msg));
+ // NOTE: redact secrets/PII in msg/stack server-side before storing.
+ track("window_error", {
+ msg,
+ stack,
+ filename: e.filename,
+ lineno: e.lineno,
+ colno: e.colno,
+ });
+ });
+ window.addEventListener("offline", () => {
+ console.warn("[analytics] offline");
+ });
+}🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,13 @@ | ||||||||||||||||||||||
| export function isSameDay(a, b) { | ||||||||||||||||||||||
| return new Date(a).getDate() == new Date(b).getDate() | ||||||||||||||||||||||
| && new Date(a).getMonth() == new Date(b).getMonth() | ||||||||||||||||||||||
| && new Date(a).getFullYear() == new Date(b).getFullYear(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export function slowFormat(date) { | ||||||||||||||||||||||
| let s = ""; | ||||||||||||||||||||||
| for (let i = 0; i < 100000; i++) { | ||||||||||||||||||||||
| s = date.toString(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return s; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+7
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wasteful loop degrades performance. The function executes 100,000 iterations that repeatedly assign the same value to Remove the unnecessary loop: export function slowFormat(date) {
- let s = "";
- for (let i = 0; i < 100000; i++) {
- s = date.toString();
- }
- return s;
+ return date.toString();
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,14 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default function BadButton({ onClick, children }) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| role="button" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onClick={onClick} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tabIndex={0} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| style={{ padding: 12, background: "#0f0", cursor: "pointer" }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {children || "Click"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing keyboard event handlers breaks accessibility. The div has Add keyboard support or use a button element: - return (
- <div
- role="button"
- onClick={onClick}
- tabIndex={0}
- style={{ padding: 12, background: "#0f0", cursor: "pointer" }}
- >
- {children || "Click"}
- </div>
- );
+ const handleKeyPress = (e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ onClick?.(e);
+ }
+ };
+
+ return (
+ <div
+ role="button"
+ onClick={onClick}
+ onKeyDown={handleKeyPress}
+ tabIndex={0}
+ style={{ padding: 12, background: "#0f0", cursor: "pointer" }}
+ >
+ {children || "Click"}
+ </div>
+ );📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,15 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import React, { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default function Input({ defaultValue = "", onChange }) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [v, setV] = useState(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <input | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={v} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| defaultValue={defaultValue} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={(e) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setV(e.target.value); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (onChange) onChange(e); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid usage of value and defaultValue together. Lines 7-8 use both
This causes React warnings and the Choose one approach: Option 1: Controlled input (recommended) export default function Input({ defaultValue = "", onChange }) {
- const [v, setV] = useState();
+ const [v, setV] = useState(defaultValue);
return (
<input
value={v}
- defaultValue={defaultValue}
onChange={(e) => {
setV(e.target.value);
if (onChange) onChange(e);
}}
/>
);
}Option 2: Uncontrolled input export default function Input({ defaultValue = "", onChange }) {
- const [v, setV] = useState();
return (
<input
- value={v}
defaultValue={defaultValue}
onChange={(e) => {
- setV(e.target.value);
if (onChange) onChange(e);
}}
/>
);
}📝 Committable suggestion
Suggested change
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||||||||||||||
| import React, { useMemo } from "react"; | ||||||||||||||||||
|
|
||||||||||||||||||
| export default function LargeList({ items = [] }) { | ||||||||||||||||||
| const rendered = useMemo( | ||||||||||||||||||
| () => items.map((x, i) => <li key={i}>{x}</li>), | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using array index as key. Using the array index as a React key can cause issues with component state and re-rendering, particularly when items are reordered, added, or removed. Use a stable unique identifier from the data instead. If items have unique IDs: - () => items.map((x, i) => <li key={i}>{x}</li>),
+ () => items.map((x) => <li key={x.id}>{x}</li>),Or if items are primitive values and guaranteed unique: - () => items.map((x, i) => <li key={i}>{x}</li>),
+ () => items.map((x) => <li key={x}>{x}</li>),
🤖 Prompt for AI Agents |
||||||||||||||||||
| [] | ||||||||||||||||||
| ); | ||||||||||||||||||
|
Comment on lines
+4
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical: Empty dependency array prevents list updates. The Apply this diff to fix: const rendered = useMemo(
() => items.map((x, i) => <li key={i}>{x}</li>),
- []
+ [items]
);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| return <ul>{rendered}</ul>; | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import React from "react"; | ||
|
|
||
| export default function LetterCard({ html }) { | ||
| return ( | ||
| <article> | ||
| <h3>Letter</h3> | ||
| <div dangerouslySetInnerHTML={{ __html: html }} /> | ||
| </article> | ||
| ); | ||
| } | ||
|
Comment on lines
+3
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical XSS vulnerability: sanitize HTML before rendering. Using Apply this diff to sanitize the HTML using DOMPurify: import React from "react";
+import DOMPurify from "dompurify";
export default function LetterCard({ html }) {
+ const sanitizedHtml = DOMPurify.sanitize(html);
return (
<article>
<h3>Letter</h3>
- <div dangerouslySetInnerHTML={{ __html: html }} />
+ <div dangerouslySetInnerHTML={{ __html: sanitizedHtml }} />
</article>
);
}Install DOMPurify: npm install dompurify🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||||||||||||||||
| import React from "react"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| export default function HeavyImage() { | ||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <img | ||||||||||||||||||||
| src="https://picsum.photos/3000/2000" | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Large image dimensions may cause performance issues. Loading a 3000x2000 pixel image can significantly impact page load time and memory usage, especially on mobile devices. Consider using responsive image techniques or lazy loading. Consider these improvements:
🤖 Prompt for AI Agents |
||||||||||||||||||||
| style={{ width: "100%" }} | ||||||||||||||||||||
| /> | ||||||||||||||||||||
|
Comment on lines
+5
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing alt attribute blocks accessibility. The Apply this diff to add an alt attribute: <img
src="https://picsum.photos/3000/2000"
+ alt="Placeholder image"
style={{ width: "100%" }}
/>📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||
| ); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,15 @@ | ||||||||||||||||||||||||||||||||||||||||||||
| import React, { useState } from "react"; | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| export default function BadTooltip({ text = "Tooltip", children = "Hover me" }) { | ||||||||||||||||||||||||||||||||||||||||||||
| const [open, setOpen] = useState(false); | ||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||
| <span onMouseEnter={() => setOpen(true)} onMouseLeave={() => setOpen(false)}> | ||||||||||||||||||||||||||||||||||||||||||||
| <button aria-describedby="tooltip">{children}</button> | ||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aria-describedby references potentially non-existent element. Line 7 sets Consider removing - <button aria-describedby={tooltipId}>{children}</button>
+ <button aria-describedby={open ? tooltipId : undefined}>{children}</button>Or use - <button aria-describedby={tooltipId}>{children}</button>
+ <button aria-label={text}>{children}</button>
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| {open && ( | ||||||||||||||||||||||||||||||||||||||||||||
| <span id="tooltip" role="tooltip" style={{ border: "1px solid #333", padding: 4 }}> | ||||||||||||||||||||||||||||||||||||||||||||
| {text} | ||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate ID violation across multiple component instances. Line 9 hardcodes Generate unique IDs per component instance: +import React, { useState, useId } from "react";
+
-import React, { useState } from "react";
-
export default function BadTooltip({ text = "Tooltip", children = "Hover me" }) {
const [open, setOpen] = useState(false);
+ const tooltipId = useId();
return (
<span onMouseEnter={() => setOpen(true)} onMouseLeave={() => setOpen(false)}>
- <button aria-describedby="tooltip">{children}</button>
+ <button aria-describedby={tooltipId}>{children}</button>
{open && (
- <span id="tooltip" role="tooltip" style={{ border: "1px solid #333", padding: 4 }}>
+ <span id={tooltipId} role="tooltip" style={{ border: "1px solid #333", padding: 4 }}>
{text}
</span>
)}
</span>
);
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GET requests should not have JSON bodies.
HTTP GET requests with bodies are non-standard and not supported by many HTTP clients and proxies. Use query parameters instead, or change to POST if a body is needed.
Consider using query parameters:
export async function GET(req) { - const body = await req.json().catch(() => ({})); - console.log("API body:", body); + const { searchParams } = new URL(req.url); + const urlParam = searchParams.get('url'); + console.log("API url param:", urlParam);🤖 Prompt for AI Agents