Skip to content

Commit 6bf7537

Browse files
authored
prefer-global-this: Fix for window-specific in checks (#2879)
1 parent a6ac894 commit 6bf7537

4 files changed

Lines changed: 86 additions & 13 deletions

File tree

rules/prefer-global-this.js

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import {isStringLiteral} from './ast/index.js';
2+
13
const MESSAGE_ID_ERROR = 'prefer-global-this/error';
24
const messages = {
35
[MESSAGE_ID_ERROR]: 'Prefer `{{replacement}}` over `{{value}}`.',
@@ -37,7 +39,7 @@ Some constructors are occasionally related to window (like Element !== iframe.co
3739
3840
Please use these criteria to decide whether an API should be added here. Context: https://github.com/sindresorhus/eslint-plugin-unicorn/pull/2410#discussion_r1695312427
3941
*/
40-
const windowSpecificAPIs = new Set([
42+
const windowSpecificApis = new Set([
4143
// Properties and methods
4244
// https://html.spec.whatwg.org/multipage/nav-history-apis.html#the-window-object
4345
'name',
@@ -103,7 +105,7 @@ const windowSpecificAPIs = new Set([
103105
'devicePixelRatio',
104106
]);
105107

106-
const webWorkerSpecificAPIs = new Set([
108+
const webWorkerSpecificApis = new Set([
107109
// https://html.spec.whatwg.org/multipage/workers.html#the-workerglobalscope-common-interface
108110
'addEventListener',
109111
'removeEventListener',
@@ -125,13 +127,18 @@ const webWorkerSpecificAPIs = new Set([
125127
'onconnect',
126128
]);
127129

130+
const environmentSpecificApisByGlobalIdentifier = new Map([
131+
['window', windowSpecificApis],
132+
['self', webWorkerSpecificApis],
133+
]);
134+
128135
/**
129136
Check if the node is a window-specific API.
130137
131138
@param {import('estree').MemberExpression} node
132139
@returns {boolean}
133140
*/
134-
const isWindowSpecificAPI = node => {
141+
const isWindowSpecificApi = node => {
135142
if (node.type !== 'MemberExpression') {
136143
return false;
137144
}
@@ -140,7 +147,7 @@ const isWindowSpecificAPI = node => {
140147
return false;
141148
}
142149

143-
if (windowSpecificAPIs.has(node.property.name)) {
150+
if (windowSpecificApis.has(node.property.name)) {
144151
if (['addEventListener', 'removeEventListener', 'dispatchEvent'].includes(node.property.name) && node.parent.type === 'CallExpression' && node.parent.callee === node) {
145152
const argument = node.parent.arguments[0];
146153
return argument && argument.type === 'Literal' && windowSpecificEvents.has(argument.value);
@@ -160,13 +167,37 @@ function isComputedMemberExpressionObject(identifier) {
160167
return identifier.parent.type === 'MemberExpression' && identifier.parent.computed && identifier.parent.object === identifier;
161168
}
162169

170+
/**
171+
Check if the identifier is used in an existence check for a known environment-specific API.
172+
173+
@param {import('estree').Identifier} identifier
174+
@returns {boolean}
175+
*/
176+
function isKnownSpecificApiExistenceCheck(identifier) {
177+
const specificApis = environmentSpecificApisByGlobalIdentifier.get(identifier.name);
178+
if (!specificApis) {
179+
return false;
180+
}
181+
182+
const {parent} = identifier;
183+
if (parent.type !== 'BinaryExpression' || parent.operator !== 'in' || parent.right !== identifier) {
184+
return false;
185+
}
186+
187+
if (!isStringLiteral(parent.left)) {
188+
return false;
189+
}
190+
191+
return specificApis.has(parent.left.value);
192+
}
193+
163194
/**
164195
Check if the node is a web worker specific API.
165196
166197
@param {import('estree').MemberExpression} node
167198
@returns {boolean}
168199
*/
169-
const isWebWorkerSpecificAPI = node => node.type === 'MemberExpression' && node.object.name === 'self' && node.property.type === 'Identifier' && webWorkerSpecificAPIs.has(node.property.name);
200+
const isWebWorkerSpecificApi = node => node.type === 'MemberExpression' && node.object.name === 'self' && node.property.type === 'Identifier' && webWorkerSpecificApis.has(node.property.name);
170201

171202
/** @param {import('eslint').Rule.RuleContext} context */
172203
const create = context => {
@@ -183,8 +214,9 @@ const create = context => {
183214
for (const {identifier} of references) {
184215
if (
185216
isComputedMemberExpressionObject(identifier)
186-
|| isWindowSpecificAPI(identifier.parent)
187-
|| isWebWorkerSpecificAPI(identifier.parent)
217+
|| isKnownSpecificApiExistenceCheck(identifier)
218+
|| isWindowSpecificApi(identifier.parent)
219+
|| isWebWorkerSpecificApi(identifier.parent)
188220
) {
189221
continue;
190222
}

test/prefer-global-this.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ test.snapshot({
5050
'self.navigator',
5151
'window.addEventListener("resize", () => {})',
5252
'window.onresize = function () {}',
53+
'\'open\' in window; window.open("https://example.com")',
5354
outdent`
5455
const {window} = jsdom()
5556
window.jQuery = jQuery;
@@ -168,6 +169,8 @@ test.snapshot({
168169
'[window.foo] = []',
169170
'foo[window]',
170171
'foo[window.foo]',
172+
'\'foo\' in window',
173+
'\'foo\' in global',
171174
'typeof window !== "undefined"',
172175
'typeof self !== "undefined"',
173176
'typeof global !== "undefined"',

test/snapshots/prefer-global-this.js.md

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,45 @@ Generated by [AVA](https://avajs.dev).
10941094
1 | foo[globalThis.foo]␊
10951095
`
10961096

1097-
## invalid(51): typeof window !== "undefined"
1097+
## invalid(51): 'foo' in window
1098+
1099+
> Input
1100+
1101+
`␊
1102+
1 | 'foo' in window␊
1103+
`
1104+
1105+
> Error 1/1
1106+
1107+
`␊
1108+
Message:␊
1109+
> 1 | 'foo' in window␊
1110+
| ^^^^^^ Prefer \`globalThis\` over \`window\`.␊
1111+
1112+
Output:␊
1113+
1 | 'foo' in globalThis␊
1114+
`
1115+
1116+
## invalid(52): 'foo' in global
1117+
1118+
> Input
1119+
1120+
`␊
1121+
1 | 'foo' in global␊
1122+
`
1123+
1124+
> Error 1/1
1125+
1126+
`␊
1127+
Message:␊
1128+
> 1 | 'foo' in global␊
1129+
| ^^^^^^ Prefer \`globalThis\` over \`global\`.␊
1130+
1131+
Output:␊
1132+
1 | 'foo' in globalThis␊
1133+
`
1134+
1135+
## invalid(53): typeof window !== "undefined"
10981136

10991137
> Input
11001138
@@ -1113,7 +1151,7 @@ Generated by [AVA](https://avajs.dev).
11131151
1 | typeof globalThis.window !== "undefined"␊
11141152
`
11151153

1116-
## invalid(52): typeof self !== "undefined"
1154+
## invalid(54): typeof self !== "undefined"
11171155

11181156
> Input
11191157
@@ -1132,7 +1170,7 @@ Generated by [AVA](https://avajs.dev).
11321170
1 | typeof globalThis.self !== "undefined"␊
11331171
`
11341172

1135-
## invalid(53): typeof global !== "undefined"
1173+
## invalid(55): typeof global !== "undefined"
11361174

11371175
> Input
11381176
@@ -1151,7 +1189,7 @@ Generated by [AVA](https://avajs.dev).
11511189
1 | typeof globalThis.global !== "undefined"␊
11521190
`
11531191

1154-
## invalid(54): typeof window.something === "function"
1192+
## invalid(56): typeof window.something === "function"
11551193

11561194
> Input
11571195
@@ -1170,7 +1208,7 @@ Generated by [AVA](https://avajs.dev).
11701208
1 | typeof globalThis.something === "function"␊
11711209
`
11721210

1173-
## invalid(55): typeof self.something === "function"
1211+
## invalid(57): typeof self.something === "function"
11741212

11751213
> Input
11761214
@@ -1189,7 +1227,7 @@ Generated by [AVA](https://avajs.dev).
11891227
1 | typeof globalThis.something === "function"␊
11901228
`
11911229

1192-
## invalid(56): typeof global.something === "function"
1230+
## invalid(58): typeof global.something === "function"
11931231

11941232
> Input
11951233
60 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)