no-did-(update|mount)-set-state: disallow-via-methods option
#742
Conversation
avaly
force-pushed
the
no-did-update-set-state-via-methods
branch
2 times, most recently
from
deb7fb5 to
c62b0f4
Compare
|
I'm not sure this needs to be a separate rule. It would also be nice if it allowed these function calls as callbacks with an option to disallow, like we do with the What do other folks think? |
avaly
force-pushed
the
no-did-update-set-state-via-methods
branch
from
c62b0f4 to
9fccdf9
Compare
avaly
changed the title
disallow-via-methods option
|
I've updated my PR to not create a separate rule for this use case. Instead it uses the logic in both |
|
👍 / 👎 ? Friendly bump! |
|
|
||
| The following patterns are considered warnings: | ||
|
|
||
| ```js |
There was a problem hiding this comment.
nit: these should be jsx
| ```js | ||
| ... | ||
| "no-did-update-set-state": [<enabled>, <mode>] | ||
| "no-did-update-set-state": [<enabled>, <modeInFunctions>, <modeViaMethods>] |
There was a problem hiding this comment.
It might be a good idea to use an options object for these modes instead of positional arguments.
That would be a breaking change, so perhaps it should be done in a followup PR.
There was a problem hiding this comment.
Both could be supported with eslint schemas as semver-minor - ie, a "mode" string, and then otherwise, an options object.
|
Ah yes, that would be preferable
…On Wed, Feb 15, 2017, 11:54 AM Jordan Harband ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In docs/rules/no-did-update-set-state.md
<#742>:
> @@ -51,7 +51,7 @@ var Hello = React.createClass({
```js
...
-"no-did-update-set-state": [<enabled>, <mode>]
+"no-did-update-set-state": [<enabled>, <modeInFunctions>, <modeViaMethods>]
Both could be supported with eslint schemas as semver-minor - ie, a "mode"
string, and then otherwise, an options object.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#742>, or mute the
thread
<https://github.com/notifications/unsubscribe-auth/AAL7zgojwTRK-O-tXmnYP-YhdnJ4BeH8ks5rc1fcgaJpZM4JdpvP>
.
|
There was a problem hiding this comment.
This needs a rebase, and it'd be great to use an options object per https://github.com/yannickcr/eslint-plugin-react/pull/742/files#r101368104
|
I've rebased this, but a few tests are failing. |
ljharb
force-pushed
the
master
branch
6 times, most recently
from
59af733 to
865ed16
Compare
ljharb
force-pushed
the
master
branch
3 times, most recently
from
069314a to
181c68f
Compare
ljharb
force-pushed
the
master
branch
2 times, most recently
from
380e32c to
51d342b
Compare
4 participants
[RFC] Adding a new rule to complement the existing
no-did-update-set-state.This rule will look for any occurrences of
this.setStateand check whether the method it is being called from is also called insidecomponentDidUpdate.If this gets a positive feedback I'll also add a doc page for it.
I could see this as an option on the
no-did-update-set-staterule, but the logic is more complex than the original rule, so I decided to keep it separate for now.