[Bug]: `react/no-unused-class-component-methods` instance property React element false positive

[jwoo92]

Is there an existing issue for this?

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

Description Overview

After upgrading from babel-eslint@^10.1.0 to @babel/eslint-parser@^7.22.7, using an instance property as a React element leads to a react/no-unused-class-component-methods false positive.

Steps to Replicate

class ViewModel extends React.Component {
  constructor(props) {
    super(props);

    this.View = View;
  }

  render() {
    return <this.View />;
  }
}

Using the example code above, run the following command in your CLI:

eslint . --ext .js,.jsx

Expected Behavior

ESLint should not return an error.

eslint-plugin-react version

v7.23.2

eslint version

v8.42.0

node version

v18.16.1

[ljharb]

That's a really strange usage pattern - why does it need to be stored on the instance, instead of just referenced via closure?

[jwoo92]

While a bare minimum example, real-world use includes legacy class-based view models before functional/hooks became trendy.

[ljharb]

React class components are never subclassed by anything, and this.View isn't set to anything dynamic, so I'm still not sure why that would have ever been a reasonable pattern.

[jwoo92]

this.View is so that the view component is not seen as unique causing premature re-rendering.

[ljharb]

@jwoo92 i have no idea what that means - React only ever considers props and state and component identity, and ignores instance properties, and that's always been the case.

[jwoo92]

@ljharb Jordan, thank you for helping me comprehend your viewpoint on this design pattern. I'd be happy to discuss it further with you separately from the issue at hand.

To keep our conversation on track, I have a couple examples of the package behavior and false positive.

class ViewModel extends React.Component {
  constructor(props) {
    super(props);

    this.View = View; // No ESLint error
  }

  render() {
    const { View } = this;

    return <View />;
  }
}

class ViewModel extends React.Component {
  constructor(props) {
    super(props);

    this.View = View; // ESLint false positive
  }

  render() {
    return <this.View />;
  }
}

Regarding this regression, would you be the individual to collaborate with to assist in its resolution?

[ljharb]

@jwoo92 it's not a design pattern that i've ever seen, and it's actively harmful in that it will make your code slower for precisely zero benefit, and it's the reason you're running into a false positive. There's no benefit in fixing a false positive that nobody would ever run into (since I can't imagine anyone else writing code this way).

In other words, the best solution seems to be fixing your code to not do that.

[jwoo92]

@ljharb are you replying to my comment above, or is our timing a coincidence?

[ljharb]

I'm replying to your comment.

[jwoo92]

@ljharb Thank you for clarifying.

There appears to be a misunderstanding here. I'm open to discussing the design pattern further.

I have followed the pull request guidelines and provided supporting evidence. Could you please state your needs and the subsequent steps we should take to make headway?

[ljharb]

First I'm trying to understand why anyone would do this.X = X for any class X, not even React-specific - every class already has this.constructor === X for that scenario anyways. Second, I'm trying to understand why you'd do that in react, where it 100% guaranteed never has or will have a rendering impact.

If we can establish that the code that gets a false positive shouldn't exist in the first place then the proper path forward is for you to change your code, and we don't need to bother fixing it here.

Alternatively, if the use case is valid, then we should fix it here.

Can you help me understand why you would do <this.View> instead of just <View> from inside the class? They'll be === identical, so React (and JS) can't possibly tell the difference.