-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Fix eslint errors #3374
Fix eslint errors #3374
Conversation
this.setState({ | ||
preview: markdown.toHTML(this.state.text), | ||
preview: markdown.toHTML(text), |
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.
What lint error is 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.
react/no-access-state-in-setstate
} | ||
|
||
init.init = true; | ||
|
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.
This is related?
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.
Ah, I just noticed that this directive is not used anymore (checked twice 😄), so decided to remove it instead of fixing eslint
warnings there.
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.
One question about the removal of client/app/components/transclude-replace.js
- is it related to this PR?
If so, go ahead and merge before anyone else.
@@ -47,13 +47,13 @@ export class DynamicForm extends React.Component { | |||
} | |||
|
|||
setActionInProgress = (actionName, inProgress) => { | |||
this.setState({ | |||
this.setState(prevState => ({ |
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.
Cool, I didn't know prevState
existed here 😅
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.
Me too. I just read the docs 😄
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.
BTW - it's the same warning as @ranbena mentioned above - react/no-access-state-in-setstate
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.
Also, eslint
is smart enough to detect code like this (the same rule):
updateCounter() {
const counter = this.state.counter + 1;
this.setState({ counter });
}
(I tried to modify code this way, but warning didn't disappear 😅)
Mostly just removed extra empty lines at EoF.
@ranbena and @gabrieldutra There are also some fixes to components you were recently working on, so please check it. Thanks!