Skip to content

Commit fabc542

Browse files
authored
No unguarded window (#17)
* Adding rule for no-unguarded-window. Also upgraded no-unguarded-document and added tests for both to catch common use cases. Added yarn test-server to start tests in watch mode.
1 parent 6ce4766 commit fabc542

File tree

13 files changed

+648
-127
lines changed

13 files changed

+648
-127
lines changed
File renamed without changes.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# No Unguarded Window References (LinkedIn specific)
2+
3+
**TL;DR: Ensure that all window references are guarded inside an if block with the condition `environment.isBrowser()`**
4+
5+
Unguarded window references will break when used with FastBoot. This is due to the Node environment
6+
not having a global `window` object.
7+
8+
To protect against errors during server side rendering, you should ensure you're guarding the usages
9+
of `window` with a check to `environment.isBrowser()`.
10+
11+
Example:
12+
13+
```js
14+
import Ember from 'ember';
15+
import environment from 'ember-stdlib/utils/environment';
16+
17+
Ember.Component.extend({
18+
init() {
19+
if (environment.isBrowser()) {
20+
this.location = window.location;
21+
}
22+
}
23+
});
24+
```
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
const { get } = require('../../utils/get');
2+
const { isBrowser, isEnvironmentBrowser, getEnvironmentImportBinding } = require('../../utils/linkedin/bpr');
3+
const { isInActions, isInteractive } = require('../../utils/interactive');
4+
const { collectObjectPatternBindings } = require('../../utils/destructed-binding');
5+
6+
const MESSAGE = 'Do not use unguarded document references. Please see the following guide for more infromation: http://github.com/chadhietala/ember-best-practices/files/guides/no-unguarded-document.md';
7+
8+
function isDocumentReference(node) {
9+
return get(node, 'callee.object.name') === 'document';
10+
}
11+
12+
module.exports = {
13+
meta: {
14+
message: MESSAGE
15+
},
16+
create(context) {
17+
let environmentImportBinding;
18+
let destructuredBindings = [];
19+
20+
return {
21+
ObjectPattern(node) {
22+
if (environmentImportBinding) {
23+
destructuredBindings = destructuredBindings.concat(collectObjectPatternBindings(node, {
24+
[environmentImportBinding]: ['isBrowser']
25+
}));
26+
}
27+
},
28+
29+
ImportDefaultSpecifier(node) {
30+
environmentImportBinding = getEnvironmentImportBinding(node);
31+
},
32+
33+
CallExpression(node) {
34+
if (isDocumentReference(node)) {
35+
if (!isInActions(node) && !isInteractive(node) && !isBrowser(node) && !isEnvironmentBrowser(node)) {
36+
context.report(node, MESSAGE);
37+
}
38+
}
39+
}
40+
};
41+
}
42+
};
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
const { get } = require('../../utils/get');
2+
const { isBrowser, isEnvironmentBrowser, getEnvironmentImportBinding } = require('../../utils/linkedin/bpr');
3+
const { isInActions, isInteractive } = require('../../utils/interactive');
4+
const { collectObjectPatternBindings } = require('../../utils/destructed-binding');
5+
6+
const MESSAGE = 'Do not use unguarded window references. Please see the following guide for more infromation: http://github.com/chadhietala/ember-best-practices/files/guides/no-unguarded-window.md';
7+
8+
function isWindowReference(node) {
9+
return get(node, 'object.name') === 'window';
10+
}
11+
12+
module.exports = {
13+
meta: {
14+
message: MESSAGE
15+
},
16+
create(context) {
17+
let environmentImportBinding;
18+
let destructuredBindings = [];
19+
20+
return {
21+
ObjectPattern(node) {
22+
if (environmentImportBinding) {
23+
destructuredBindings = destructuredBindings.concat(collectObjectPatternBindings(node, {
24+
[environmentImportBinding]: ['isBrowser']
25+
}));
26+
}
27+
},
28+
29+
ImportDefaultSpecifier(node) {
30+
environmentImportBinding = getEnvironmentImportBinding(node);
31+
},
32+
33+
MemberExpression(node) {
34+
if (isWindowReference(node)) {
35+
const isEnvironmentImported = !!environmentImportBinding && destructuredBindings.includes('isBrowser');
36+
const isGuarded = (isEnvironmentImported && isBrowser(node)) || isEnvironmentBrowser(node);
37+
38+
if (!isInActions(node) && !isInteractive(node) && !isGuarded) {
39+
context.report(node, MESSAGE);
40+
}
41+
}
42+
}
43+
};
44+
}
45+
};

lib/rules/no-timers.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
const { getParent } = require('../utils/get');
2-
const { isGuarded } = require('../utils/fast-boot');
1+
const { isInActions, isInteractive } = require('../utils/interactive');
2+
const { isBrowser, isEnvironmentBrowser } = require('../utils/linkedin/bpr');
33

44
const TIMERS = ['setTimeout', 'setInterval'];
55

@@ -9,10 +9,6 @@ function isTimer(name) {
99
return TIMERS.includes(name);
1010
}
1111

12-
function isInAction(node) {
13-
return !!getParent(node, (node) => node.type === 'Property' && node.key.name === 'actions');
14-
}
15-
1612
function getName(node) {
1713
if (!node.callee) {
1814
return '';
@@ -30,8 +26,10 @@ module.exports = {
3026
CallExpression(node) {
3127
let name = getName(node);
3228

33-
if (name && isTimer(name) && !isInAction(node) && !isGuarded(node)) {
34-
context.report(node, MESSAGE);
29+
if (name && isTimer(name)) {
30+
if (!isInActions(node) && !isInteractive(node) && !isBrowser(node) && !isEnvironmentBrowser(node)) {
31+
context.report(node, MESSAGE);
32+
}
3533
}
3634
}
3735
};

lib/rules/no-unguarded-document.js

Lines changed: 0 additions & 23 deletions
This file was deleted.

lib/utils/fast-boot.js

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +0,0 @@
1-
const { getParent } = require('../utils/get');
2-
3-
function isEnvironmentBrowser(node) {
4-
let callee = node.test.callee;
5-
6-
if (callee) {
7-
let obj = callee.object;
8-
let prop = callee.property;
9-
10-
return obj.name === 'environment' && prop.name === 'isBrowser';
11-
}
12-
13-
return false;
14-
}
15-
16-
function isGuarded(node) {
17-
return getParent(node, (node) => node.type === 'IfStatement' && isEnvironmentBrowser(node));
18-
}
19-
20-
module.exports = {
21-
isGuarded
22-
};

lib/utils/interactive.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
const { getParent } = require('../utils/get');
2+
3+
const INTERACTIVE_LIFECYLE_HOOKS = [
4+
'willRender',
5+
'didRender',
6+
'willInsertElement',
7+
'didInsertElement'
8+
];
9+
10+
function isInActions(node) {
11+
return !!getParent(node, (node) => node.type === 'Property' && node.key.name === 'actions');
12+
}
13+
14+
function isInteractive(node) {
15+
return !!getParent(node, node => node.type === 'Property' && INTERACTIVE_LIFECYLE_HOOKS.includes(node.key.name));
16+
}
17+
18+
module.exports = {
19+
isInActions,
20+
isInteractive
21+
};

lib/utils/linkedin/bpr.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
const { get, getParent } = require('../../utils/get');
2+
3+
function _getCondition(node) {
4+
return node.test ? node.test.callee : null;
5+
}
6+
7+
function _isBrowser(node) {
8+
let callee = _getCondition(node);
9+
10+
return callee && callee.name === 'isBrowser';
11+
}
12+
13+
function _isEnvironmentBrowser(node) {
14+
let callee = _getCondition(node);
15+
16+
if (callee) {
17+
let obj = callee.object;
18+
let prop = callee.property;
19+
20+
return obj && obj.name === 'environment' && prop && prop.name === 'isBrowser';
21+
}
22+
23+
return false;
24+
}
25+
26+
function isIfStatement(node) {
27+
return node.type === 'IfStatement';
28+
}
29+
30+
function isBrowser(node) {
31+
return getParent(node, (node) => isIfStatement(node) && _isBrowser(node));
32+
}
33+
34+
function isEnvironmentBrowser(node) {
35+
return getParent(node, (node) => isIfStatement(node) && _isEnvironmentBrowser(node));
36+
}
37+
38+
function getEnvironmentImportBinding(node) {
39+
if (get(node, 'parent.source.raw').includes('ember-stdlib/utils/environment')) {
40+
return node.local.name;
41+
}
42+
}
43+
44+
module.exports = {
45+
isBrowser,
46+
isEnvironmentBrowser,
47+
getEnvironmentImportBinding
48+
};

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
"test": "tests"
99
},
1010
"scripts": {
11-
"test": "mocha tests tests/lib/rules/*.js tests/**/*-test.js"
11+
"test": "mocha --recursive tests",
12+
"test-server": "mocha --recursive --growl --watch tests"
1213
},
1314
"repository": "",
1415
"engines": {

0 commit comments

Comments
 (0)