[Feature request] [eslint-plugin-react-hooks] Naming convention to indicate hook return value does not change (and helper hooks to ensure it)

[nathggns]

Apologies if this has already been discussed somewhere. I did have a look, but I couldn't find anything relevant.

Problem

Currently, useRef has a super power that react-hooks/exhaustive-deps will know not to complain about references to a variable using useRef when not included inside dependencies for useEffect, useMemo, and useCallback. That works because the return value from useRef is stable and can never change.

This is a really useful super power, and there are other cases where it might be useful.

Firstly, it is possible to use useMemo and useCallback in such a way that the return value will never change (by passing an empty dependencies array), but react-hooks/exhaustive-deps will still complain if these are not included in dependencies.

Example:

const immutableRef = useRef();
const immutableMemo = useMemo(() => Math.random(), []);
const immutableCallback = useCallback(() => {}, []);

useEffect(() => {
  immutableRef.current = 1;
  console.log(immutableMemo);
  immutableCallback();
}, []);

While it may be possible for the eslint plugin to "check" the dependency array of the useMemo and useCallback calls before deciding to trigger a warning, that still wouldn't work if the hook was called inside a custom hook. Additionally, useRef's super power also only works so long as the ref is created inside the same component/hook as the effect/memo/callback is created.

function useRefWrapped(initialValue) {
  return useRef(initialValue);
}

function App() {
  const immutableRef = useRefWrapped();

  useEffect(() => {
    immutableRef.current = 1;
  }, []);

  return null;
}

Proposal

The creation of useMemoImmutable and useCallbackImmutable hooks that are synonymous with useMemo and useCallback when called with an empty dependencies array. The names of these hooks are not important to the proposal, they can be whatever the team feels are most appropriate.

const useMemoImmutable = valueFactory => useMemo(valueFactory, []);
const useCallbackImmutable = callback => useCallback(callback, []);

Update: it's been mentioned to me that useMemo and useCallback does not actually guarantee that the value will never change. It's likely that the these hooks would have to be based on useRef instead

const useMemoImmutable = valueFactory => {
  const ref = useRef({ hasSet: false, value: null });
  
  if (!ref.current.hasSet) {
    ref.current.value = valueFactory();
    ref.current.hasSet = true;
  }
  
  return ref.current.value;
};
const useCallbackImmutable = callback => useRef(callback).current;

react-hooks/exhaustive-deps can then suggest that any variables that are assigned to a usage of these hooks has the suffix of Immutable. Again, the naming is not important and can be changed to whatever the team feels most appropriate.

It can also ensure that any hooks that return a variable with the suffix Immutable (or a direct return of useImmutableMemo or useImmutableCallback) also have the suffix in their name.

It could also warn if the Immutable suffix is used on a value the linter cannot be sure does not change.

This would allow the linter to "ignore" the use of any variable with the suffix immutable inside a use of useCallback, useMemo, and useEffect, even if these variables are not inside their dependency arrays.

This proposal would allow other hooks to "share" useRef's super power.

I know that a naming convention isn't the most ideal way of indicating things like this, but the React team has already accepted it's worthwhile to enforce one if the return on value is high enough (i.e, the use prefix for custom hooks).

[nathggns]

I can create a PR for an RFC for this proposal, I just wanted to ensure there wasn't already an alternative to this before doing so.

[theKashey]

The problem with react-hooks/exhaustive-deps and useEffect is that Effect don't have to be rerun on any of it "actual" dependencies change, but only on the right ones.

With the current approach the rule or force you to add all used variables, or disable rule completely. What about fixing especially this moment?

const ignoreDependecy = (a) => undefined;

useEffect(() => {
  immutableRef.current = 1;
  console.log(immutableMemo);
  immutableCallback();
}, [
  immutableRef,
  ignoreDependecy(immutableMemo), // yep, I know about this dep, but could we ignore it?
  ignoreDependecy(immutableCallback)
]);

Right now such pattern will generate two errors - the use of complex expression, and the missing dependency as well.
But how else one can indicate that some dependencies are really should not trigger the update, and still be "protected" from the "oh, I've forgot it again" accidents.

[nathggns]

@theKashey personally I’m not sure “ignoring dependencies that we know could change but we don’t care” is part of the story of “how do I tell react I know a variable can’t change in such a way that react can semi-enforce this and give us helpful hints based on this guarantee”, which is what this proposal attempts to address. These feel like separate issues.

[theKashey]

For me that's that same story:

• (your one) Hey React! You know - these variables are actually immutable, so could you ignore them?

Well, if they are immutable why not to add them to deps and not worry?

• (mine one) Hey? React! You know - I know that you should not be interested in these variables, so could you ignore them?

---

Actually the main question here - are you sure that you really have "immutable" variables and need any special treatment for them?

[nathggns]

We’ll have to agree to disagree about these being the same stories cause I really could not disagree more.

On the question of whether I know these values won’t change - the sample hooks above are based on useRef, which useEffect trusts never to change, so yeah, we can be pretty sure.

[theKashey]

But what are you really want to do and why you cannot just "accept" the fix from eslint rule, as long as adding those variables into useEffect will have no consequences for you - you will have nothing more that a "protection", which, if I am getting you correctly, is not needed.

However let's take the most immutable hook in the world, and apply HotModuleReplacement - the logic inside Refresh will persist ref and state, however will update any other hook - useCallback and useMemo included.
So, by fact, "immutable" hooks could be mutable, at least in dev mode. And don't forget that useMemo could "lose" it's value as well, so it's not very immutable by design.

[nathggns]

You’re missing the point. It’s possible to implement similar APIs to useMemo and useCallback when called with no dependencies using useRef (as shown in the sample implementations above), and so hot module replacement is not a concern. It would still be as immutable as useRef.

The benefits of not including them in the dependency array is less client side dependency checking on every render. And we shouldn’t need to dismiss eslint to get that benefit. Why bother implementing it at all for useRef if it’s not considered a worthwhile quality of life improvement?

[gaearon]

Hi, thanks for your suggestion. RFCs should be submitted as pull requests, not issues. I will close this issue but feel free to resubmit in the PR format.