add rule for no setState in componentDidUpdate and tests

[VictorHom]

rule

from the sheet in #341

the rule is for ES5, but I added checks for ES6 class components. Let me know if that's not correct. Happy to make changes.

[sebmck]

This only covers setState in very specific positions. This rule could be simplified with only:

function inComponentDidUpdate(path: Path): boolean {
  return path.findAncestry(({node}) => node.type === 'ClassMethod' && node.key.type === 'StaticPropertyKey' && node.key.value.type === 'Identifier' && node.key.value.name === 'componentDidUpdate') !== undefined;
}

if (doesNodeMatchPattern(node, 'this.setState') && inComponentDidUpdate(path)) {
  // Add diagnostic
}

I wouldn't worry about checking if the class extends matches React.Component or Component since a componentDidUpdate method is already enough signal that we're in a component.

[VictorHom]

@sebmck , so it sounds like the createClass/createReactClass case isn't something we should write against in tests then? (from checking your snippet)

One question from the docs: https://reactjs.org/docs/react-component.html#componentdidupdate

You may call setState() immediately in componentDidUpdate() but note that it must be wrapped in a condition like in the example above, or you’ll cause an infinite loop. It would also cause an extra re-rendering which, while not visible to the user, can affect the component performance. If you’re trying to “mirror” some state to a prop coming from above, consider using the prop directly instead. Read more about why copying props into state causes bugs.

Should wrapped this.setState be an exception in the lint rule?

[sebmck]

Yeah don't bother handling createClass or createReactClass.

To handle the conditional case you could do:

import {isFunctionNode, isConditional} from '@romejs/js-ast-utils';

function inComponentDidUpdate(path: Path): boolean {
  const func = path.findAncestry(({node}) => isFunctionNode(node) || isConditional(node));
  return func !== undefined && func.type === 'ClassMethod' && func.key.type === 'StaticPropertyKey' && func.key.value.type === 'Identifier' && func.key.value.name === 'componentDidUpdate';
}

if (doesNodeMatchPattern(node, 'this.setState') && inComponentDidUpdate(path)) {
  // Add diagnostic
}

[VictorHom]

@sebmck had to modify your suggestion with 2 path.findAncestry calls. let me know if that doesn't work

[sebmck]

Looks good to me. We're only incurring the findAncestry overhead when the node matches this.setState so this is probably as fast as it could be.

[sebmck]

Thank you!