Skip to content

feat(nextjs): add 7 new Next.js best-practice rules from vercel-labs skill#680

Open
devin-ai-integration[bot] wants to merge 9 commits into
mainfrom
devin/1780571023-nextjs-best-practice-rules
Open

feat(nextjs): add 7 new Next.js best-practice rules from vercel-labs skill#680
devin-ai-integration[bot] wants to merge 9 commits into
mainfrom
devin/1780571023-nextjs-best-practice-rules

Conversation

@devin-ai-integration
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot commented Jun 4, 2026

Summary

Four new Next.js rules derived from the vercel-labs/next-skills/next-best-practices skill, covering gaps in error handling, route handlers, and script optimization:

nextjs-error-boundary-missing-use-client (error) — error.tsx/global-error.tsx in /app/ without 'use client' silently fails to catch errors. Detection: filename pattern + hasDirective.

nextjs-global-error-missing-html-body (error) — global-error.tsx must render <html>+<body> since the root layout unmounts on error. Detection: walkAst for JSX tag names.

nextjs-no-default-export-in-route-handler (error) — export default in route.ts is silently ignored; Next.js only recognizes named HTTP method exports (GET, POST, etc.). Detection: ExportDefaultDeclaration + scan for named HTTP method exports via programHasNamedHttpMethodExport.

nextjs-no-google-analytics-script (warn) — Manual <script>/<Script> loading Google Analytics hurts performance; @next/third-parties/google loads optimally. Detection: src attribute matching GOOGLE_ANALYTICS_SCRIPT_PATTERN.

Also extracts ROUTE_HANDLER_HTTP_METHODS from local constant in server-hoist-static-io.ts into constants/nextjs.ts to deduplicate.

Link to Devin session: https://app.devin.ai/sessions/653e55fe57524520a26e1f8fa57eafa2
Requested by: @aidenybai


Note

Low Risk
Additive static-analysis rules and shared constants only; no runtime or security-sensitive behavior changes in consumer apps.

Overview
Adds seven Next.js-focused oxlint rules to react-doctor, wired through generated rule-registry entries and shared patterns in constants/nextjs.ts.

Error handling & App Router conventions: flags error.tsx / global-error.tsx under app/ without 'use client', and global-error.tsx missing <html> / <body> (via new fileContainsJsxElements helper). Route handlers: warns when route.ts uses export default (or re-exports default) without named GET/POST/… exports. Scripts & OG: errors on <Script> nested in <Head>, warns on manual GA script tags, @vercel/og imports, and runtime = 'edge' on opengraph/twitter image routes.

ROUTE_HANDLER_HTTP_METHODS is centralized in nextjs.ts and reused by server-hoist-static-io (replacing a local duplicate).

Reviewed by Cursor Bugbot for commit 161a6c5. Bugbot is set up for automated code reviews on this repo. Configure here.


Open in Devin Review

- nextjs-error-boundary-missing-use-client: error.tsx/global-error.tsx must have 'use client'
- nextjs-global-error-missing-html-body: global-error.tsx must render <html> and <body>
- nextjs-no-default-export-in-route-handler: route.ts must use named HTTP method exports
- nextjs-no-google-analytics-script: use @next/third-parties instead of manual GA scripts

Also extract ROUTE_HANDLER_HTTP_METHODS to constants/nextjs.ts to deduplicate with server-hoist-static-io.ts

Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Jun 4, 2026

Open in StackBlitz

npm i https://pkg.pr.new/eslint-plugin-react-doctor@680
npm i https://pkg.pr.new/oxlint-plugin-react-doctor@680
npm i https://pkg.pr.new/react-doctor@680

commit: 161a6c5

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

No React Doctor issues found. 🎉

Reviewed by React Doctor for commit 161a6c5.

Comment thread packages/oxlint-plugin-react-doctor/src/plugin/constants/nextjs.ts Outdated
@aidenybai
Copy link
Copy Markdown
Member

