feat: make module window check more robust

.changeset/honest-peas-peel.md

@@ -0,0 +1,15 @@
+---
+"eslint-plugin-react-server-components": minor
+---
+
+Making 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:
+```
+const HREF = typeof window !== 'undefined' ? window.location.href : '';
+
+const MyComponent = () => {
+    return <div>{HREF}</div>;
+}
+```
+does not need to be marked with a "use client" because all of it's client only actions are behind a server check.

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 <div />;
-    }
-    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 <div />;
+}`,
         },
       ],
       invalid: [
@@ -99,6 +91,82 @@ function Bar() {
   window.addEventListener('scroll', () => {})
   return <div />;
 }`,
+        },
+        {
+          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 <div />;
+    }
+    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 <div />;
+    }
+    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
         {

src/rules/use-client.ts

@@ -1,5 +1,6 @@
 import type { Rule } from "eslint";
 import type {
+  BinaryExpression,
   Expression,
   ExpressionStatement,
   Identifier,
@@ -26,6 +27,8 @@ const browserOnlyGlobals = Object.keys(globals.browser).reduce<
   return acc;
 }, new Set());
 
+const validGlobalsForServerChecks = new Set(["document", "window"]);
+
 type Options = [
   {
     allowedServerHooks?: string[];
@@ -113,12 +116,97 @@ const create = Components.detect(
       });
     }
 
+    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;
+    }
+
+    function getBinaryBranchExecutedOnServer(node: BinaryExpression): {
+      isWindowCheck: boolean;
+      serverBranch: Rule.Node | null;
+    } {
+      const isWindowCheck =
+        node.left?.type === "UnaryExpression" &&
+        node.left.operator === "typeof" &&
+        node.left.argument?.type === "Identifier" &&
+        validGlobalsForServerChecks.has(node.left.argument?.name) &&
+        node.right?.type === "Literal" &&
+        node.right.value === "undefined" &&
+        (node.operator === "===" || node.operator === "!==");
+
+      let serverBranch = null;
+
+      if (!isWindowCheck) {
+        return { isWindowCheck, serverBranch };
+      }
+
+      //@ts-expect-error
+      const { parent } = node;
+      if (!parent) {
+        return { isWindowCheck, 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 { isWindowCheck, serverBranch };
+    }
+
+    const isNodePartOfSafelyExecutedServerBranch = (
+      node: Rule.Node
+    ): boolean => {
+      let isUsedInServerBranch = false;
+      serverBranches.forEach((serverBranch) => {
+        if (isNodeInTree(node, serverBranch)) {
+          isUsedInServerBranch = true;
+        }
+      });
+      return isUsedInServerBranch;
+    };
+
     const reactImports: Record<string | "namespace", string | string[]> = {
       namespace: [],
     };
 
     const undeclaredReferences = new Set();
 
+    const serverBranches = new Set<Rule.Node>();
+
     return {
       Program(node) {
         for (const block of node.body) {
@@ -195,26 +283,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 <div />;
-        // }
+      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 of window is being used in a `typeof window === undefined`-like check
+          const binaryExpressionNode = findFirstParentOfType(
+            node,
+            "BinaryExpression"
+          ) as BinaryExpression | null;
+          if (binaryExpressionNode) {
+            const { isWindowCheck, serverBranch } =
+              getBinaryBranchExecutedOnServer(binaryExpressionNode);
+            // if this instance isn't part of a window check we report it
+            if (!isWindowCheck) {
+              instances.push(name);
+              reportMissingDirective("addUseClientBrowserAPI", node);
+            } else if (isWindowCheck && serverBranch) {
+              // if it is part of a window check, we don't report it and we save the server branch so we can check if future window instances are a part of the branch of code safely executed on the server
+              serverBranches.add(serverBranch);
+            }
+          } else {
+            // if the window 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) {