-
Notifications
You must be signed in to change notification settings - Fork 110
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
chore: fix for deprecated eslint rule #165
Conversation
Thanks @cwonrails ! We'll need to remember to follow up on this when import-js/eslint-plugin-import#390 lands. |
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'll leave to @ccpricenytimes for an additional check & merge.
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.
Looks awesome. One tiny change
@@ -20,7 +20,8 @@ | |||
"prefer-reflect": 2, | |||
"no-warning-comments": [1, { "terms": ["todo", "fixme"], "location": "start" }], | |||
"react/sort-comp": 0, | |||
"react/jsx-filename-extension": [1, { "extensions": [".js", ".jsx"] }], | |||
"react/require-extension": "off", | |||
"import/extensions": [1, { "js": "never", "jsx": "never" }], |
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.
You can remove jsx here since kyt doesn't support that extension.
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.
Will do.
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.
Fix pushed, let me know if I should squash or amend the original instead based on the Times' preference.
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.
We squash & merge PRs, so it should be fine.
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 can squash when we merge from github so separate commits is fine
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.
Great, thanks! Here's the followup commit: c7dadc3
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.
Looks great and works for me!
* master: fix for deprecated eslint rule (#165) # Conflicts: # .eslintrc
* feature/jest: Remove unused README PR comments Add reference to jest config fix for deprecated eslint rule (#165)
Closes #164
Replaces the deprecated
react/require-extension
in.eslintrc
with the recommendedimport/extensions
rule fromeslint-plugin-import
.Additionally, applies this fix to prevent the following (now incorrect) console output:
The react/require-extension rule is deprecated. Please use the import/extensions rule from eslint-plugin-import instead.
See these lines in
eslint-config-airbnb
for the relevant TODO.