/rde parity

@react-doctor-evals
Copy link
Copy Markdown

react-doctor-evals Bot commented Jun 4, 2026

Parity OK — no diagnostic differences.

Baseline: main · This PR: devin/1780571023-nextjs-best-practice-rules (7cb2ba4)

ℹ️ Re-run this parity check by commenting /rde parity on this PR.

trace ca7f0fe0698b271ead59c6ee5abdc8a7 · rde

Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
Copy link
Copy Markdown
Contributor Author

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines +56 to +57
export const GOOGLE_ANALYTICS_SCRIPT_PATTERN =
/google-analytics\.com|googletagmanager\.com\/gtag|www\.googletagmanager\.com/;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🟡 GOOGLE_ANALYTICS_SCRIPT_PATTERN regex matches Google Tag Manager scripts, producing misleading diagnostics

The third alternative in the regex (www\.googletagmanager\.com) matches all scripts from www.googletagmanager.com, including Google Tag Manager scripts (e.g. https://www.googletagmanager.com/gtm.js?id=GTM-XXXXX). These are not Google Analytics scripts, yet the rule fires with the message "Manual Google Analytics script blocks rendering" and recommends importing GoogleAnalytics from @next/third-parties/google. For a GTM script, the correct component would be GoogleTagManager. Additionally, this matching is inconsistent — GTM scripts from googletagmanager.com (without www.) are not caught, as confirmed by testing the regex.

Regex test results
  • www.googletagmanager.com/gtm.js?id=GTM-XXX → matches (false positive, it's GTM not GA)
  • googletagmanager.com/gtm.js?id=GTM-XXX → does NOT match (inconsistent)
  • www.googletagmanager.com/gtag/js?id=G-XXX → matches (correct, GA4)
  • www.google-analytics.com/analytics.js → matches (correct, old GA)
Suggested change
export const GOOGLE_ANALYTICS_SCRIPT_PATTERN =
/google-analytics\.com|googletagmanager\.com\/gtag|www\.googletagmanager\.com/;
export const GOOGLE_ANALYTICS_SCRIPT_PATTERN =
/google-analytics\.com|googletagmanager\.com\/gtag/;
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — the third alternative matched all GTM scripts, not just GA. Fixed by narrowing to google-analytics\.com|googletagmanager\.com\/gtag which only matches GA4 (/gtag/js) and legacy GA (google-analytics.com).

devin-ai-integration Bot and others added 2 commits June 4, 2026 11:18
Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
…rt, no-edge-og-runtime

- nextjs-no-script-in-head: <Script> inside <Head> is silently ignored
- nextjs-no-vercel-og-import: use next/og instead of @vercel/og
- nextjs-no-edge-og-runtime: avoid Edge runtime in OG image files

Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
@devin-ai-integration devin-ai-integration Bot changed the title feat(nextjs): add 4 new Next.js best-practice rules from vercel-labs skill feat(nextjs): add 7 new Next.js best-practice rules from vercel-labs skill Jun 4, 2026
devin-ai-integration Bot and others added 2 commits June 4, 2026 11:26
Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
- fix: nextjs-no-script-in-head skip self-closing <Head /> to avoid false positives
- refactor: extract fileContainsJsxElements to utils/ (one-utility-per-file)
- refactor: global-error rule walks AST once instead of twice
- refactor: no-edge-og-runtime hoists filename check to Program handler

Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
Comment thread packages/oxlint-plugin-react-doctor/src/plugin/constants/nextjs.ts Outdated
devin-ai-integration Bot and others added 2 commits June 4, 2026 12:12
- target/found → targetTagNames/foundTagNames in fileContainsJsxElements
- tag → tagName in filter/map callbacks
- REQUIRED_TAGS → REQUIRED_HTML_TAGS

Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit aeec7ff. Configure here.

Co-Authored-By: Aiden Bai <aiden.bai05@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant