Skip to content

Refactor performance optimizations and security headers - #19

Merged
urjitc merged 4 commits into
mainfrom
refactor/no-ref/performance-optimizations-and-security-headers
Jan 15, 2026
Merged

Refactor performance optimizations and security headers#19
urjitc merged 4 commits into
mainfrom
refactor/no-ref/performance-optimizations-and-security-headers

Conversation

@urjitc

@urjitc urjitc commented Jan 15, 2026

Copy link
Copy Markdown
Member

Important

This PR refactors API routes for parallel execution, optimizes imports, and adds security headers to enhance performance and security in a React/Next.js app.

  • API Routes:
    • Implement parallelization in GET, POST, PATCH, and DELETE methods in route.ts and events/route.ts by starting independent operations concurrently.
  • Configuration:
    • Add security headers in vercel.json for enhanced security, including HSTS, X-Content-Type-Options, and more.
    • Optimize barrel file imports in next.config.ts using optimizePackageImports for lucide-react.
  • Components:
    • Remove unoptimized attribute from Image components in FourWays.tsx and ThreeSteps.tsx.
  • Documentation:
    • Add VERCEL_CONFIG.md to explain Vercel configuration and security headers.
    • Add web-design-guidelines/SKILL.md for UI code review guidelines.

This description was created by Ellipsis for 9379d75. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • New Features

    • Added comprehensive React/Next.js best practices guide with 40+ performance rules across 8 categories for improved codebase optimization.
    • Introduced web design guidelines skill for UI/design reviews.
    • Configured security headers and intelligent caching policies for improved performance and safety.
  • Improvements

    • Optimized API route handlers for faster parallel data fetching.
    • Enabled automatic image optimization across the app.
    • Added package import optimization for faster builds.

✏️ Tip: You can customize this high-level summary in your review settings.

…elize API route operations with Promise.all; enable Next.js image optimization; add security headers and caching policies in vercel.json
@greptile-apps

greptile-apps Bot commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

Skipped: This PR changes more files than the configured file change limit: (112 files found, 100 file limit)

@vercel

vercel Bot commented Jan 15, 2026

Copy link
Copy Markdown
Contributor

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

Project Deployment Review Updated (UTC)
thinkexv2 Canceled Canceled Jan 15, 2026 1:58am

@coderabbitai

coderabbitai Bot commented Jan 15, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

This PR adds a comprehensive React/Next.js performance optimization guide targeting AI agents (40+ rules across 8 categories), refactors API routes to parallelize independent async operations, configures Next.js bundle optimization, and establishes Vercel deployment caching policies.

Changes

Cohort / File(s) Summary
React Best Practices Guide
.claude/skills/vercel-react-best-practices/
.github/skills/vercel-react-best-practices/
AGENTS.md, README.md, SKILL.md, metadata.json
rules/_sections.md, rules/_template.md
rules/async-*.md, rules/bundle-*.md, rules/client-*.md, rules/js-*.md, rules/rendering-*.md, rules/rerender-*.md, rules/server-*.md, rules/advanced-*.md
Adds comprehensive documentation of 40+ performance rules organized across 8 categories (Waterfalls, Bundle Size, Server-Side, Client-Side, Re-renders, Rendering, JavaScript, Advanced). Duplicated in both .claude/ and .github/ directories. Each rule includes impact level, incorrect vs. correct code examples, and practical guidance for automated refactoring.
API Route Parallelization
src/app/api/workspaces/[id]/events/route.ts
src/app/api/workspaces/[id]/route.ts
src/app/api/workspaces/slug/[slug]/route.ts
Refactors GET/POST/PATCH/DELETE handlers to parallelize independent I/O operations (params, headers, body) using promise scheduling rather than sequential awaits. Reduces latency by overlapping async waits without changing response structure or error handling.
Next.js & Deployment Configuration
next.config.ts
vercel.json
Enables experimental.optimizePackageImports for lucide-react in Next.js config. Adds Vercel deployment configuration with security headers (HSTS, X-Frame-Options, etc.), cache policies for static assets (long-term), API routes (no-cache with noindex), and general resources (revalidate).
Image Component Optimization
src/components/landing/FourWays.tsx
src/components/landing/ThreeSteps.tsx
Removes unoptimized prop from Next.js Image components, enabling default Next.js image optimization.
Web Design Guidelines Skill
.claude/skills/web-design-guidelines/SKILL.md
.github/skills/web-design-guidelines/SKILL.md
Adds skill definition for UI/design review workflow that fetches guidelines from remote source, applies rules to target files, and outputs findings in terse format.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


