rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

RFC: Declared Dependencies

Open dantman opened this issue 5 years ago • 4 comments

This RFC is for the proposal/discussion of a potential common convention.

I regularly see devs (myself, library authors, etc) disabling the react-hooks/exhaustive-deps rule on specific hooks because they want to explicitly declare the conditions which one of the deps actually changes in a meaningful way under. There are also some scenarios where you still want the value to change but do not want effects to re-run.

I would like to create alternative solutions to disabling the lint rule for an entire deps list just to declare the change conditions of one dep.

View formatted RFC

dantman avatar Oct 17 '20 23:10 dantman

I think this proposal breaks one of react's base principle:

useRef returns a mutable ref object whose .current property is initialized to the passed argument (initialValue). The returned object will persist for the full lifetime of the component.

And in your RFC, you're suggesting to return "refs" but that are recreated when one (or more) of the dep changes. IMO as it breaks this base principle, it shouldn't be called a ref.

Ayc0 avatar Aug 01 '21 09:08 Ayc0

I think this proposal breaks one of react's base principle:

useRef returns a mutable ref object whose .current property is initialized to the passed argument (initialValue). The returned object will persist for the full lifetime of the component.

And in your RFC, you're suggesting to return "refs" but that are recreated when one (or more) of the dep changes. IMO as it breaks this base principle, it shouldn't be called a ref.

The principle you are referring to only applies to the useRef hook. It is simply a declaration of that specific hook's behaviour, not a principle that refs follow. It does not apply to other object refs like those created by createRef.

dantman avatar Dec 10 '21 21:12 dantman

The root cause is the "all or nothing" presence of eslint rule. It's or working, complaining about wrong dependencies, or not working at all. What is the root cause of it? How one can solve it in a better way:

  • of one uses dependency, which is not used inside a function, and there are certain use cases when it's usefull, I want only one dep to be explicitly added as an extra dep.
  • of one is not declaring used dep, I want to mark it in a special way
    • in some cases React known about stability of used variables (like setState). It would be nice to be mark some user-space variables in the same way

While the result might contain twice as much comments, it can lead to a better understand of the result, and reasons behind for both for eslint rule and the end developer.

config = React.useMemo(
  () => configToObject(config),
  [JSON.stringify(config)]
)

// ⬇️
config = React.useMemo(
   // react-ignore-usage comment: do not react on variable change
  () => configToObject(config),
  [
   // react-additional-dependency comment: we use string representation of config to make result less reactive to some changes
  JSON.stringify(config)
  ]
)

// ⬇️
/**
 * @stable this variable is "stable" and don't have to be added into any dependency
 */
const config = ...
config = React.useMemo(
  () => configToObject(config), // config is "stable"
  [
   // react-additional-dependency
  JSON.stringify(config)
  ]
)

theKashey avatar Dec 12 '21 00:12 theKashey

@theKashey Sure, that is an alternative I listed (no. 3).

I will note though. One of the original idea for hooks was supposed to be that they could eventually be automatic. i.e. Instead of an eslint rule you have to run, you write code without the deps array at all and the JS compiler or bundler would automatically insert the deps.

Although it looks like instead of going that route of only having auto deps for the useMemo/etc... recently (React Forget) it looks like auto-memoizing everything might be the direction things go.

Either way. The one thing to note is that adding comments to the deps array to declare dependencies in some way gets in the way of those auto-deps/memoization plans. But the RFC's way of declaring special cases to deps with a hook instead of comments does play well with auto-deps and auto-memoization.


With React Forget and this RFC the axios-hooks library example would just be:

const configRef = useDependencyDeclaration(config, [JSON.stringify(config)]);
config = configToObject(configRef.current);

Although to fit in with React Forget it needs a completely different name and maybe the configRef could be ditched. The fundamental idea here is using a hook to declare "this is the condition under which config actually changes".

In fact for React Forget you could probably ditch the ref entirely and use the hook to declare the "hasConfigChanged" condition the React Forget compiler uses.

useEquality(config, isEqual)
config = configToObject(config);

Though IMHO this really just suggests that we shouldn't be focusing on memoization for this RFC. Because even if all this is fixed for memoization other examples that run in useEffect, etc... like the auth example are not covered by this and may still need something ref based.

dantman avatar Dec 12 '21 02:12 dantman