-
Notifications
You must be signed in to change notification settings - Fork 5
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
UW-24112 Update eslint npm dependencies #78
Conversation
Update eslint to version 7 Fix linting errors
package.json
Outdated
@@ -32,7 +32,7 @@ | |||
"babel-plugin-dynamic-import-node": "2.3.3", | |||
"babel-plugin-lodash": "3.3.4", | |||
"css-loader": "3.4.2", | |||
"eslint": "6.8.0", | |||
"eslint": "7.12.1", | |||
"eslint-config-airbnb-base": "14.0.0", | |||
"eslint-plugin-import": "2.20.0", | |||
"eslint-plugin-vue": "6.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we bump all the eslint*
deps in tandem to ensure they all play nice together? The vue one is a major bump so unsure if that brings about new rules that cause breakages for us or not? If it does, we can hold off on that and do it separately
eslint 6.8.0 6.8.0 7.12.1 vue-ssr-build
eslint-config-airbnb-base 14.0.0 14.0.0 14.2.0 vue-ssr-build
eslint-plugin-import 2.20.0 2.20.0 2.22.1 vue-ssr-build
eslint-plugin-vue 6.1.2 6.1.2 7.1.0 vue-ssr-build
And then we can bump the version of this to the next beta release and I'll tag once merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried a very quick install of this branch over in SlipStream and got the following error trying to lint. I'm going to try a more thorough npm ci
though and see if it was maybe just a package mismatch from a different branch:
Oops! Something went wrong! :(
ESLint: 7.12.1
TypeError: Cannot read property 'value' of null
Occurred while linting /Users/brophym1/urbn/repos/slipstream/.storybook/preview.js:143
at checkSpacingBefore (/Users/brophym1/urbn/repos/slipstream/node_modules/eslint/lib/rules/template-curly-spacing.js:52:24)
at TemplateElement (/Users/brophym1/urbn/repos/slipstream/node_modules/eslint/lib/rules/template-curly-spacing.js:136:17)
at /Users/brophym1/urbn/repos/slipstream/node_modules/eslint/lib/linter/safe-emitter.js:45:58
...
Just want to make sure that we don't need a subsequent change (and thus a new beta release) after we merge/tag this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I followed the solution steps given in babel/babel#10904 and pushed that change here. Now on master of SlipStream, I have 167 linting errors (mostly coming from test and storybook files). Should I push up those fixes under another UW-24112 PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah - let me see what they look like and we can see if they're rules we want to enforce or not also. LEt's get this merged and then I'll bump to beta.5 and we can open a PR in slipstram to point to that and see what the errors look like
Ticket: https://urbnit.atlassian.net/browse/UW-24112
Doing as suggested in https://github.com/urbn/SlipStream/pull/4370#pullrequestreview-518171699