From 903efe5cc6812ecf2edde5772db706f901bfd0a8 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 20:18:08 +0200 Subject: [PATCH 01/11] Fix for linting failure It seems like the linter is failing because of an error in the babel eslint package combined with dependencies being called in way that conflicts with yarn workspaces. --- packages/react-error-overlay/package.json | 2 +- tasks/e2e-simple.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-error-overlay/package.json b/packages/react-error-overlay/package.json index aa28f9dba96..fabf72cdd16 100644 --- a/packages/react-error-overlay/package.json +++ b/packages/react-error-overlay/package.json @@ -37,7 +37,7 @@ "@babel/code-frame": "7.5.5", "@babel/core": "7.5.5", "anser": "1.4.8", - "babel-eslint": "10.0.2", + "babel-eslint": "10.0.3", "babel-jest": "^24.8.0", "babel-loader": "8.0.6", "babel-preset-react-app": "^9.0.1", diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index ff6d2954b5f..2718a5c3cff 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -96,7 +96,7 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml ./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/ cd packages/react-error-overlay/ -./node_modules/.bin/eslint --max-warnings 0 src/ +yarn workspace react-error-overlay run eslint --max-warnings 0 src/ yarn test if [ $APPVEYOR != 'True' ]; then # Flow started hanging on AppVeyor after we moved to Yarn Workspaces :-( From 94a6a3d8d985c111b0df3879991770d238b1378f Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 20:31:25 +0200 Subject: [PATCH 02/11] Add lerna bootstrap to script The yarn workspace command works locally but not in the script so I figured I would add lerna bootstrap --- tasks/e2e-simple.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index 2718a5c3cff..1d3dc690864 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -83,6 +83,7 @@ fi # Bootstrap monorepo yarn +lerna bootstrap # Start the local NPM registry startLocalRegistry "$root_path"/tasks/verdaccio.yaml @@ -97,6 +98,7 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml cd packages/react-error-overlay/ yarn workspace react-error-overlay run eslint --max-warnings 0 src/ + yarn test if [ $APPVEYOR != 'True' ]; then # Flow started hanging on AppVeyor after we moved to Yarn Workspaces :-( From 1a4e5876c8b568e9af244f2c3eec466ce89e4e83 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 20:35:32 +0200 Subject: [PATCH 03/11] Forgot to call lerna through yarn --- tasks/e2e-simple.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index 1d3dc690864..bec4e9f11af 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -83,7 +83,7 @@ fi # Bootstrap monorepo yarn -lerna bootstrap +yarn lerna bootstrap # Start the local NPM registry startLocalRegistry "$root_path"/tasks/verdaccio.yaml From df9213c85143546bfb88b43e614f8169376b3d6f Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 20:51:50 +0200 Subject: [PATCH 04/11] Add lock file Adding lock file as it is the only thing that differs from my implementation and the one in the script From b7843fb5dc23af83967b47a3eec89df42252162b Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 21:10:28 +0200 Subject: [PATCH 05/11] Update lock file From dc9535aa80ae3ebbcba310a3b64193167c7c9fe2 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 21:20:27 +0200 Subject: [PATCH 06/11] Running eslint in overlay like the others Seems like there is some issue with trying to run the script the way I would like, trying running eslint like the others that exist in the script. --- tasks/e2e-simple.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index bec4e9f11af..aa9bff9fac6 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -95,9 +95,9 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml ./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/ +./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/src/ cd packages/react-error-overlay/ -yarn workspace react-error-overlay run eslint --max-warnings 0 src/ yarn test if [ $APPVEYOR != 'True' ]; then From e3ee4291bb6583b08ec7733a18de34d8d6f29482 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 21:39:05 +0200 Subject: [PATCH 07/11] Install babel eslint This is a bit of a last resort, I want to see if anything else fails. I don't particularly like this solution --- tasks/e2e-simple.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index aa9bff9fac6..71fc54218e7 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -83,7 +83,6 @@ fi # Bootstrap monorepo yarn -yarn lerna bootstrap # Start the local NPM registry startLocalRegistry "$root_path"/tasks/verdaccio.yaml @@ -95,10 +94,11 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml ./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/ -./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/src/ -cd packages/react-error-overlay/ +yarn workspace react-error-overlay add -D babel-eslint +yarn workspace react-error-overlay run eslint --max-warnings 0 src/ +cd packages/react-error-overlay/ yarn test if [ $APPVEYOR != 'True' ]; then # Flow started hanging on AppVeyor after we moved to Yarn Workspaces :-( From af38857740e2ee989a2c09275f2c6a4d15112e29 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 21:42:55 +0200 Subject: [PATCH 08/11] Revert eslint upgrade --- packages/react-error-overlay/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-error-overlay/package.json b/packages/react-error-overlay/package.json index fabf72cdd16..aa28f9dba96 100644 --- a/packages/react-error-overlay/package.json +++ b/packages/react-error-overlay/package.json @@ -37,7 +37,7 @@ "@babel/code-frame": "7.5.5", "@babel/core": "7.5.5", "anser": "1.4.8", - "babel-eslint": "10.0.3", + "babel-eslint": "10.0.2", "babel-jest": "^24.8.0", "babel-loader": "8.0.6", "babel-preset-react-app": "^9.0.1", From 5edd8445271b054b3b39f622768f9496ba5b9d59 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 21:49:02 +0200 Subject: [PATCH 09/11] Babel eslint needs to be locked to 10.0.2 React scripts requires that babel eslint is 10.0.2 --- tasks/e2e-simple.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index 71fc54218e7..0f36a8abf97 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -95,7 +95,7 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml ./node_modules/.bin/eslint --max-warnings 0 packages/react-dev-utils/ ./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/ -yarn workspace react-error-overlay add -D babel-eslint +yarn workspace react-error-overlay add -D babel-eslint@10.0.2 yarn workspace react-error-overlay run eslint --max-warnings 0 src/ cd packages/react-error-overlay/ From f3e7ff4d673091e1b5fe5c5270f8c7560cba868e Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Fri, 6 Sep 2019 22:06:55 +0200 Subject: [PATCH 10/11] Move to babel eslint 10.0.3 This was needed to fix for in false no unused var bug --- packages/react-error-overlay/package.json | 2 +- packages/react-error-overlay/src/utils/dom/css.js | 2 +- packages/react-scripts/package.json | 2 +- tasks/e2e-simple.sh | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-error-overlay/package.json b/packages/react-error-overlay/package.json index aa28f9dba96..fabf72cdd16 100644 --- a/packages/react-error-overlay/package.json +++ b/packages/react-error-overlay/package.json @@ -37,7 +37,7 @@ "@babel/code-frame": "7.5.5", "@babel/core": "7.5.5", "anser": "1.4.8", - "babel-eslint": "10.0.2", + "babel-eslint": "10.0.3", "babel-jest": "^24.8.0", "babel-loader": "8.0.6", "babel-preset-react-app": "^9.0.1", diff --git a/packages/react-error-overlay/src/utils/dom/css.js b/packages/react-error-overlay/src/utils/dom/css.js index 4f2dbf53c8d..2280dada70a 100644 --- a/packages/react-error-overlay/src/utils/dom/css.js +++ b/packages/react-error-overlay/src/utils/dom/css.js @@ -35,7 +35,7 @@ function removeCss(document: Document, ref: number) { function applyStyles(element: HTMLElement, styles: Object) { element.setAttribute('style', ''); - for (const key in styles) { + for (let key in styles) { if (!styles.hasOwnProperty(key)) { continue; } diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index 6d86a6d8daa..52d7f56f21b 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -32,7 +32,7 @@ "@svgr/webpack": "4.3.2", "@typescript-eslint/eslint-plugin": "1.13.0", "@typescript-eslint/parser": "1.13.0", - "babel-eslint": "10.0.2", + "babel-eslint": "10.0.3", "babel-jest": "^24.8.0", "babel-loader": "8.0.6", "babel-plugin-named-asset-import": "^0.3.3", diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index 0f36a8abf97..9ea8e808d39 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -95,7 +95,7 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml ./node_modules/.bin/eslint --max-warnings 0 packages/react-dev-utils/ ./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/ -yarn workspace react-error-overlay add -D babel-eslint@10.0.2 +yarn workspace react-error-overlay add -D babel-eslint@10.0.3 yarn workspace react-error-overlay run eslint --max-warnings 0 src/ cd packages/react-error-overlay/ From 916fe006491e917a9430a92de47e502173f506bd Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Sat, 7 Sep 2019 08:44:09 +0200 Subject: [PATCH 11/11] Add packages to eslint plugin Found a better solution by adding a few packages to eslint config react app, this should fix it so that it should always work in the tests. Using yarn workspaces or lerna in the scripts should probably be an improvement in another PR. --- packages/eslint-config-react-app/package.json | 4 ++++ packages/react-error-overlay/src/utils/dom/css.js | 2 +- tasks/e2e-simple.sh | 4 +--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/eslint-config-react-app/package.json b/packages/eslint-config-react-app/package.json index e991f8d5589..ce9adbd90cf 100644 --- a/packages/eslint-config-react-app/package.json +++ b/packages/eslint-config-react-app/package.json @@ -27,5 +27,9 @@ }, "dependencies": { "confusing-browser-globals": "^1.0.8" + }, + "devDependencies": { + "babel-eslint": "10.0.3", + "eslint-plugin-react-hooks": "^2.0.1" } } diff --git a/packages/react-error-overlay/src/utils/dom/css.js b/packages/react-error-overlay/src/utils/dom/css.js index 2280dada70a..4f2dbf53c8d 100644 --- a/packages/react-error-overlay/src/utils/dom/css.js +++ b/packages/react-error-overlay/src/utils/dom/css.js @@ -35,7 +35,7 @@ function removeCss(document: Document, ref: number) { function applyStyles(element: HTMLElement, styles: Object) { element.setAttribute('style', ''); - for (let key in styles) { + for (const key in styles) { if (!styles.hasOwnProperty(key)) { continue; } diff --git a/tasks/e2e-simple.sh b/tasks/e2e-simple.sh index 9ea8e808d39..ff6d2954b5f 100755 --- a/tasks/e2e-simple.sh +++ b/tasks/e2e-simple.sh @@ -95,10 +95,8 @@ startLocalRegistry "$root_path"/tasks/verdaccio.yaml ./node_modules/.bin/eslint --max-warnings 0 packages/react-dev-utils/ ./node_modules/.bin/eslint --max-warnings 0 packages/react-scripts/ -yarn workspace react-error-overlay add -D babel-eslint@10.0.3 -yarn workspace react-error-overlay run eslint --max-warnings 0 src/ - 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 :-(