[Bug]: `no-this-in-sfc` reports false positives on inner functions that are not function components

[juona]

Is there an existing issue for this?

• I have searched the existing issues and my issue is unique
• My issue appears in the command-line and not only in the text editor

Description Overview

The react/no-this-in-sfc rule reports an error if a function component has some internal function that uses this that refers to an entity other than the function component.

const MyComponent = () => {
  function doMagik(this: any, arg: any) {
    this.value = arg; // error here
  }

  class Internal {
    private value: number;

    constructor() {
      this.value = 10; // and here
    }
  }

  doMagik(new Internal());

  return <div />;
};

IMHO whether writing code this way is a "good practice" is a separate concern that could be addressed by a separate linting rule or a config option under this one. The point is that the rule as it stands today reports a false positive. If I understand its intent correctly, that is.

Thank you!

Expected Behavior

The rule should either:

• not report any errors in the above scenarios;
• have a flag that would allow the developer to adjust its behavior so that it does not report any errors in the above scenarios.

eslint-plugin-react version

^7.37.5

eslint version

v9.39.4

node version

v24.14.0

[ljharb]

I mean, defining a function or a class repeatedly on every render isn’t just a bad practice, it’s a horrific one - thats not really something that’s lintable, though.

I agree this is a false positive, but since no reasonable code would or should ever do this, I’m not sure it’s worth the complexity to filter it out.

[juona]

I hope you don't mind me trying to learn something from this. Why is defining a function on every render is a horrific practice? If it were, wouldn't the use of useCallback be enforced? E.g.:

const MyComponent = () => {
  const [value, setValue] = useState<number>(0);

  const onClick = () => {
    setValue(old => old + 1);
  };

  return (
    <div>
      {value}
      <button type="button" onClick={onClick}>
        Do stuff
      </button>
    </div>
  );
};

onClick is recreated every time... Not sure if wrapping it in useCallback brings much benefit. Perhaps you had certain assumptions in mind along with that statement?

[ljharb]

In that example, you'd indeed want to use useCallback on the onClick callback, so it would only be a new function when setValue changed (which it never would) - because then instead of N functions added to the event delegation, only 1 would be.

In general, anything that can be hoisted to module level should be, and that's particularly true in React, where components are rendered a very large number of times.