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

Feature Request: Mutable Mode for Eslint Hooks PLugin #20064

Closed
samcooke98 opened this issue Oct 20, 2020 · 2 comments
Closed

Feature Request: Mutable Mode for Eslint Hooks PLugin #20064

samcooke98 opened this issue Oct 20, 2020 · 2 comments

Comments

@samcooke98
Copy link

samcooke98 commented Oct 20, 2020

At the company I work at, we currently use MobX to manage the majority of our state. We recently noticed that the linting plugin can lead our engineers astray.

This can happen because the plugin reports a value as an unnecessary dependency thinking that mutating them wouldn't cause a re-render. However due to MobX, it's possible the value is actually an observable, and mutations could trigger a re-render.

This can happen when you have some code like:

const store = mobx.observable({
  counter: 0
});

const MyCounter = observer(({ name }) => {
  React.useEffect(() => {
    console.log("Effect: " + store.counter);
  }, [store.counter]);
//React Hook React.useEffect has an unnecessary dependency: 'store.counter'. Either exclude it or remove the dependency array. Outer scope values like 'store.counter' aren't valid dependencies because mutating them doesn't re-render the component. (react-hooks/exhaustive-deps)eslint


  return <div>Count: {store.counter} </div>;
})

https://codesandbox.io/s/mobx-hooks-m6iwp

My idea is to introduce an optional configuration for eslint-hooks-plugin that enables a "mutable" mode, which makes the plugin treat any value accessed in an effect as potentially unsafe and changeable.

I suppose it wouldn't be autofixable anymore, as an engineer would need to know what is\isn't observable.

I realize a solution here might be for somebody to fork the eslint-hooks-plugin and build this feature, however I wanted to open an issue to see if an upstreamed "mutable" mode would be something useful.


My understanding is that in Concurrent Mode, this will need to change anyway to use useMutableSource, so I'm not sure if it's something the React team want to address, but thought it'd be worth creating an issue to discuss.

@samcooke98 samcooke98 changed the title Feature Request: Mutable Mode for Feature Request: Mutable Mode for Eslint Hooks PLugin Oct 20, 2020
@gaearon
Copy link
Collaborator

gaearon commented Oct 20, 2020

Yeah, this seems out of scope for the canonical plugin. Re: future React features, many of them are not possible to implement without immutability. MobX may come up with some clever way for some of them to mostly work (except when you use it as a replacement for local state), but this is probably a discussion they'll have more context on. Overall we've been pretty explicit that immutability is preferred in React wherever possible.

@gaearon gaearon closed this as completed Oct 20, 2020
@samcooke98
Copy link
Author

Thanks for the speedy reply!

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

No branches or pull requests

2 participants