Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "ctors" option to @no-direct-mutation-state" rule #1355

Conversation

doochik
Copy link
Contributor

@doochik doochik commented Aug 10, 2017

This option allow to check specified methods like constructor.

Motivation
I have BaseComponent in my project and use it in this way:

class BaseComponent extends React.PureComponent {
   constructor(props, context) {
        try {
            this._init(props, context)
        } catch(e) {
           // log error
        }
   }
}

class MyComponent extends BaseComponent {
    _init() {
        this.state = {
            foo: 'bar'
        };
    }
}

_init method is actual constructor for MyComponent and I want to check it like constructor with the "ctors" options

"react/no-direct-mutation-state": [<enabled>, {
  ctors: ['_init']
}]

This option allow to check specified methods like constructor.
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a -1 on this; React components should not use inheritance to share code. It's discouraged by the React team, it's not idiomatic in the community, and it's not even the best way to share code in CS overall.

```js
...
"react/no-direct-mutation-state": [<enabled>, {
ctors: <ctors>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please spell out "constructors" - letters don't cost money, but confusing abbreviations can.

@doochik
Copy link
Contributor Author

doochik commented Aug 10, 2017

@ljharb Let my clarify puprose of BaseComponent in my project. This is very tiny class which wraps in try-catch some lifecycle methods like constructor, render, etc. It has no functionality.
Basically it has same ideas like componentDidCatch in react@16.

At this time I can't update to esling-pligin-react@7.2.0 because you've fixed #832 :)

@ljharb
Copy link
Member

ljharb commented Aug 10, 2017

I'd recommend doing that with a babel transform to change React.Component to your base component, and definitely not putting an unidiomatic pattern in your code.

Alternatively, you could also define componentDidCatch when needed, and you could use that babel transform only on components where that was defined.

@doochik
Copy link
Contributor Author

doochik commented Aug 10, 2017

Ok, I understand your point

@doochik doochik closed this Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants