Skip to content

Commit

Permalink
Add restricted globals package (#2286)
Browse files Browse the repository at this point in the history
* Add restricted globals package

* Use new package in eslint-config

* Add eslint-restricted-globals dependency

* Fixes

* Update dependencies

* Update test and README

* Use jest

* tweaks

* Add lint/test to CI

* Fix lint
  • Loading branch information
sidoshi authored and Timer committed Jan 14, 2018
1 parent 7079451 commit 2d07427
Show file tree
Hide file tree
Showing 7 changed files with 169 additions and 62 deletions.
48 changes: 48 additions & 0 deletions packages/confusing-browser-globals/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# confusing-browser-globals

A curated list of browser globals that commonly cause confusion and are not recommended to use without an explicit `window.` qualifier.

## Motivation

Some global variables in browser are likely to be used by people without the intent of using them as globals, such as `status`, `name`, `event`, etc.

For example:

```js
handleClick() { // missing `event` argument
this.setState({
text: event.target.value // uses the `event` global: oops!
});
}
```

This package exports a list of globals that are often used by mistake. You can feed this list to a static analysis tool like ESLint to prevent their usage without an explicit `window.` qualifier.


## Installation

```
npm install --save confusing-browser-globals
```


## Usage

If you use Create React App, you don't need to configure anything, as this rule is already included in the default `eslint-config-react-app` preset.

If you maintain your own ESLint configuration, you can do this:

```js
var restrictedGlobals = require('confusing-browser-globals');

module.exports = {
rules: {
'no-restricted-globals': ['error'].concat(restrictedGlobals),
}
};
```


## License

MIT
69 changes: 69 additions & 0 deletions packages/confusing-browser-globals/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

module.exports = [
'addEventListener',
'blur',
'close',
'closed',
'confirm',
'defaultStatus',
'defaultstatus',
'event',
'external',
'find',
'focus',
'frameElement',
'frames',
'history',
'innerHeight',
'innerWidth',
'length',
'location',
'locationbar',
'menubar',
'moveBy',
'moveTo',
'name',
'onblur',
'onerror',
'onfocus',
'onload',
'onresize',
'onunload',
'open',
'opener',
'opera',
'outerHeight',
'outerWidth',
'pageXOffset',
'pageYOffset',
'parent',
'print',
'removeEventListener',
'resizeBy',
'resizeTo',
'screen',
'screenLeft',
'screenTop',
'screenX',
'screenY',
'scroll',
'scrollbars',
'scrollBy',
'scrollTo',
'scrollX',
'scrollY',
'self',
'status',
'statusbar',
'stop',
'toolbar',
'top',
];
21 changes: 21 additions & 0 deletions packages/confusing-browser-globals/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
"name": "confusing-browser-globals",
"version": "1.0.0",
"description": "A list of browser globals that are often used by mistake instead of local variables",
"license": "MIT",
"main": "index.js",
"scripts": {
"test": "jest"
},
"repository": "facebookincubator/create-react-app",
"keywords": [
"eslint",
"globals"
],
"files": [
"index.js"
],
"devDependencies": {
"jest": "22.0.6"
}
}
20 changes: 20 additions & 0 deletions packages/confusing-browser-globals/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/* eslint-env jest */

'use strict';

let globals = require('./index');

it('should return an Array of globals', () => {
expect(Array.isArray(globals)).toBe(true);
});

it('should contain "event" variable', () => {
expect(globals).toContain('event');
});
61 changes: 1 addition & 60 deletions packages/eslint-config-react-app/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,66 +21,7 @@
// This is dangerous as it hides accidentally undefined variables.
// We blacklist the globals that we deem potentially confusing.
// To use them, explicitly reference them, e.g. `window.name` or `window.status`.
var restrictedGlobals = [
'addEventListener',
'blur',
'close',
'closed',
'confirm',
'defaultStatus',
'defaultstatus',
'event',
'external',
'find',
'focus',
'frameElement',
'frames',
'history',
'innerHeight',
'innerWidth',
'length',
'location',
'locationbar',
'menubar',
'moveBy',
'moveTo',
'name',
'onblur',
'onerror',
'onfocus',
'onload',
'onresize',
'onunload',
'open',
'opener',
'opera',
'outerHeight',
'outerWidth',
'pageXOffset',
'pageYOffset',
'parent',
'print',
'removeEventListener',
'resizeBy',
'resizeTo',
'screen',
'screenLeft',
'screenTop',
'screenX',
'screenY',
'scroll',
'scrollbars',
'scrollBy',
'scrollTo',
'scrollX',
'scrollY',
'self',
'status',
'statusbar',
'stop',
'toolbar',
'top',
];
var restrictedGlobals = require('confusing-browser-globals');

module.exports = {
root: true,
Expand Down
3 changes: 3 additions & 0 deletions packages/eslint-config-react-app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@
"eslint-plugin-import": "^2.6.0",
"eslint-plugin-jsx-a11y": "^6.0.2",
"eslint-plugin-react": "^7.1.0"
},
"dependencies": {
"confusing-browser-globals": "^1.0.0"
}
}
9 changes: 7 additions & 2 deletions tasks/e2e-simple.sh
Original file line number Diff line number Diff line change
Expand Up @@ -100,24 +100,29 @@ npx npm-cli-login@0.0.10 -u user -p password -e user@example.com -r "$custom_reg

# Lint own code
./node_modules/.bin/eslint --max-warnings 0 packages/babel-preset-react-app/
./node_modules/.bin/eslint --max-warnings 0 packages/confusing-browser-globals/
./node_modules/.bin/eslint --max-warnings 0 packages/create-react-app/
./node_modules/.bin/eslint --max-warnings 0 packages/eslint-config-react-app/
./node_modules/.bin/eslint --max-warnings 0 packages/react-dev-utils/
./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/

cd packages/react-error-overlay/
./node_modules/.bin/eslint --max-warnings 0 src/
yarn test

if [ $APPVEYOR != 'True' ]; then
# Flow started hanging on AppVeyor after we moved to Yarn Workspaces :-(
yarn flow
fi

cd ../..

cd packages/react-dev-utils/
yarn test
cd ../..

cd packages/confusing-browser-globals/
yarn test
cd ../..

# ******************************************************************************
# First, test the create-react-app development environment.
# This does not affect our users but makes sure we can develop it.
Expand Down

0 comments on commit 2d07427

Please sign in to comment.