Disabling prop-types on the function declaration line is no longer always sufficient

[ThiefMaster]

I have this function (which isn't even a component):

// eslint-disable-next-line react/prop-types
const renderEquipment = ({equipment, features}) => {
  const count = equipment.length + features.length;
  if (!count) {
    return null;
  }
  return (
    <>
      <Translate>Equipment</Translate>
      <Label circular horizontal className="white" size="tiny" styleName="filter-bar-button-label">
        {count}
      </Label>
    </>
  );
};

As you can see, I disabled the prop-types validation on it. However, after updating from 7.13 to 7.14.1, I'm now getting additional errors on the line where I'm accessing the length of the arrays:

  39:27  error  'equipment.length' is missing in props validation  react/prop-types
  39:45  error  'features.length' is missing in props validation   react/prop-types

I guess this happened because now accessing nested props also trigger a prop-type validation - but of course this makes it very ugly to disable prop-type validation for a whole (non-)component...

Maybe the component detection should never consider functions whose name starts with a lowercase character components, since they can't be components (when used in JSX)...

[yannickcr]

Props detection became more powerful in last release, hence these new errors.

Disabling component detection on lowercase functions was discussed in #512 but no decision was made yet.

Right now the easiest way to disable react/prop-types on your component is to wrap it between eslint-disable/eslint-enable directives:

/* eslint-disable react/prop-types */
const renderEquipment = ({equipment, features}) => {
  const count = equipment.length + features.length;
  if (!count) {
    return null;
  }
  return (
    <>
      <Translate>Equipment</Translate>
      <Label circular horizontal className="white" size="tiny" styleName="filter-bar-button-label">
        {count}
      </Label>
    </>
  );
};
/* eslint-enable react/prop-types */

[ThiefMaster]

Yes, that's what I went for in the end.

[GreenGremlin]

This seems like a breaking change, in a minor version release. Can it be rolled back and pushed in a major version release?

[GreenGremlin]

Also, this is going to be very confusing for developers. Where was this change discussed?

[ljharb]

the issue here isn’t the change; its that that render function is a component by our heuristic (I’d argue it is one conceptually and thus should be defined as one, but that’s a separate topic).

If you define a propType for both of those props as an array, you won’t get a warning.

[GreenGremlin]

Also, this is going to be very confusing for developers. Where was this change discussed?

Digging a little more, I'm ok with the change, just not rolling it out as a minor version bump.

[yannickcr]

We try to follow the same Semantic Versioning Policy as ESLint, this case being covered by:

A bug fix in a rule that results in ESLint reporting more errors.

So AFAIK it is ok to release it in a minor.

[jzabala]

With #2699 render functions are not considered components. This one should be fixed now.

@ljharb