Skip to content

Commit caf8cb5

Browse files
authored
[Linter] Add a linter rule to prevent usage of window (Azure#13873)
## What - Adds a rule to prevent usage of `window` and prefer `self` instead ## Why - This came up recently in a few cases: Azure#11067 and in this discussion: Azure#13232 (comment) - window is not always the global object, but self's reference changes depending on the context and will point to the right global object - Adding a linter rule makes this easy to remember instead of relying on diligence Resolves Azure#13472
1 parent 014636f commit caf8cb5

File tree

16 files changed

+126
-17
lines changed

16 files changed

+126
-17
lines changed

common/tools/eslint-plugin-azure-sdk/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ Some rules (see table below) are fixable using the `--fix` ESLint option (added
101101
| [**ts-naming-options**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md) | :triangular_flag_on_post: | :x: | `1.2.0` |
102102
| [**ts-naming-subclients**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-subclients.md) | :triangular_flag_on_post: | :x: | `1.2.0` |
103103
| [**ts-no-const-enums**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-no-const-enums.md) | :warning: | :heavy_check_mark: | `1.1.0` |
104+
| [**ts-no-window**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-no-window.md) | :triangular_flag_on_post: | :heavy_check_mark: | `3.1.0` |
104105
| [**ts-no-namespaces**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-no-namespaces.md) | :triangular_flag_on_post: | :x: | `1.2.0` |
105106
| [**ts-package-json-author**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-author.md) | :triangular_flag_on_post: | :heavy_check_mark: | `1.0.0` |
106107
| [**ts-package-json-bugs**](https://github.com/Azure/azure-sdk-for-js/blob/master/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-bugs.md) | :triangular_flag_on_post: | :heavy_check_mark: | `1.0.0` |
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# ts-no-window
2+
3+
Recommends against the usage of `window` in favor of `self`.
4+
5+
This rule is fixable using the `--fix` option.
6+
7+
## Examples
8+
9+
### Good
10+
11+
```ts
12+
const navigator = self.navigator as NavigatorEx;
13+
```
14+
15+
### Bad
16+
17+
```ts
18+
const navigator = window.navigator as NavigatorEx;
19+
```
20+
21+
## When to turn off
22+
23+
Only if the rule breaks.

common/tools/eslint-plugin-azure-sdk/src/configs/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export = {
4747
"@azure/azure-sdk/ts-naming-subclients": "error",
4848
"@azure/azure-sdk/ts-no-const-enums": "warn",
4949
"@azure/azure-sdk/ts-no-namespaces": "error",
50+
"@azure/azure-sdk/ts-no-window": "error",
5051
"@azure/azure-sdk/ts-package-json-author": "error",
5152
"@azure/azure-sdk/ts-package-json-bugs": "error",
5253
"@azure/azure-sdk/ts-package-json-engine-is-present": "error",

common/tools/eslint-plugin-azure-sdk/src/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import tsNamingOptions from "./ts-naming-options";
3131
import tsNamingSubclients from "./ts-naming-subclients";
3232
import tsNoConstEnums from "./ts-no-const-enums";
3333
import tsNoNamespaces from "./ts-no-namespaces";
34+
import tsNoWindow from "./ts-no-window";
3435
import tsPackageJsonAuthor from "./ts-package-json-author";
3536
import tsPackageJsonBugs from "./ts-package-json-bugs";
3637
import tsPackageJsonEngineIsPresent from "./ts-package-json-engine-is-present";
@@ -79,6 +80,7 @@ export = {
7980
"ts-naming-subclients": tsNamingSubclients,
8081
"ts-no-const-enums": tsNoConstEnums,
8182
"ts-no-namespaces": tsNoNamespaces,
83+
"ts-no-window": tsNoWindow,
8284
"ts-package-json-author": tsPackageJsonAuthor,
8385
"ts-package-json-bugs": tsPackageJsonBugs,
8486
"ts-package-json-engine-is-present": tsPackageJsonEngineIsPresent,
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
/**
5+
* @file Rule to forbid usage of window.
6+
* @author Maor Leger
7+
*/
8+
9+
import { Rule } from "eslint";
10+
import { getRuleMetaData } from "../utils";
11+
12+
//------------------------------------------------------------------------------
13+
// Rule Definition
14+
//------------------------------------------------------------------------------
15+
16+
export = {
17+
meta: getRuleMetaData("ts-no-window", "forbid usage of window", "code"),
18+
create: (context: Rule.RuleContext): Rule.RuleListener =>
19+
({
20+
// report on any window identifiers
21+
"Identifier[name=window]": (node: any): void => {
22+
context.report({
23+
node: node,
24+
message: "`window` should not be used, please use `self` instead.",
25+
fix: (fixer: Rule.RuleFixer): Rule.Fix =>
26+
fixer.replaceTextRange([node.range[0], node.range[0] + "window".length], "self")
27+
});
28+
}
29+
} as Rule.RuleListener)
30+
};

common/tools/eslint-plugin-azure-sdk/tests/plugin.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const ruleList = [
3939
"ts-naming-subclients",
4040
"ts-no-const-enums",
4141
"ts-no-namespaces",
42+
"ts-no-window",
4243
"ts-package-json-author",
4344
"ts-package-json-bugs",
4445
"ts-package-json-engine-is-present",
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
/**
5+
* @file Testing the ts-no-window rule.
6+
* @author Maor Leger
7+
*/
8+
9+
import rule from "../../src/rules/ts-no-window";
10+
import { RuleTester } from "eslint";
11+
12+
//------------------------------------------------------------------------------
13+
// Tests
14+
//------------------------------------------------------------------------------
15+
16+
const ruleTester = new RuleTester({
17+
parser: require.resolve("@typescript-eslint/parser"),
18+
parserOptions: {
19+
createDefaultProgram: true,
20+
project: "./tsconfig.json",
21+
sourceType: "module"
22+
},
23+
settings: {
24+
main: "test.ts"
25+
}
26+
});
27+
28+
ruleTester.run("ts-no-window", rule, {
29+
valid: [
30+
{
31+
code: "self.navigator",
32+
filename: "src/test.ts"
33+
},
34+
{
35+
code: '"quoted window should be fine"',
36+
filename: "src/test.ts"
37+
}
38+
],
39+
invalid: [
40+
{
41+
code: "window.navigator",
42+
filename: "src/test.ts",
43+
errors: [
44+
{
45+
message: "`window` should not be used, please use `self` instead."
46+
}
47+
],
48+
output: "self.navigator"
49+
}
50+
]
51+
});

sdk/core/core-amqp/src/ConnectionContextBase.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,9 @@ export const ConnectionContextBase = {
142142

143143
if (
144144
parameters.config.webSocket ||
145-
(!isNode && typeof window !== "undefined" && (window as any).WebSocket)
145+
(!isNode && typeof self !== "undefined" && (self as any).WebSocket)
146146
) {
147-
const socket = parameters.config.webSocket || (window as any).WebSocket;
147+
const socket = parameters.config.webSocket || (self as any).WebSocket;
148148
const host = parameters.config.host;
149149
const endpoint = parameters.config.webSocketEndpointPath || "";
150150
const socketOptions = parameters.config.webSocketConstructorOptions || {};

sdk/core/core-amqp/src/errors.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -610,9 +610,9 @@ function isBrowserWebsocketError(err: any): boolean {
610610
let result: boolean = false;
611611
if (
612612
!isNode &&
613-
window &&
613+
self &&
614614
err.type === "error" &&
615-
err.target instanceof (window as any).WebSocket
615+
err.target instanceof (self as any).WebSocket
616616
) {
617617
result = true;
618618
}

sdk/core/core-amqp/src/shims.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,4 @@ interface Window {
1313
}
1414
declare let navigator: Navigator;
1515

16-
declare let window: Window;
16+
declare let self: Window & typeof globalThis

0 commit comments

Comments
 (0)