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

react/no-direct-mutation-state doesn't catch all cases #1418

Open
yantakus opened this issue Sep 8, 2017 · 8 comments
Open

react/no-direct-mutation-state doesn't catch all cases #1418

yantakus opened this issue Sep 8, 2017 · 8 comments

Comments

@yantakus
Copy link

yantakus commented Sep 8, 2017

  const { model } = this.state;
  model.rating = rate;

In this case eslint won't show any errors.

@jseminck
Copy link
Contributor

jseminck commented Sep 8, 2017

I think we figured this out yesterday. Somebody created a PR for not allowing mutation for this.props and added support for this case. Once that PR is finished we'll try to move all the logic over to no-direct-mutation-state rule as well.

#1416

@meldafert
Copy link

is there any news on this? or alternatives that i could use?

@kangax
Copy link

kangax commented Apr 24, 2019

@meldafert I just found out the other day about this rule not catching all cases. Ended up marking state immutable via Flow instead, which is perfect for this job:

// ...
type State = $ReadOnly<{| ... |}>
class Smth extends Component<Props, State> {
  // ...
}

@meldafert
Copy link

@kangax thank you, this is exactly what i needed. (turns out typescript has this too).

@kangax
Copy link

kangax commented May 3, 2019

@meldafert lint rule pending gajus/eslint-plugin-flowtype#400

@maniator
Copy link

maniator commented May 3, 2019

@kangax thank you, this is exactly what i needed. (turns out typescript has this too).

@meldafert TypeScript already disallows changing props as they are marked as readonly through typescript typing already (the app will not build if you try to mutate props)

@meldafert
Copy link

meldafert commented May 3, 2019

@meldafert TypeScript already disallows changing props as they are marked as readonly through typescript typing already (the app will not build if you try to mutate props)

@maniator True, but only for mutating this.props itself, but not for eg. this.props.list.push(), which is what i'm trying to catch.
(if i declare the prop as list: Readonly<Item[]>, it does prevent me from using push())

@maniator
Copy link

maniator commented May 3, 2019

@meldafert ahh, so it does not do a deap check.

You can probably also do (in ts)

readonly list: Item[],

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

6 participants