Skip to content

Commit fe5dec3

Browse files
cmackenthuncmackenthun
committed
rework
1 parent 9846500 commit fe5dec3

File tree

2 files changed

+189
-75
lines changed

2 files changed

+189
-75
lines changed

‎src/rules/use-client.test.ts‎

Lines changed: 78 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,37 +24,22 @@ describe("use client", () => {
2424
code: 'const foo = "bar"',
2525
},
2626
{
27-
code: `import {createContext, useContext, useEffect} from 'react';
28-
const context = createContext()
29-
export function useTheme() {
30-
const context = useContext(context);
31-
useEffect(() => {
32-
window.setTimeout(() => {});
33-
});
34-
return context;
35-
}`,
27+
code: `const HREF = typeof window === 'undefined' ? undefined : window.location.href;`,
3628
},
3729
{
38-
code: `import * as React from 'react';
39-
const context = React.createContext()
40-
export function Foo() {
41-
return <div />;
42-
}
43-
export function useTheme() {
44-
const context = React.useContext(context);
45-
React.useEffect(() => {
46-
window.setTimeout(() => {});
47-
});
48-
return context;
49-
}`,
30+
code: `const HREF = typeof window !== 'undefined' ? window.location.href : '';`,
5031
},
5132
{
52-
code: `const HREF = typeof window === 'undefined' ? undefined : window.location.href;`
33+
code: `const el = typeof document === 'undefined' ? undefined : document.createElement('element');`,
5334
},
5435
{
55-
code: `const HREF = typeof window !== 'undefined' ? window.location.href : '';`
36+
code: `const foo = "bar";
37+
function Bar() {
38+
if(typeof document !== 'undefined')
39+
document.addEventListener('scroll', () => {})
40+
return <div />;
41+
}`,
5642
},
57-
5843
],
5944
invalid: [
6045
// DOCUMENT
@@ -106,13 +91,82 @@ function Bar() {
10691
window.addEventListener('scroll', () => {})
10792
return <div />;
10893
}`,
94+
},
95+
{
96+
code: `import {createContext, useContext, useEffect} from 'react';
97+
98+
const context = createContext()
99+
export function useTheme() {
100+
const context = useContext(context);
101+
useEffect(() => {
102+
window.setTimeout(() => {});
103+
});
104+
return context;
105+
}`,
106+
errors: [{ messageId: "addUseClientBrowserAPI" }],
107+
output: `'use client';
108+
109+
import {createContext, useContext, useEffect} from 'react';
110+
111+
const context = createContext()
112+
export function useTheme() {
113+
const context = useContext(context);
114+
useEffect(() => {
115+
window.setTimeout(() => {});
116+
});
117+
return context;
118+
}`,
119+
},
120+
{
121+
code: `import * as React from 'react';
122+
123+
const context = React.createContext()
124+
export function Foo() {
125+
return <div />;
126+
}
127+
export function useTheme() {
128+
const context = React.useContext(context);
129+
React.useEffect(() => {
130+
window.setTimeout(() => {});
131+
});
132+
return context;
133+
}`,
134+
errors: [{ messageId: "addUseClientBrowserAPI" }],
135+
output: `'use client';
136+
137+
import * as React from 'react';
138+
139+
const context = React.createContext()
140+
export function Foo() {
141+
return <div />;
142+
}
143+
export function useTheme() {
144+
const context = React.useContext(context);
145+
React.useEffect(() => {
146+
window.setTimeout(() => {});
147+
});
148+
return context;
149+
}`,
109150
},
110151
{
111152
code: `const HREF = typeof window === 'undefined' ? window.location.href : window.location.href.slice(0,10);`,
112153
errors: [{ messageId: "addUseClientBrowserAPI" }],
113154
output: `'use client';
114155
115156
const HREF = typeof window === 'undefined' ? window.location.href : window.location.href.slice(0,10);`,
157+
},
158+
{
159+
code: `let HREF = '';
160+
if (typeof window === 'undefined') {
161+
HREF = window.location.href;
162+
}`,
163+
errors: [{ messageId: "addUseClientBrowserAPI" }],
164+
output: `'use client';
165+
166+
let HREF = '';
167+
if (typeof window === 'undefined') {
168+
HREF = window.location.href;
169+
}`,
116170
},
117171
// OBSERVERS
118172
{

‎src/rules/use-client.ts‎

Lines changed: 111 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import type { Rule } from "eslint";
22
import type {
3+
BinaryExpression,
34
Expression,
45
ExpressionStatement,
56
Identifier,
67
ImportSpecifier,
7-
MemberExpression,
88
Node,
99
Program,
1010
SpreadElement,
@@ -27,6 +27,8 @@ const browserOnlyGlobals = Object.keys(globals.browser).reduce<
2727
return acc;
2828
}, new Set());
2929

30+
const validGlobalsForServerChecks = new Set(["document", "window"]);
31+
3032
type Options = [
3133
{
3234
allowedServerHooks?: string[];
@@ -114,42 +116,97 @@ const create = Components.detect(
114116
});
115117
}
116118

117-
function getIsSafeWindowCheck(node: Rule.NodeParentExtension) {
118-
// check if the window usage is behind a typeof window === 'undefined' check
119-
const conditionalExpressionNode = node.parent?.parent;
119+
function findFirstParentOfType(
120+
node: Rule.Node,
121+
type: string
122+
): Rule.Node | null {
123+
let currentNode: Rule.Node | null = node;
124+
125+
while (currentNode) {
126+
if (currentNode.type === type) {
127+
return currentNode;
128+
}
129+
currentNode = currentNode?.parent;
130+
}
131+
132+
return null;
133+
}
134+
135+
function isNodeInTree(node: Rule.Node, target: Rule.Node): boolean {
136+
let currentNode: Rule.Node | null = node;
137+
138+
while (currentNode) {
139+
if (currentNode === target) {
140+
return true;
141+
}
142+
currentNode = currentNode.parent;
143+
}
144+
145+
return false;
146+
}
147+
148+
function getBinaryBranchExecutedOnServer(node: BinaryExpression): {
149+
isWindowCheck: boolean;
150+
serverBranch: Rule.Node | null;
151+
} {
120152
const isWindowCheck =
121-
conditionalExpressionNode?.type === "ConditionalExpression" &&
122-
conditionalExpressionNode.test?.type === "BinaryExpression" &&
123-
conditionalExpressionNode.test.left?.type === "UnaryExpression" &&
124-
conditionalExpressionNode.test.left.operator === "typeof" &&
125-
conditionalExpressionNode.test.left.argument?.type === "Identifier" &&
126-
conditionalExpressionNode.test.left.argument?.name === "window" &&
127-
conditionalExpressionNode.test.right?.type === "Literal" &&
128-
conditionalExpressionNode.test.right.value === "undefined";
129-
130-
// checks to see if it's `typeof window !== 'undefined'` or `typeof window === 'undefined'`
131-
const isNegatedWindowCheck =
132-
isWindowCheck &&
133-
conditionalExpressionNode.test?.type === "BinaryExpression" &&
134-
conditionalExpressionNode.test.operator === "!==";
135-
136-
// checks to see if window is being accessed safely behind a window check
137-
const isSafelyBehindWindowCheck =
138-
(isWindowCheck &&
139-
!isNegatedWindowCheck &&
140-
conditionalExpressionNode.alternate === node?.parent) ||
141-
(isNegatedWindowCheck &&
142-
conditionalExpressionNode.consequent === node?.parent);
143-
144-
return isSafelyBehindWindowCheck;
153+
node.left?.type === "UnaryExpression" &&
154+
node.left.operator === "typeof" &&
155+
node.left.argument?.type === "Identifier" &&
156+
validGlobalsForServerChecks.has(node.left.argument?.name) &&
157+
node.right?.type === "Literal" &&
158+
node.right.value === "undefined" &&
159+
(node.operator === "===" || node.operator === "!==");
160+
161+
let serverBranch = null;
162+
163+
if (!isWindowCheck) {
164+
return { isWindowCheck, serverBranch };
165+
}
166+
167+
//@ts-expect-error
168+
const { parent } = node;
169+
if (!parent) {
170+
return { isWindowCheck, serverBranch };
171+
}
172+
173+
if (node.operator === "===") {
174+
serverBranch =
175+
parent.type === "IfStatement" ||
176+
parent.type === "ConditionalExpression"
177+
? parent.alternate
178+
: null;
179+
} else {
180+
serverBranch =
181+
parent.type === "IfStatement" ||
182+
parent.type === "ConditionalExpression"
183+
? parent.consequent
184+
: null;
185+
}
186+
187+
return { isWindowCheck, serverBranch };
145188
}
146189

190+
const isNodePartOfSafelyExecutedServerBranch = (
191+
node: Rule.Node
192+
): boolean => {
193+
let isUsedInServerBranch = false;
194+
serverBranches.forEach((serverBranch) => {
195+
if (isNodeInTree(node, serverBranch)) {
196+
isUsedInServerBranch = true;
197+
}
198+
});
199+
return isUsedInServerBranch;
200+
};
201+
147202
const reactImports: Record<string | "namespace", string | string[]> = {
148203
namespace: [],
149204
};
150205

151206
const undeclaredReferences = new Set();
152207

208+
const serverBranches = new Set<Rule.Node>();
209+
153210
return {
154211
Program(node) {
155212
for (const block of node.body) {
@@ -226,30 +283,33 @@ const create = Components.detect(
226283
});
227284
}
228285
},
229-
MemberExpression(node) {
230-
// Catch uses of browser APIs in module scope
231-
// or React component scope.
232-
// eg:
233-
// const foo = window.foo
234-
// window.addEventListener(() => {})
235-
// const Foo() {
236-
// const foo = window.foo
237-
// return <div />;
238-
// }
286+
Identifier(node) {
287+
const name = node.name;
239288
// @ts-expect-error
240-
const name = node.object.name;
241-
const scopeType = context.getScope().type;
242-
243-
const isSafelyBehindWindowCheck = getIsSafeWindowCheck(node);
244-
245-
if (
246-
undeclaredReferences.has(name) &&
247-
browserOnlyGlobals.has(name) &&
248-
(scopeType === "module" || !!util.getParentComponent(node)) &&
249-
!isSafelyBehindWindowCheck
250-
) {
251-
instances.push(name);
252-
reportMissingDirective("addUseClientBrowserAPI", node.object);
289+
if (undeclaredReferences.has(name) && browserOnlyGlobals.has(name)) {
290+
// find the nearest binary expression so we can see if this instance of window is being used in a `typeof window === undefined`-like check
291+
const binaryExpressionNode = findFirstParentOfType(
292+
node,
293+
"BinaryExpression"
294+
) as BinaryExpression | null;
295+
if (binaryExpressionNode) {
296+
const { isWindowCheck, serverBranch } =
297+
getBinaryBranchExecutedOnServer(binaryExpressionNode);
298+
// if this instance isn't part of a window check we report it
299+
if (!isWindowCheck) {
300+
instances.push(name);
301+
reportMissingDirective("addUseClientBrowserAPI", node);
302+
} else if (isWindowCheck && serverBranch) {
303+
// 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
304+
serverBranches.add(serverBranch);
305+
}
306+
} else {
307+
// 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
308+
if (!isNodePartOfSafelyExecutedServerBranch(node)) {
309+
instances.push(name);
310+
reportMissingDirective("addUseClientBrowserAPI", node);
311+
}
312+
}
253313
}
254314
},
255315
ExpressionStatement(node) {

0 commit comments

Comments
 (0)