Extract used propTypes detection

[alexzherdev]

Pushing for visibility.
This is failing one existing test, which will get fixed after #1939 is merged (also will need a rebase).

Resolves #963, resolves #1871.

[ljharb]

LGTM overall

[ljharb]

for..in should be avoided; can this use normal array iteration methods?

[alexzherdev]

Sure. Object.keys(list).forEach perhaps?
There's about a dozen other files already doing the same, handle them all?

[ljharb]

yes, that would be great!

[alexzherdev]

already done in b35ea0c 😄

[alexzherdev]

I went ahead and fixed other for..in occurrences; I can go back and undo those if necessary. Also rebased.
prop-types is 201 line now, no-unused-prop-types is 144 🎉

[ljharb]

maybe if (list[component].invalidProps && list[component].invalidProps.length > 0)?

[ljharb]

Object.keys(list).filter(component => !list[component].hasDisplayName).forEach?

[ljharb]

(component, i) and then we don't need to ++ it, nor have let i = 0 above.

[alexzherdev]

Can't do this because we only want to ++i if isIgnored is false and doesn't short-circuit. I.e. we want to start reporting from the second non-ignored component.

[alexzherdev]

Actually found a way to achieve this, also using filter.

[ljharb]

Object.keys(list).filter(component => !isValid(list[component])).forEach?

[ljharb]

Object.keys(list).filter(component => mustBeValidated(list[component])).forEach?

[ljharb]

no need for the else

[ljharb]

also here (ie, the no-else-return eslint rule)

[alexzherdev]

yeah, I wonder why this isn't warning

[ljharb]

name in Object.prototype would be better, so as not to trigger getters

[ljharb]

this is just:

if (node.properties.length > 0) {
  markPropTypesAsUsed(node.parent);
} 

the forEach seems useless?

[ljharb]

Object.keys(list).filter(component => mustBeValidated(list[component])).forEach?

[ThiefMaster]

I believe this change caused #2017.

The old logic was:

    function isPropTypesUsage(node) {
      const isClassUsage = (
        (utils.getParentES6Component() || utils.getParentES5Component()) &&
        node.object.type === 'ThisExpression' && node.property.name === 'props'
      );
      const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node);
      const isNextPropsUsage = node.object.name === 'nextProps' && (inComponentWillReceiveProps() || inShouldComponentUpdate());
      return isClassUsage || isStatelessFunctionUsage || isNextPropsUsage;
}

and it got replaced with:

  function isPropTypesUsage(node) {
    const isClassUsage = (
      (utils.getParentES6Component() || utils.getParentES5Component()) &&
      ((node.object.type === 'ThisExpression' && node.property.name === 'props')
      || isPropArgumentInSetStateUpdater(node))
    );
    const isStatelessFunctionUsage = node.object.name === 'props' && !isAssignmentToProp(node);
    return isClassUsage || isStatelessFunctionUsage || inLifeCycleMethod();
  }