Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add restricted globals package #2286

Merged
merged 10 commits into from
Jan 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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