-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Update dependencies #3600
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
Update dependencies #3600
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,10 @@ | ||
{ | ||
"extends": "rackt", | ||
"extends": ["rackt", "plugin:react/recommended"], | ||
"globals": { | ||
"__DEV__": true | ||
}, | ||
"rules": { | ||
"react/jsx-uses-react": 1, | ||
"react/jsx-no-undef": 2, | ||
"react/display-name": 0, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These are in the recommended config |
||
"react/wrap-multilines": 2 | ||
}, | ||
"plugins": [ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"rules": { | ||
"react/prop-types": 0 | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,9 +27,6 @@ const App = React.createClass({ | |
}, | ||
|
||
updateContacts() { | ||
if (!this.isMounted()) | ||
return | ||
|
||
this.setState({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think these isMounted checks were no-ops. The example tears down the subscribers on unmount. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or anyway I don't get state warnings when I run through the example. |
||
contacts: ContactStore.getContacts(), | ||
loading: false | ||
|
@@ -91,9 +88,6 @@ const Contact = withRouter( | |
}, | ||
|
||
updateContact() { | ||
if (!this.isMounted()) | ||
return | ||
|
||
this.setState(this.getStateFromStore()) | ||
}, | ||
|
||
|
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 want to cut this over to Airbnb + overrides, but this is a smaller step.
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've been using this config lately:
Regardless, we should probably remove the
rackt
extension, since that repo doesn't exist anymore.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.
It actually took me a bit to find it; looks like @pwmckenna adopted it at https://github.com/pwmckenna/eslint-config-rackt.
I like Airbnb because it's a bit more restrictive and has gone through the effort of setting all the rules I never think about, but if it's more than a half dozen or so overrides, it might be better to go with the above.
We'll see – I want to tackle really updating the ESLint config in a separate PR though.
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, that's fair. It's a separate thing.