-
Notifications
You must be signed in to change notification settings - Fork 387
feat(nextjs): add 7 new Next.js best-practice rules from vercel-labs skill #680
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 1 commit
7cb2ba4
d77b116
253417e
197c11b
92b337b
9967e6e
78f0f66
aeec7ff
161a6c5
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -38,3 +38,20 @@ export const MUTATING_ROUTE_SEGMENTS = new Set([ | |||||||||
| "cancel", | ||||||||||
| "deactivate", | ||||||||||
| ]); | ||||||||||
|
|
||||||||||
| export const ERROR_BOUNDARY_FILE_PATTERN = /\/(error|global-error)\.(tsx?|jsx?)$/; | ||||||||||
|
|
||||||||||
| export const GLOBAL_ERROR_FILE_PATTERN = /\/global-error\.(tsx?|jsx?)$/; | ||||||||||
|
|
||||||||||
| export const ROUTE_HANDLER_HTTP_METHODS = new Set([ | ||||||||||
| "GET", | ||||||||||
| "POST", | ||||||||||
| "PUT", | ||||||||||
| "PATCH", | ||||||||||
| "DELETE", | ||||||||||
| "OPTIONS", | ||||||||||
| "HEAD", | ||||||||||
| ]); | ||||||||||
|
|
||||||||||
| export const GOOGLE_ANALYTICS_SCRIPT_PATTERN = | ||||||||||
| /google-analytics\.com|googletagmanager\.com\/gtag|www\.googletagmanager\.com/; | ||||||||||
|
Contributor
Author
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. 🟡 GOOGLE_ANALYTICS_SCRIPT_PATTERN regex matches Google Tag Manager scripts, producing misleading diagnostics The third alternative in the regex ( Regex test results
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
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. Good catch — the third alternative matched all GTM scripts, not just GA. Fixed by narrowing to |
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { APP_DIRECTORY_PATTERN, ERROR_BOUNDARY_FILE_PATTERN } from "../../constants/nextjs.js"; | ||
| import { defineRule } from "../../utils/define-rule.js"; | ||
| import { hasDirective } from "../../utils/has-directive.js"; | ||
| import { normalizeFilename } from "../../utils/normalize-filename.js"; | ||
| import type { Rule } from "../../utils/rule.js"; | ||
| import type { RuleContext } from "../../utils/rule-context.js"; | ||
| import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; | ||
|
|
||
| export const nextjsErrorBoundaryMissingUseClient = defineRule<Rule>({ | ||
| id: "nextjs-error-boundary-missing-use-client", | ||
| title: "Error boundary missing 'use client'", | ||
| tags: ["test-noise"], | ||
| requires: ["nextjs"], | ||
| severity: "error", | ||
| recommendation: | ||
| "Add `'use client'` at the top of this file — error boundaries must be Client Components to catch and render fallback UI", | ||
| create: (context: RuleContext) => ({ | ||
| Program(programNode: EsTreeNodeOfType<"Program">) { | ||
| const filename = normalizeFilename(context.filename ?? ""); | ||
| if (!APP_DIRECTORY_PATTERN.test(filename)) return; | ||
| if (!ERROR_BOUNDARY_FILE_PATTERN.test(filename)) return; | ||
| if (hasDirective(programNode, "use client")) return; | ||
|
|
||
| context.report({ | ||
| node: programNode, | ||
| message: | ||
| "This error boundary silently does nothing without 'use client' — Next.js requires error.tsx to be a Client Component.", | ||
| }); | ||
| }, | ||
| }), | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { APP_DIRECTORY_PATTERN, GLOBAL_ERROR_FILE_PATTERN } from "../../constants/nextjs.js"; | ||
| import { defineRule } from "../../utils/define-rule.js"; | ||
| import { normalizeFilename } from "../../utils/normalize-filename.js"; | ||
| import { walkAst } from "../../utils/walk-ast.js"; | ||
| import type { EsTreeNode } from "../../utils/es-tree-node.js"; | ||
| import type { Rule } from "../../utils/rule.js"; | ||
| import type { RuleContext } from "../../utils/rule-context.js"; | ||
| import { isNodeOfType } from "../../utils/is-node-of-type.js"; | ||
| import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; | ||
|
|
||
| const fileContainsJsxElement = (programNode: EsTreeNode, tagName: string): boolean => { | ||
| let didFind = false; | ||
| walkAst(programNode, (child: EsTreeNode) => { | ||
| if (didFind) return false; | ||
| if ( | ||
| isNodeOfType(child, "JSXOpeningElement") && | ||
| isNodeOfType(child.name, "JSXIdentifier") && | ||
| child.name.name === tagName | ||
| ) { | ||
| didFind = true; | ||
| return false; | ||
| } | ||
| }); | ||
| return didFind; | ||
| }; | ||
|
|
||
| export const nextjsGlobalErrorMissingHtmlBody = defineRule<Rule>({ | ||
| id: "nextjs-global-error-missing-html-body", | ||
| title: "global-error.tsx missing <html>/<body>", | ||
| tags: ["test-noise"], | ||
| requires: ["nextjs"], | ||
| severity: "error", | ||
| recommendation: | ||
| "Wrap your error UI in `<html><body>…</body></html>` — the root layout is unmounted when global-error renders", | ||
| create: (context: RuleContext) => ({ | ||
| Program(programNode: EsTreeNodeOfType<"Program">) { | ||
| const filename = normalizeFilename(context.filename ?? ""); | ||
| if (!APP_DIRECTORY_PATTERN.test(filename)) return; | ||
| if (!GLOBAL_ERROR_FILE_PATTERN.test(filename)) return; | ||
|
|
||
| const hasHtmlTag = fileContainsJsxElement(programNode, "html"); | ||
| const hasBodyTag = fileContainsJsxElement(programNode, "body"); | ||
|
|
||
| if (!hasHtmlTag || !hasBodyTag) { | ||
| const missingTags = [!hasHtmlTag && "<html>", !hasBodyTag && "<body>"] | ||
| .filter(Boolean) | ||
| .join(" and "); | ||
|
|
||
| context.report({ | ||
| node: programNode, | ||
| message: `global-error.tsx is missing ${missingTags} — the root layout unmounts on error, so this page renders broken HTML.`, | ||
| }); | ||
| } | ||
| }, | ||
| }), | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { | ||
| APP_DIRECTORY_PATTERN, | ||
| ROUTE_HANDLER_FILE_PATTERN, | ||
| ROUTE_HANDLER_HTTP_METHODS, | ||
| } from "../../constants/nextjs.js"; | ||
| import { defineRule } from "../../utils/define-rule.js"; | ||
| import { normalizeFilename } from "../../utils/normalize-filename.js"; | ||
| import type { Rule } from "../../utils/rule.js"; | ||
| import type { RuleContext } from "../../utils/rule-context.js"; | ||
| import { isNodeOfType } from "../../utils/is-node-of-type.js"; | ||
| import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; | ||
|
|
||
| const programHasNamedHttpMethodExport = (programNode: EsTreeNodeOfType<"Program">): boolean => { | ||
| for (const statement of programNode.body ?? []) { | ||
| if (!isNodeOfType(statement, "ExportNamedDeclaration")) continue; | ||
| const declaration = statement.declaration; | ||
| if ( | ||
| isNodeOfType(declaration, "FunctionDeclaration") && | ||
| declaration.id?.name && | ||
| ROUTE_HANDLER_HTTP_METHODS.has(declaration.id.name) | ||
| ) { | ||
| return true; | ||
| } | ||
| if (isNodeOfType(declaration, "VariableDeclaration")) { | ||
| for (const declarator of declaration.declarations ?? []) { | ||
| if ( | ||
| isNodeOfType(declarator.id, "Identifier") && | ||
| ROUTE_HANDLER_HTTP_METHODS.has(declarator.id.name) | ||
| ) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| for (const specifier of statement.specifiers ?? []) { | ||
| if ( | ||
| isNodeOfType(specifier, "ExportSpecifier") && | ||
| isNodeOfType(specifier.exported, "Identifier") && | ||
| ROUTE_HANDLER_HTTP_METHODS.has(specifier.exported.name) | ||
| ) { | ||
| return true; | ||
| } | ||
| } | ||
| } | ||
| return false; | ||
| }; | ||
|
|
||
| export const nextjsNoDefaultExportInRouteHandler = defineRule<Rule>({ | ||
| id: "nextjs-no-default-export-in-route-handler", | ||
| title: "Default export in route handler", | ||
| tags: ["test-noise"], | ||
| requires: ["nextjs"], | ||
| severity: "error", | ||
| recommendation: | ||
| "Replace `export default` with named HTTP method exports: `export async function GET(request) { … }`", | ||
| create: (context: RuleContext) => { | ||
| let isAppRouteHandler = false; | ||
| let programNode: EsTreeNodeOfType<"Program"> | null = null; | ||
|
|
||
| return { | ||
| Program(node: EsTreeNodeOfType<"Program">) { | ||
| const filename = normalizeFilename(context.filename ?? ""); | ||
| isAppRouteHandler = | ||
| APP_DIRECTORY_PATTERN.test(filename) && ROUTE_HANDLER_FILE_PATTERN.test(filename); | ||
| programNode = node; | ||
| }, | ||
| ExportDefaultDeclaration(node: EsTreeNodeOfType<"ExportDefaultDeclaration">) { | ||
| if (!isAppRouteHandler || !programNode) return; | ||
| if (programHasNamedHttpMethodExport(programNode)) return; | ||
|
|
||
| context.report({ | ||
| node, | ||
| message: | ||
| "Default exports in route.ts are silently ignored — Next.js only recognizes named HTTP method exports (GET, POST, etc.).", | ||
| }); | ||
| }, | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| }; | ||
| }, | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { GOOGLE_ANALYTICS_SCRIPT_PATTERN } from "../../constants/nextjs.js"; | ||
| import { defineRule } from "../../utils/define-rule.js"; | ||
| import { findJsxAttribute } from "../../utils/find-jsx-attribute.js"; | ||
| import type { Rule } from "../../utils/rule.js"; | ||
| import type { RuleContext } from "../../utils/rule-context.js"; | ||
| import { isNodeOfType } from "../../utils/is-node-of-type.js"; | ||
| import type { EsTreeNodeOfType } from "../../utils/es-tree-node-of-type.js"; | ||
|
|
||
| export const nextjsNoGoogleAnalyticsScript = defineRule<Rule>({ | ||
| id: "nextjs-no-google-analytics-script", | ||
| title: "Manual Google Analytics script", | ||
| tags: ["test-noise"], | ||
| requires: ["nextjs"], | ||
| severity: "warn", | ||
| recommendation: | ||
| "Use `import { GoogleAnalytics } from '@next/third-parties/google'` for automatic optimization & smaller bundles", | ||
| create: (context: RuleContext) => ({ | ||
| JSXOpeningElement(node: EsTreeNodeOfType<"JSXOpeningElement">) { | ||
| if (!isNodeOfType(node.name, "JSXIdentifier")) return; | ||
| if (node.name.name !== "script" && node.name.name !== "Script") return; | ||
|
|
||
| const srcAttribute = findJsxAttribute(node.attributes ?? [], "src"); | ||
| if (!srcAttribute?.value) return; | ||
|
|
||
| const srcValue = isNodeOfType(srcAttribute.value, "Literal") | ||
| ? srcAttribute.value.value | ||
| : null; | ||
|
|
||
| if (typeof srcValue === "string" && GOOGLE_ANALYTICS_SCRIPT_PATTERN.test(srcValue)) { | ||
| context.report({ | ||
| node, | ||
| message: | ||
| "Manual Google Analytics script blocks rendering — @next/third-parties loads it with optimal strategy.", | ||
| }); | ||
| } | ||
| }, | ||
| }), | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.