Added CookieKit provider to benchmarks - feat: add with-cookiekit Next.js benchmark app#3
Conversation
…ing dependencies for React, React DOM, and React Cookie Manager. Update pnpm-lock.yaml to reflect new dependencies and devDependencies for TypeScript and related packages.
|
@JackHyp is attempting to deploy a commit to the Consent Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a new Next.js benchmark app “with-cookiekit” including a client RootLayout wrapping children with CookieManager, a simple Home page, project configs (Next.js, TypeScript), a benchmark config JSON, and a package manifest with scripts and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant N as Next.js App
participant L as RootLayout (client)
participant C as CookieManager
participant P as Home Page
U->>N: Request /
N->>L: Render layout
L->>C: Wrap children with CookieManager
C->>P: Render page content
P-->>C: Static markup (h1 "Benchmark")
C-->>L: Children wrapped
L-->>N: HTML with <html lang="en" suppressHydrationWarning>
N-->>U: Response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
benchmarks/with-cookiekit/app/favicon.icois excluded by!**/*.icopnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
benchmarks/with-cookiekit/app/layout.tsx(1 hunks)benchmarks/with-cookiekit/app/page.tsx(1 hunks)benchmarks/with-cookiekit/config.json(1 hunks)benchmarks/with-cookiekit/next-env.d.ts(1 hunks)benchmarks/with-cookiekit/next.config.ts(1 hunks)benchmarks/with-cookiekit/package.json(1 hunks)benchmarks/with-cookiekit/tsconfig.json(1 hunks)
🔇 Additional comments (5)
benchmarks/with-cookiekit/package.json (2)
14-18: Confirm Next/React/Cookie Manager compatibility.You’re pinning Next 15.3.3 with React 19 and react-cookie-manager 3.8.1. Please confirm this trio is certified-compatible (SSR/streaming, RSC boundaries, DOM APIs).
Would you like me to check upstream release notes and compatibility matrices?
11-11: No change required — bin name matches "benchmark-cli".
packages/cli/package.json defines "bin": { "benchmark-cli": "dist/index.mjs" }, so the script "pnpm exec benchmark-cli benchmark" is correct.Likely an incorrect or invalid review comment.
benchmarks/with-cookiekit/config.json (1)
5-16: Ensure banner selectors actually render in the DOM — add a stable class/data attribute to the CookieManager wrapper.benchmarks/with-cookiekit/config.json expects selectors ".cookie-manager", ".react-cookie-manager", and "[data-react-cookie-manager]"; I could not locate any component in this repo that emits those selectors — either add className="react-cookie-manager" and/or data-react-cookie-manager on the CookieManager wrapper, or update the config to match the real DOM.
benchmarks/with-cookiekit/tsconfig.json (1)
1-11: TS config looks fine for a Next app.benchmarks/with-cookiekit/next-env.d.ts (1)
1-5: Standard Next env types — LGTM.
| 'use client'; | ||
|
|
||
| /* | ||
| * If you're using Next.js, we recommend installing the @c15t/nextjs package. | ||
| * The Next.js package is a wrapper around the React package that provides | ||
| * additional features for Next.js. | ||
| */ | ||
|
|
||
| import { | ||
| CookieManager | ||
| } from 'react-cookie-manager'; | ||
| import type { ReactNode } from 'react'; | ||
|
|
||
|
|
||
| export default function RootLayout({ | ||
| children, | ||
| }: Readonly<{ | ||
| children: ReactNode; | ||
| }>) { | ||
| return ( | ||
| <html lang="en" suppressHydrationWarning> | ||
| <body> | ||
| <CookieManager>{children}</CookieManager> | ||
| </body> | ||
| </html> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make layout a Server Component; move CookieManager into a client Providers wrapper.
Marking the root layout as client disables RSC/streaming optimizations. Keep the layout server-side and render a small client Providers wrapper that hosts CookieManager with stable selectors.
Apply to this file:
-'use client';
-
-/*
- * If you're using Next.js, we recommend installing the @c15t/nextjs package.
- * The Next.js package is a wrapper around the React package that provides
- * additional features for Next.js.
- */
-
-import {
- CookieManager
-} from 'react-cookie-manager';
-import type { ReactNode } from 'react';
+import type { ReactNode } from 'react';
+import { Providers } from './providers';
export default function RootLayout({
children,
}: Readonly<{
children: ReactNode;
}>) {
return (
<html lang="en" suppressHydrationWarning>
<body>
- <CookieManager>{children}</CookieManager>
+ <Providers>{children}</Providers>
</body>
</html>
);
}Add a new client wrapper (outside this hunk) to ensure benchmark selectors exist:
// benchmarks/with-cookiekit/app/providers.tsx
'use client';
import type { ReactNode } from 'react';
import { CookieManager } from 'react-cookie-manager';
export function Providers({ children }: { children: ReactNode }) {
return (
<CookieManager className="react-cookie-manager" data-react-cookie-manager="">
{children}
</CookieManager>
);
}Optional: remove or update the comment referencing @c15t/nextjs unless you intend to add that package.
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/app/layout.tsx lines 1-27, the root layout is
marked 'use client' which disables server components and streaming; remove the
'use client' directive so the layout is a Server Component, and replace the
inline CookieManager usage by importing and wrapping children with a new client
Providers wrapper. Create a new file benchmarks/with-cookiekit/app/providers.tsx
(a client component) that exports a Providers component which renders
CookieManager with the stable className "react-cookie-manager" and
data-react-cookie-manager="" attribute around its children, then update
layout.tsx to import Providers and render <Providers>{children}</Providers>
inside <body>; optionally remove or update the comment about @c15t/nextjs.
| export default function Home() { | ||
| return ( | ||
| <div> | ||
| <h1>Benchmark</h1> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use semantic markup for the main region.
Switch the wrapper to <main> for basic semantics. No behavior change.
export default function Home() {
return (
- <div>
+ <main>
<h1>Benchmark</h1>
- </div>
+ </main>
);
}📝 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.
| export default function Home() { | |
| return ( | |
| <div> | |
| <h1>Benchmark</h1> | |
| </div> | |
| ); | |
| } | |
| export default function Home() { | |
| return ( | |
| <main> | |
| <h1>Benchmark</h1> | |
| </main> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/app/page.tsx around lines 1 to 7, the top-level
wrapper uses a generic <div> but should use a semantic <main> element; update
the JSX to replace the wrapper div with a main element (keeping the same
children and layout) so the page has correct main-region semantics and no
behavioral changes.
| "serviceName": "Cookiekit" | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Unify service naming (“CookieKit”).
The PR title uses “CookieKit”, but config has "serviceName": "Cookiekit" and tag "cookiekit". Standardize capitalization to avoid fragmented analytics.
- "serviceName": "Cookiekit"
+ "serviceName": "CookieKit"Also applies to: 46-46
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/config.json around lines 15-16 (also applies to
line 46), the serviceName and tag use inconsistent capitalization ("Cookiekit"
and "cookiekit") compared to the PR title "CookieKit"; update the JSON values to
use the canonical "CookieKit" (and the corresponding tag to "cookiekit" or the
desired canonical casing) so naming is standardized across config and tags;
ensure both serviceName and any tag entries at line 46 match the chosen
"CookieKit" capitalization.
| "techStack": { | ||
| "bundler": "vite", | ||
| "bundleType": ["esm", "cjs"], | ||
| "frameworks": ["react"], | ||
| "languages": ["typescript", "javascript"], | ||
| "packageManager": "pnpm", | ||
| "typescript": true | ||
| }, |
There was a problem hiding this comment.
Incorrect bundler metadata for a Next.js app.
This package is a Next app; declaring "bundler": "vite" is misleading and may skew reports. Use “next” (or “webpack/turbopack” if your schema expects the underlying engine).
- "bundler": "vite",
- "bundleType": ["esm", "cjs"],
- "frameworks": ["react"],
+ "bundler": "next",
+ "bundleType": ["esm"],
+ "frameworks": ["nextjs", "react"],📝 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.
| "techStack": { | |
| "bundler": "vite", | |
| "bundleType": ["esm", "cjs"], | |
| "frameworks": ["react"], | |
| "languages": ["typescript", "javascript"], | |
| "packageManager": "pnpm", | |
| "typescript": true | |
| }, | |
| "techStack": { | |
| "bundler": "next", | |
| "bundleType": ["esm"], | |
| "frameworks": ["nextjs", "react"], | |
| "languages": ["typescript", "javascript"], | |
| "packageManager": "pnpm", | |
| "typescript": true | |
| }, |
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/config.json around lines 21 to 28, the bundler is
incorrectly set to "vite" for a Next.js app; update the "techStack.bundler"
field to "next" (or to the underlying engine expected by your schema such as
"webpack" or "turbopack") and adjust or remove "bundleType" if it doesn't apply
to Next so the metadata accurately reflects this is a Next app.
| const nextConfig: NextConfig = {}; | ||
|
|
||
| export default nextConfig; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider enabling strict mode for clearer dev signals.
Minimal config is fine for a benchmark, but turn on React Strict Mode for better warnings.
-const nextConfig: NextConfig = {};
+const nextConfig: NextConfig = {
+ reactStrictMode: true,
+};📝 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.
| const nextConfig: NextConfig = {}; | |
| export default nextConfig; | |
| const nextConfig: NextConfig = { | |
| reactStrictMode: true, | |
| }; | |
| export default nextConfig; |
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/next.config.ts around lines 3 to 5, the exported
NextConfig is minimal and does not enable React Strict Mode; update the
nextConfig object to include reactStrictMode: true (e.g. const nextConfig:
NextConfig = { reactStrictMode: true };) so the app runs with React Strict Mode
enabled and surfaces stronger dev warnings.
| "dev": "next dev --port 3001", | ||
| "build": "next build", | ||
| "start": "next start", | ||
| "lint": "next lint", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align dev/start ports to avoid harness mismatches.
dev runs on 3001 but start defaults to 3000. Use the same port for both.
- "dev": "next dev --port 3001",
+ "dev": "next dev --port 3001",
"build": "next build",
- "start": "next start",
+ "start": "next start --port 3001",📝 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.
| "dev": "next dev --port 3001", | |
| "build": "next build", | |
| "start": "next start", | |
| "lint": "next lint", | |
| "dev": "next dev --port 3001", | |
| "build": "next build", | |
| "start": "next start --port 3001", | |
| "lint": "next lint", |
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/package.json around lines 6 to 9 the "dev" script
uses port 3001 while "start" uses the default 3000 causing harness mismatches;
update the "start" script to explicitly use the same port as "dev" (e.g. change
it to run "next start --port 3001") so both dev and start use port 3001.
| "build": "next build", | ||
| "start": "next start", | ||
| "lint": "next lint", | ||
| "fmt": "pnpm biome format --write . && pnpm biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix fmt script to use the Biome binary (and ensure it exists).
pnpm biome will fail unless you’ve aliased it at the workspace root. Use pnpm exec and ensure @biomejs/biome is installed (root or local).
- "fmt": "pnpm biome format --write . && pnpm biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write",
+ "fmt": "pnpm exec biome format --write . && pnpm exec biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write",Run to verify the CLI availability and whether Biome is declared anywhere in the workspace:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for Biome in workspace devDependencies..."
rg -nP '"@biomejs/biome"\s*:\s*".+?"' -g 'package.json' -C1 || trueLength of output: 184
Fix fmt script to use pnpm exec for Biome
Root package.json contains @biomejs/biome@1.9.4; still prefer using pnpm exec to reliably resolve the CLI from any workspace package.
- "fmt": "pnpm biome format --write . && pnpm biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write",
+ "fmt": "pnpm exec biome format --write . && pnpm exec biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write",📝 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.
| "fmt": "pnpm biome format --write . && pnpm biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write", | |
| "fmt": "pnpm exec biome format --write . && pnpm exec biome check --formatter-enabled=false --linter-enabled=false --organize-imports-enabled=true --write", |
🤖 Prompt for AI Agents
In benchmarks/with-cookiekit/package.json around line 10, the "fmt" script
currently invokes the Biome CLI directly; update it to use "pnpm exec" so the
workspace-resolved CLI is used reliably: replace the direct "pnpm biome"
invocations with "pnpm exec biome" for both the format and check commands
(preserving all flags --write, --formatter-enabled=false,
--linter-enabled=false, --organize-imports-enabled=true), ensuring the script
runs the CLI via pnpm exec.
…on and pnpm-lock.yaml for 'with-cookiekit' benchmark project.
1 participant
Summary by CodeRabbit
New Features
Chores