diff --git a/.changeset/honest-peas-peel.md b/.changeset/honest-peas-peel.md new file mode 100644 index 0000000..8a9e189 --- /dev/null +++ b/.changeset/honest-peas-peel.md @@ -0,0 +1,15 @@ +--- +"eslint-plugin-react-server-components": major +--- + +Makes checks for window usage more robust to not require "use client" when safely accessed behind a `typeof window !== 'undefined'` or `typeof document !== 'undefined'`check. + +For example: + +```jsx +const href = typeof window !== 'undefined' ? window.location.href : ''; + +const MyComponent = () =>
{href}
; +``` + +This does not need to be marked with a "use client" because all of its client-only actions are behind a safety check. diff --git a/src/rules/use-client.test.ts b/src/rules/use-client.test.ts index 603fe4a..53a9d8a 100644 --- a/src/rules/use-client.test.ts +++ b/src/rules/use-client.test.ts @@ -24,29 +24,21 @@ describe("use client", () => { code: 'const foo = "bar"', }, { - code: `import {createContext, useContext, useEffect} from 'react'; - const context = createContext() - export function useTheme() { - const context = useContext(context); - useEffect(() => { - window.setTimeout(() => {}); - }); - return context; - }`, + code: `const HREF = typeof window === 'undefined' ? undefined : window.location.href;`, }, { - code: `import * as React from 'react'; - const context = React.createContext() - export function Foo() { - return
; - } - export function useTheme() { - const context = React.useContext(context); - React.useEffect(() => { - window.setTimeout(() => {}); - }); - return context; - }`, + code: `const HREF = typeof window !== 'undefined' ? window.location.href : '';`, + }, + { + code: `const el = typeof document === 'undefined' ? undefined : document.createElement('element');`, + }, + { + code: `const foo = "bar"; +function Bar() { + if(typeof document !== 'undefined') + document.addEventListener('scroll', () => {}) + return
; +}`, }, ], invalid: [ @@ -99,6 +91,82 @@ function Bar() { window.addEventListener('scroll', () => {}) return
; }`, + }, + { + code: `import {createContext, useContext, useEffect} from 'react'; + + const context = createContext() + export function useTheme() { + const context = useContext(context); + useEffect(() => { + window.setTimeout(() => {}); + }); + return context; + }`, + errors: [{ messageId: "addUseClientBrowserAPI" }], + output: `'use client'; + +import {createContext, useContext, useEffect} from 'react'; + + const context = createContext() + export function useTheme() { + const context = useContext(context); + useEffect(() => { + window.setTimeout(() => {}); + }); + return context; + }`, + }, + { + code: `import * as React from 'react'; + + const context = React.createContext() + export function Foo() { + return
; + } + export function useTheme() { + const context = React.useContext(context); + React.useEffect(() => { + window.setTimeout(() => {}); + }); + return context; + }`, + errors: [{ messageId: "addUseClientBrowserAPI" }], + output: `'use client'; + +import * as React from 'react'; + + const context = React.createContext() + export function Foo() { + return
; + } + export function useTheme() { + const context = React.useContext(context); + React.useEffect(() => { + window.setTimeout(() => {}); + }); + return context; + }`, + }, + { + code: `const HREF = typeof window === 'undefined' ? window.location.href : window.location.href.slice(0,10);`, + errors: [{ messageId: "addUseClientBrowserAPI" }], + output: `'use client'; + +const HREF = typeof window === 'undefined' ? window.location.href : window.location.href.slice(0,10);`, + }, + { + code: `let HREF = ''; + if (typeof window === 'undefined') { + HREF = window.location.href; + }`, + errors: [{ messageId: "addUseClientBrowserAPI" }], + output: `'use client'; + +let HREF = ''; + if (typeof window === 'undefined') { + HREF = window.location.href; + }`, }, // OBSERVERS { diff --git a/src/rules/use-client.ts b/src/rules/use-client.ts index 4e39062..9aba989 100644 --- a/src/rules/use-client.ts +++ b/src/rules/use-client.ts @@ -1,5 +1,6 @@ import type { Rule } from "eslint"; import type { + BinaryExpression, Expression, ExpressionStatement, Identifier, @@ -113,12 +114,69 @@ const create = Components.detect( }); } + + function getBinaryBranchExecutedOnServer(node: BinaryExpression): { + isGlobalClientPropertyCheck: boolean; + serverBranch: Rule.Node | null; + } { + const isGlobalClientPropertyCheck = + node.left?.type === "UnaryExpression" && + node.left.operator === "typeof" && + node.left.argument?.type === "Identifier" && + browserOnlyGlobals.has(node.left.argument?.name as any) && + node.right?.type === "Literal" && + node.right.value === "undefined" && + (node.operator === "===" || node.operator === "!=="); + + let serverBranch = null; + + if (!isGlobalClientPropertyCheck) { + return { isGlobalClientPropertyCheck, serverBranch }; + } + + //@ts-expect-error + const { parent } = node; + if (!parent) { + return { isGlobalClientPropertyCheck, serverBranch }; + } + + if (node.operator === "===") { + serverBranch = + parent.type === "IfStatement" || + parent.type === "ConditionalExpression" + ? parent.alternate + : null; + } else { + serverBranch = + parent.type === "IfStatement" || + parent.type === "ConditionalExpression" + ? parent.consequent + : null; + } + + return { isGlobalClientPropertyCheck, serverBranch }; + } + + const isNodePartOfSafelyExecutedServerBranch = ( + node: Rule.Node + ): boolean => { + let isUsedInServerBranch = false; + serverBranches.forEach((serverBranch) => { + if (isNodeInTree(node, serverBranch)) { + isUsedInServerBranch = true; + } + }); + return isUsedInServerBranch; + }; + const reactImports: Record = { namespace: [], }; const undeclaredReferences = new Set(); + const serverBranches = new Set(); + return { Program(node) { for (const block of node.body) { @@ -195,26 +253,33 @@ const create = Components.detect( }); } }, - MemberExpression(node) { - // Catch uses of browser APIs in module scope - // or React component scope. - // eg: - // const foo = window.foo - // window.addEventListener(() => {}) - // const Foo() { - // const foo = window.foo - // return
; - // } + Identifier(node) { + const name = node.name; // @ts-expect-error - const name = node.object.name; - const scopeType = context.getScope().type; - if ( - undeclaredReferences.has(name) && - browserOnlyGlobals.has(name) && - (scopeType === "module" || !!util.getParentComponent(node)) - ) { - instances.push(name); - reportMissingDirective("addUseClientBrowserAPI", node.object); + if (undeclaredReferences.has(name) && browserOnlyGlobals.has(name)) { + // find the nearest binary expression so we can see if this instance is being used in a `typeof window === undefined`-like check + const binaryExpressionNode = findFirstParentOfType( + node, + "BinaryExpression" + ) as BinaryExpression | null; + if (binaryExpressionNode) { + const { isGlobalClientPropertyCheck, serverBranch } = + getBinaryBranchExecutedOnServer(binaryExpressionNode); + // if this instance isn't part of a server check we report it + if (!isGlobalClientPropertyCheck) { + instances.push(name); + reportMissingDirective("addUseClientBrowserAPI", node); + } else if (isGlobalClientPropertyCheck && serverBranch) { + // if it is part of a check, we don't report it and we save the server branch so we can check if future instances are a part of the branch of code safely executed on the server + serverBranches.add(serverBranch); + } + } else { + // if the usage isn't part of the binary expression, we check to see if it's part of a safely checked server branch and report if not + if (!isNodePartOfSafelyExecutedServerBranch(node)) { + instances.push(name); + reportMissingDirective("addUseClientBrowserAPI", node); + } + } } }, ExpressionStatement(node) { @@ -316,4 +381,33 @@ function isFunction(def: any) { return false; } +function findFirstParentOfType( + node: Rule.Node, + type: string +): Rule.Node | null { + let currentNode: Rule.Node | null = node; + + while (currentNode) { + if (currentNode.type === type) { + return currentNode; + } + currentNode = currentNode?.parent; + } + + return null; +} + +function isNodeInTree(node: Rule.Node, target: Rule.Node): boolean { + let currentNode: Rule.Node | null = node; + + while (currentNode) { + if (currentNode === target) { + return true; + } + currentNode = currentNode.parent; + } + + return false; +} + export const ClientComponents = { meta, create };