From da173c4ae9f9b6db26cd6792ccdf7aaa0ba2f4bb Mon Sep 17 00:00:00 2001
From: Siddharth Doshi <sidoshi510@gmail.com>
Date: Sun, 14 Jan 2018 07:32:32 +0530
Subject: [PATCH] Add restricted globals package (#2286)

* 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
---
 packages/confusing-browser-globals/README.md  | 48 +++++++++++++
 packages/confusing-browser-globals/index.js   | 69 +++++++++++++++++++
 .../confusing-browser-globals/package.json    | 21 ++++++
 packages/confusing-browser-globals/test.js    | 20 ++++++
 packages/eslint-config-react-app/index.js     | 61 +---------------
 packages/eslint-config-react-app/package.json |  3 +
 tasks/e2e-simple.sh                           |  9 ++-
 7 files changed, 169 insertions(+), 62 deletions(-)
 create mode 100644 packages/confusing-browser-globals/README.md
 create mode 100644 packages/confusing-browser-globals/index.js
 create mode 100644 packages/confusing-browser-globals/package.json
 create mode 100644 packages/confusing-browser-globals/test.js

diff --git a/packages/confusing-browser-globals/README.md b/packages/confusing-browser-globals/README.md
new file mode 100644
index 00000000000..1f811f71041
--- /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 00000000000..98bf349962e
--- /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 00000000000..211a2df30a9
--- /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 00000000000..bdc26204205
--- /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 2912f8ad603..f029f9626bf 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 4c7172f8a82..806b05b29a7 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 b566ae37a3d..8ea08b55611 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.