🐰 Forty rules of wisdom, now compiled with care,
API waterfalls turned to dances in the air,
From bundle bloat to renders clean and bright,
Performance patterns guide the way toward light!

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9eec54f and 75490ca.

📒 Files selected for processing (111)
  • .claude/skills/vercel-react-best-practices/AGENTS.md
  • .claude/skills/vercel-react-best-practices/README.md
  • .claude/skills/vercel-react-best-practices/SKILL.md
  • .claude/skills/vercel-react-best-practices/metadata.json
  • .claude/skills/vercel-react-best-practices/rules/_sections.md
  • .claude/skills/vercel-react-best-practices/rules/_template.md
  • .claude/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md
  • .claude/skills/vercel-react-best-practices/rules/advanced-use-latest.md
  • .claude/skills/vercel-react-best-practices/rules/async-api-routes.md
  • .claude/skills/vercel-react-best-practices/rules/async-defer-await.md
  • .claude/skills/vercel-react-best-practices/rules/async-dependencies.md
  • .claude/skills/vercel-react-best-practices/rules/async-parallel.md
  • .claude/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md
  • .claude/skills/vercel-react-best-practices/rules/bundle-barrel-imports.md
  • .claude/skills/vercel-react-best-practices/rules/bundle-conditional.md
  • .claude/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md
  • .claude/skills/vercel-react-best-practices/rules/bundle-dynamic-imports.md
  • .claude/skills/vercel-react-best-practices/rules/bundle-preload.md
  • .claude/skills/vercel-react-best-practices/rules/client-event-listeners.md
  • .claude/skills/vercel-react-best-practices/rules/client-swr-dedup.md
  • .claude/skills/vercel-react-best-practices/rules/js-batch-dom-css.md
  • .claude/skills/vercel-react-best-practices/rules/js-cache-function-results.md
  • .claude/skills/vercel-react-best-practices/rules/js-cache-property-access.md
  • .claude/skills/vercel-react-best-practices/rules/js-cache-storage.md
  • .claude/skills/vercel-react-best-practices/rules/js-combine-iterations.md
  • .claude/skills/vercel-react-best-practices/rules/js-early-exit.md
  • .claude/skills/vercel-react-best-practices/rules/js-hoist-regexp.md
  • .claude/skills/vercel-react-best-practices/rules/js-index-maps.md
  • .claude/skills/vercel-react-best-practices/rules/js-length-check-first.md
  • .claude/skills/vercel-react-best-practices/rules/js-min-max-loop.md
  • .claude/skills/vercel-react-best-practices/rules/js-set-map-lookups.md
  • .claude/skills/vercel-react-best-practices/rules/js-tosorted-immutable.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-activity.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-conditional-render.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-content-visibility.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-hydration-no-flicker.md
  • .claude/skills/vercel-react-best-practices/rules/rendering-svg-precision.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-defer-reads.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-dependencies.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-derived-state.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-lazy-state-init.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-memo.md
  • .claude/skills/vercel-react-best-practices/rules/rerender-transitions.md
  • .claude/skills/vercel-react-best-practices/rules/server-after-nonblocking.md
  • .claude/skills/vercel-react-best-practices/rules/server-cache-lru.md
  • .claude/skills/vercel-react-best-practices/rules/server-cache-react.md
  • .claude/skills/vercel-react-best-practices/rules/server-parallel-fetching.md
  • .claude/skills/vercel-react-best-practices/rules/server-serialization.md
  • .claude/skills/web-design-guidelines/SKILL.md
  • .github/skills/vercel-react-best-practices/AGENTS.md
  • .github/skills/vercel-react-best-practices/README.md
  • .github/skills/vercel-react-best-practices/SKILL.md
  • .github/skills/vercel-react-best-practices/metadata.json
  • .github/skills/vercel-react-best-practices/rules/_sections.md
  • .github/skills/vercel-react-best-practices/rules/_template.md
  • .github/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md
  • .github/skills/vercel-react-best-practices/rules/advanced-use-latest.md
  • .github/skills/vercel-react-best-practices/rules/async-api-routes.md
  • .github/skills/vercel-react-best-practices/rules/async-defer-await.md
  • .github/skills/vercel-react-best-practices/rules/async-dependencies.md
  • .github/skills/vercel-react-best-practices/rules/async-parallel.md
  • .github/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md
  • .github/skills/vercel-react-best-practices/rules/bundle-barrel-imports.md
  • .github/skills/vercel-react-best-practices/rules/bundle-conditional.md
  • .github/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md
  • .github/skills/vercel-react-best-practices/rules/bundle-dynamic-imports.md
  • .github/skills/vercel-react-best-practices/rules/bundle-preload.md
  • .github/skills/vercel-react-best-practices/rules/client-event-listeners.md
  • .github/skills/vercel-react-best-practices/rules/client-swr-dedup.md
  • .github/skills/vercel-react-best-practices/rules/js-batch-dom-css.md
  • .github/skills/vercel-react-best-practices/rules/js-cache-function-results.md
  • .github/skills/vercel-react-best-practices/rules/js-cache-property-access.md
  • .github/skills/vercel-react-best-practices/rules/js-cache-storage.md
  • .github/skills/vercel-react-best-practices/rules/js-combine-iterations.md
  • .github/skills/vercel-react-best-practices/rules/js-early-exit.md
  • .github/skills/vercel-react-best-practices/rules/js-hoist-regexp.md
  • .github/skills/vercel-react-best-practices/rules/js-index-maps.md
  • .github/skills/vercel-react-best-practices/rules/js-length-check-first.md
  • .github/skills/vercel-react-best-practices/rules/js-min-max-loop.md
  • .github/skills/vercel-react-best-practices/rules/js-set-map-lookups.md
  • .github/skills/vercel-react-best-practices/rules/js-tosorted-immutable.md
  • .github/skills/vercel-react-best-practices/rules/rendering-activity.md
  • .github/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md
  • .github/skills/vercel-react-best-practices/rules/rendering-conditional-render.md
  • .github/skills/vercel-react-best-practices/rules/rendering-content-visibility.md
  • .github/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md
  • .github/skills/vercel-react-best-practices/rules/rendering-hydration-no-flicker.md
  • .github/skills/vercel-react-best-practices/rules/rendering-svg-precision.md
  • .github/skills/vercel-react-best-practices/rules/rerender-defer-reads.md
  • .github/skills/vercel-react-best-practices/rules/rerender-dependencies.md
  • .github/skills/vercel-react-best-practices/rules/rerender-derived-state.md
  • .github/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md
  • .github/skills/vercel-react-best-practices/rules/rerender-lazy-state-init.md
  • .github/skills/vercel-react-best-practices/rules/rerender-memo.md
  • .github/skills/vercel-react-best-practices/rules/rerender-transitions.md
  • .github/skills/vercel-react-best-practices/rules/server-after-nonblocking.md
  • .github/skills/vercel-react-best-practices/rules/server-cache-lru.md
  • .github/skills/vercel-react-best-practices/rules/server-cache-react.md
  • .github/skills/vercel-react-best-practices/rules/server-parallel-fetching.md
  • .github/skills/vercel-react-best-practices/rules/server-serialization.md
  • .github/skills/web-design-guidelines/SKILL.md
  • next.config.ts
  • src/app/api/workspaces/[id]/events/route.ts
  • src/app/api/workspaces/[id]/route.ts
  • src/app/api/workspaces/slug/[slug]/route.ts
  • src/components/landing/FourWays.tsx
  • src/components/landing/ThreeSteps.tsx
  • vercel.json

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

15 issues found across 112 files

Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name=".claude/skills/vercel-react-best-practices/rules/js-cache-storage.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/js-cache-storage.md:49">
P2: Cookie parsing is incorrect when values contain `=` characters. `c.split('=')` will truncate values like `token=abc=def` to just `abc`. Use `c.split('=', 2)` with proper value handling, or split only on the first `=`:
```typescript
document.cookie.split('; ').map(c => {
  const idx = c.indexOf('=')
  return [c.slice(0, idx), c.slice(idx + 1)]
})
```</violation>
</file>

<file name=".github/skills/vercel-react-best-practices/rules/client-event-listeners.md">

<violation number="1" location=".github/skills/vercel-react-best-practices/rules/client-event-listeners.md:57">
P2: The `useSWRSubscription` subscribe function is missing required parameters. According to SWR documentation, it should be `(key, { next }) => unsubscribe`, not `() => unsubscribe`. Even though `key` and `next` aren't used in this example, omitting them doesn't match the API signature and could confuse developers referencing this guide.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/rerender-derived-state.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/rerender-derived-state.md:18">
P2: The JSX `<nav>` element is missing its closing tag `</nav>`, resulting in invalid syntax. Since this is documentation for best practices, the code example should be syntactically correct.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/AGENTS.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/AGENTS.md:428">
P2: The code example calls `setEnabled(false)` but `enabled` is a prop, not state, so `setEnabled` doesn't exist. This would cause a runtime error. Consider either:
- Adding local state: `const [enabled, setEnabled] = useState(initialEnabled)`
- Or using a different error handling approach like `setFrames([])`</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md:15">
P2: For Next.js applications, use `@vercel/analytics/next` instead of `@vercel/analytics/react`. According to Vercel's official documentation, the `/next` export is specifically optimized for Next.js and should be used in RootLayout components.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/rendering-activity.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/rendering-activity.md:15">
P1: The `Activity` component is experimental in React 19 and must be imported as `unstable_Activity`. The current import will fail at runtime.

The correct import is: `import { unstable_Activity as Activity } from 'react'`</violation>
</file>

<file name=".github/skills/vercel-react-best-practices/AGENTS.md">

<violation number="1" location=".github/skills/vercel-react-best-practices/AGENTS.md:428">
P2: The catch handler calls `setEnabled(false)` but `setEnabled` is not defined in this component. The `enabled` parameter is a prop, not state. Consider using a different error handling approach such as setting an error state or clearing frames.</violation>
</file>

<file name=".github/skills/vercel-react-best-practices/rules/bundle-barrel-imports.md">

<violation number="1" location=".github/skills/vercel-react-best-practices/rules/bundle-barrel-imports.md:30">
P2: The lucide-react import paths use internal `dist/esm` structure which may be fragile. The recommended public path is `lucide-react/icons/<icon-name>` instead of `lucide-react/dist/esm/icons/<icon-name>`.</violation>
</file>

<file name=".github/skills/vercel-react-best-practices/rules/bundle-conditional.md">

<violation number="1" location=".github/skills/vercel-react-best-practices/rules/bundle-conditional.md:22">
P1: The `.catch()` handler calls `setEnabled(false)`, but `setEnabled` is not defined. `enabled` is a prop, not state. This example would cause a runtime error. Consider adding error state or using a different error handling approach.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/README.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/README.md:86">
P2: Missing closing code fence for the markdown code block. The ` ```markdown ` block opened to show the rule file structure is never closed, causing all subsequent sections (File Naming Convention, Impact Levels, Scripts, Contributing, Acknowledgments) to render incorrectly as code.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/js-batch-dom-css.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/js-batch-dom-css.md:14">
P2: CSS syntax is not valid TypeScript. This code block mixes CSS and TypeScript but is marked as `typescript`. Split into separate code blocks with appropriate language identifiers for proper syntax highlighting and clarity.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/js-length-check-first.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/js-length-check-first.md:33">
P2: Misleading comment: says "sort/join" but the code doesn't use `.join()` - it uses element-by-element comparison instead. The comment should reflect the actual implementation.</violation>
</file>

<file name=".github/skills/vercel-react-best-practices/rules/bundle-preload.md">

<violation number="1" location=".github/skills/vercel-react-best-practices/rules/bundle-preload.md:50">
P2: This explanation is misleading. The `typeof window !== 'undefined'` check is a runtime check that does NOT affect bundling or code splitting. Bundlers don't analyze runtime conditionals.

For the useEffect example specifically, the window check is unnecessary since `useEffect` only runs on the client.

To actually prevent SSR bundling, use `next/dynamic` with `{ ssr: false }` (as shown in `bundle-dynamic-imports.md`).</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/bundle-conditional.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/bundle-conditional.md:22">
P2: The example code references `setEnabled(false)` in the catch handler, but `setEnabled` is undefined. The component receives `enabled` as a prop, not as state, so there's no setter function available. This would cause a runtime error if developers copy this pattern.</violation>
</file>

<file name=".claude/skills/vercel-react-best-practices/rules/client-swr-dedup.md">

<violation number="1" location=".claude/skills/vercel-react-best-practices/rules/client-swr-dedup.md:48">
P2: Incorrect import syntax: `useSWRMutation` is a default export from `swr/mutation`, not a named export. This should use default import syntax.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

**Usage:**

```tsx
import { Activity } from 'react'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: The Activity component is experimental in React 19 and must be imported as unstable_Activity. The current import will fail at runtime.

The correct import is: import { unstable_Activity as Activity } from 'react'

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/rendering-activity.md, line 15:

<comment>The `Activity` component is experimental in React 19 and must be imported as `unstable_Activity`. The current import will fail at runtime.

The correct import is: `import { unstable_Activity as Activity } from 'react'`</comment>

<file context>
@@ -0,0 +1,26 @@
+**Usage:**
+
+```tsx
+import { Activity } from 'react'
+
+function Dropdown({ isOpen }: Props) {
</file context>
Suggested change
import { Activity } from 'react'
import { unstable_Activity as Activity } from 'react'

if (enabled && !frames && typeof window !== 'undefined') {
import('./animation-frames.js')
.then(mod => setFrames(mod.frames))
.catch(() => setEnabled(false))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1: The .catch() handler calls setEnabled(false), but setEnabled is not defined. enabled is a prop, not state. This example would cause a runtime error. Consider adding error state or using a different error handling approach.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/skills/vercel-react-best-practices/rules/bundle-conditional.md, line 22:

<comment>The `.catch()` handler calls `setEnabled(false)`, but `setEnabled` is not defined. `enabled` is a prop, not state. This example would cause a runtime error. Consider adding error state or using a different error handling approach.</comment>

<file context>
@@ -0,0 +1,31 @@
+    if (enabled && !frames && typeof window !== 'undefined') {
+      import('./animation-frames.js')
+        .then(mod => setFrames(mod.frames))
+        .catch(() => setEnabled(false))
+    }
+  }, [enabled, frames])
</file context>

function getCookie(name: string) {
if (!cookieCache) {
cookieCache = Object.fromEntries(
document.cookie.split('; ').map(c => c.split('='))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Cookie parsing is incorrect when values contain = characters. c.split('=') will truncate values like token=abc=def to just abc. Use c.split('=', 2) with proper value handling, or split only on the first =:

document.cookie.split('; ').map(c => {
  const idx = c.indexOf('=')
  return [c.slice(0, idx), c.slice(idx + 1)]
})
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/js-cache-storage.md, line 49:

<comment>Cookie parsing is incorrect when values contain `=` characters. `c.split('=')` will truncate values like `token=abc=def` to just `abc`. Use `c.split('=', 2)` with proper value handling, or split only on the first `=`:
```typescript
document.cookie.split('; ').map(c => {
  const idx = c.indexOf('=')
  return [c.slice(0, idx), c.slice(idx + 1)]
})
```</comment>

<file context>
@@ -0,0 +1,70 @@
+function getCookie(name: string) {
+  if (!cookieCache) {
+    cookieCache = Object.fromEntries(
+      document.cookie.split('; ').map(c => c.split('='))
+    )
+  }
</file context>

}
}, [key, callback])

useSWRSubscription('global-keydown', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The useSWRSubscription subscribe function is missing required parameters. According to SWR documentation, it should be (key, { next }) => unsubscribe, not () => unsubscribe. Even though key and next aren't used in this example, omitting them doesn't match the API signature and could confuse developers referencing this guide.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/skills/vercel-react-best-practices/rules/client-event-listeners.md, line 57:

<comment>The `useSWRSubscription` subscribe function is missing required parameters. According to SWR documentation, it should be `(key, { next }) => unsubscribe`, not `() => unsubscribe`. Even though `key` and `next` aren't used in this example, omitting them doesn't match the API signature and could confuse developers referencing this guide.</comment>

<file context>
@@ -0,0 +1,74 @@
+    }
+  }, [key, callback])
+
+  useSWRSubscription('global-keydown', () => {
+    const handler = (e: KeyboardEvent) => {
+      if (e.metaKey && keyCallbacks.has(e.key)) {
</file context>
Suggested change
useSWRSubscription('global-keydown', () => {
useSWRSubscription('global-keydown', (_key, { next }) => {

function Sidebar() {
const width = useWindowWidth() // updates continuously
const isMobile = width < 768
return <nav className={isMobile ? 'mobile' : 'desktop'}>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The JSX <nav> element is missing its closing tag </nav>, resulting in invalid syntax. Since this is documentation for best practices, the code example should be syntactically correct.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/rerender-derived-state.md, line 18:

<comment>The JSX `<nav>` element is missing its closing tag `</nav>`, resulting in invalid syntax. Since this is documentation for best practices, the code example should be syntactically correct.</comment>

<file context>
@@ -0,0 +1,29 @@
+function Sidebar() {
+  const width = useWindowWidth()  // updates continuously
+  const isMobile = width < 768
+  return <nav className={isMobile ? 'mobile' : 'desktop'}>
+}
+```
</file context>


**Incorrect (multiple reflows):**

```typescript

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: CSS syntax is not valid TypeScript. This code block mixes CSS and TypeScript but is marked as typescript. Split into separate code blocks with appropriate language identifiers for proper syntax highlighting and clarity.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/js-batch-dom-css.md, line 14:

<comment>CSS syntax is not valid TypeScript. This code block mixes CSS and TypeScript but is marked as `typescript`. Split into separate code blocks with appropriate language identifiers for proper syntax highlighting and clarity.</comment>

<file context>
@@ -0,0 +1,82 @@
+
+**Incorrect (multiple reflows):**
+
+```typescript
+function updateElementStyles(element: HTMLElement) {
+  // Each line triggers a reflow
</file context>

if (current.length !== original.length) {
return true
}
// Only sort/join when lengths match

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Misleading comment: says "sort/join" but the code doesn't use .join() - it uses element-by-element comparison instead. The comment should reflect the actual implementation.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/js-length-check-first.md, line 33:

<comment>Misleading comment: says "sort/join" but the code doesn't use `.join()` - it uses element-by-element comparison instead. The comment should reflect the actual implementation.</comment>

<file context>
@@ -0,0 +1,49 @@
+  if (current.length !== original.length) {
+    return true
+  }
+  // Only sort/join when lengths match
+  const currentSorted = current.toSorted()
+  const originalSorted = original.toSorted()
</file context>
Suggested change
// Only sort/join when lengths match
// Only sort when lengths match, then compare element by element

}
```

The `typeof window !== 'undefined'` check prevents bundling preloaded modules for SSR, optimizing server bundle size and build speed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: This explanation is misleading. The typeof window !== 'undefined' check is a runtime check that does NOT affect bundling or code splitting. Bundlers don't analyze runtime conditionals.

For the useEffect example specifically, the window check is unnecessary since useEffect only runs on the client.

To actually prevent SSR bundling, use next/dynamic with { ssr: false } (as shown in bundle-dynamic-imports.md).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/skills/vercel-react-best-practices/rules/bundle-preload.md, line 50:

<comment>This explanation is misleading. The `typeof window !== 'undefined'` check is a runtime check that does NOT affect bundling or code splitting. Bundlers don't analyze runtime conditionals.

For the useEffect example specifically, the window check is unnecessary since `useEffect` only runs on the client.

To actually prevent SSR bundling, use `next/dynamic` with `{ ssr: false }` (as shown in `bundle-dynamic-imports.md`).</comment>

<file context>
@@ -0,0 +1,50 @@
+}
+```
+
+The `typeof window !== 'undefined'` check prevents bundling preloaded modules for SSR, optimizing server bundle size and build speed.
</file context>

if (enabled && !frames && typeof window !== 'undefined') {
import('./animation-frames.js')
.then(mod => setFrames(mod.frames))
.catch(() => setEnabled(false))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: The example code references setEnabled(false) in the catch handler, but setEnabled is undefined. The component receives enabled as a prop, not as state, so there's no setter function available. This would cause a runtime error if developers copy this pattern.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/bundle-conditional.md, line 22:

<comment>The example code references `setEnabled(false)` in the catch handler, but `setEnabled` is undefined. The component receives `enabled` as a prop, not as state, so there's no setter function available. This would cause a runtime error if developers copy this pattern.</comment>

<file context>
@@ -0,0 +1,31 @@
+    if (enabled && !frames && typeof window !== 'undefined') {
+      import('./animation-frames.js')
+        .then(mod => setFrames(mod.frames))
+        .catch(() => setEnabled(false))
+    }
+  }, [enabled, frames])
</file context>

**For mutations:**

```tsx
import { useSWRMutation } from 'swr/mutation'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Incorrect import syntax: useSWRMutation is a default export from swr/mutation, not a named export. This should use default import syntax.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .claude/skills/vercel-react-best-practices/rules/client-swr-dedup.md, line 48:

<comment>Incorrect import syntax: `useSWRMutation` is a default export from `swr/mutation`, not a named export. This should use default import syntax.</comment>

<file context>
@@ -0,0 +1,56 @@
+**For mutations:**
+
+```tsx
+import { useSWRMutation } from 'swr/mutation'
+
+function UpdateButton() {
</file context>
Suggested change
import { useSWRMutation } from 'swr/mutation'
import useSWRMutation from 'swr/mutation'

@urjitc
urjitc merged commit 51965d3 into main Jan 15, 2026
3 of 4 checks passed
@urjitc
urjitc deleted the refactor/no-ref/performance-optimizations-and-security-headers branch January 15, 2026 01:51
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Dev Board Jan 15, 2026
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
11.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@ellipsis-dev ellipsis-dev Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to 9379d75 in 9 minutes and 40 seconds. Click for details.
  • Reviewed 10781 lines of code in 112 files
  • Skipped 0 files when reviewing.
  • Skipped posting 12 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .claude/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md:44
  • Draft comment:
    Good example! Note that with the latest React Compiler, manual hoisting isn’t needed as static JSX is auto-hoisted.
  • Reason this comment was not posted:
    Confidence changes required: 10% <= threshold 50% None
2. .claude/skills/vercel-react-best-practices/rules/rendering-hydration-no-flicker.md:80
  • Draft comment:
    Inline script injection via dangerouslySetInnerHTML is effective here; ensure that any content used in the script is controlled to mitigate XSS risks.
  • Reason this comment was not posted:
    Confidence changes required: 20% <= threshold 50% None
3. .claude/skills/vercel-react-best-practices/rules/rerender-defer-reads.md:29
  • Draft comment:
    Good demonstration of reading URL parameters on demand instead of subscribing to changes; this avoids unnecessary re-renders.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
4. .claude/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md:12
  • Draft comment:
    Clear examples on using functional updates; these reduce stale closures and unnecessary dependency array dependencies.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. .claude/skills/vercel-react-best-practices/rules/server-cache-lru.md:35
  • Draft comment:
    Solid implementation using LRUCache. Consider adding error handling or logging if the database query fails to improve debuggability.
  • Reason this comment was not posted:
    Confidence changes required: 10% <= threshold 50% None
6. next.config.ts:10
  • Draft comment:
    Experimental optimizePackageImports is enabled. Verify compatibility with your production environment as this feature may have changing behavior.
  • Reason this comment was not posted:
    Confidence changes required: 10% <= threshold 50% None
7. src/app/api/workspaces/[id]/events/route.ts:230
  • Draft comment:
    Good use of parallel operations using Promise.all and independent promises. Snapshot creation is handled non-blocking; ensure its errors are logged appropriately.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
8. src/app/api/workspaces/[id]/route.ts:81
  • Draft comment:
    Parallel fetching of request params, headers, and body is well done. Ownership and error checks are clear.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
9. src/components/landing/FourWays.tsx:240
  • Draft comment:
    The GSAP timeline and ScrollTrigger implementation appear solid. Double-check that the trigger kill logic doesn’t inadvertently affect other scroll triggers.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
10. vercel.json:87
  • Draft comment:
    Header configuration looks comprehensive. Review the ordering of Cache-Control rules to ensure static assets are not overridden by the catch-all rule.
  • Reason this comment was not posted:
    Confidence changes required: 10% <= threshold 50% None
11. .github/skills/vercel-react-best-practices/AGENTS.md:428
  • Draft comment:
    Typo: In the AnimationPlayer code snippet, the catch block calls setEnabled(false), but there’s no state updater defined for "enabled". Likely, this is a typo and should be corrected (for example, to reset frames or trigger an error state appropriately).
  • Reason this comment was not posted:
    Marked as duplicate.
12. .github/skills/vercel-react-best-practices/rules/async-defer-await.md:19
  • Draft comment:
    Typographical suggestion: In the comment "Returns immediately but still waited for userData", consider using "waits" instead of "waited" for consistency with the present tense (e.g., "Returns immediately but still waits for userData").
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a purely stylistic/grammatical comment about documentation text. While the grammar suggestion is technically correct (present tense would be more consistent), this falls under "obvious or unimportant" comments. The rules explicitly state "Do NOT make comments that are obvious or unimportant" and "Do NOT comment unless there is clearly a code change required." A minor tense inconsistency in a comment doesn't require a code change - it's a trivial stylistic issue. The meaning is perfectly clear either way. This type of minor grammatical nitpick doesn't add meaningful value to the review. The grammar is technically incorrect and fixing it would improve consistency. Documentation quality matters, and maintaining proper grammar in teaching materials (which this appears to be) could be considered important for professionalism and clarity. While documentation quality does matter, this is an extremely minor issue that doesn't affect understanding or functionality. The rules explicitly prioritize avoiding obvious or unimportant comments, and a single word tense inconsistency in a code comment falls squarely into that category. This is the type of nitpick that would typically be caught by a spell-checker or style guide automation, not a code review. This comment should be deleted. It's a minor grammatical nitpick that doesn't meet the threshold for requiring a code change. While technically correct, it's too trivial and obvious to warrant inclusion in the review.

Workflow ID: wflow_owhawTcHbLWhg6Ct

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

if (enabled && !frames && typeof window !== 'undefined') {
import('./animation-frames.js')
.then(mod => setFrames(mod.frames))
.catch(() => setEnabled(false))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo: In the .catch callback, setEnabled is used but no such function is defined. Perhaps this is meant to be setFrames or another valid state setter?

Suggested change
.catch(() => setEnabled(false))
.catch(() => setFrames(null))

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant