diff --git a/packages/confusing-browser-globals/README.md b/packages/confusing-browser-globals/README.md new file mode 100644 index 0000000000..1f811f7104 --- /dev/null +++ b/packages/confusing-browser-globals/README.md @@ -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 diff --git a/packages/confusing-browser-globals/index.js b/packages/confusing-browser-globals/index.js new file mode 100644 index 0000000000..98bf349962 --- /dev/null +++ b/packages/confusing-browser-globals/index.js @@ -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', +]; diff --git a/packages/confusing-browser-globals/package.json b/packages/confusing-browser-globals/package.json new file mode 100644 index 0000000000..211a2df30a --- /dev/null +++ b/packages/confusing-browser-globals/package.json @@ -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" + } +} diff --git a/packages/confusing-browser-globals/test.js b/packages/confusing-browser-globals/test.js new file mode 100644 index 0000000000..bdc2620420 --- /dev/null +++ b/packages/confusing-browser-globals/test.js @@ -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'); +}); diff --git a/packages/eslint-config-react-app/index.js b/packages/eslint-config-react-app/index.js index 2912f8ad60..f029f9626b 100644 --- a/packages/eslint-config-react-app/index.js +++ b/packages/eslint-config-react-app/index.js @@ -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, diff --git a/packages/eslint-config-react-app/package.json b/packages/eslint-config-react-app/package.json index b15be1dd29..6f2b244a1e 100644 --- a/packages/eslint-config-react-app/package.json +++ b/packages/eslint-config-react-app/package.json @@ -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" } } diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index b566ae37a3..8ea08b5561 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -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.