Currently behaviour of useEffect hook is defined by the last parameter. It is either not defined at all, empty array, or enumerates the values.
I believe this adds a bit of cognitive load to the developer. The intention is not clear until I look at the end of the function. Second, the difference between empty array [] and not defined array is not that drastic. Furthermore, natural flow is like that: define values, and when any of them changes, then apply effect, so I assume specifying the list of tracked values first is better.
I personally think, it would be nice to have something like this:
import {useEffect} from "react";
/**
* Applies effect on every render of the component.
*
* @param {React.EffectCallback} effect - Imperative function that can return a cleanup function.
*/
function useEffectAlways(effect){
useEffect(effect);
}
/**
* Applies effect only if one of the values of deps array changes.
*
* @param {any[]} deps - Array of values which are tracked by React to determine whether effect should be applied.
* @param {React.EffectCallback} effect - Imperative function that can return a cleanup function.
*/
/*
DECISION: deps intentionally goes first, because it resembles the flow: if 'deps' changed then apply 'effect'.
and it is much easier to spot the list of dependencies, when it goes first.
Compare:
useEffectOnChanges([a, b, c], () => {
...
...
...
});
and
useEffectOnChanges(() => {
...
...
...
}, [a, d, c]);
*/
function useEffectOnChanges(deps, effect){
if (!deps || deps.length === 0){
throw new Error("Either provide non empty deps array or call useEffectAlways/useEffectOnce.");
}
useEffect(effect, deps);
}
// defining a constant for empty array allows us to avoid memory allocation on every call.
const __emptyArray = [];
/**
* Applies effect only once on the first render of the component. Equivalent to componentDidMount lifecycle method.
*
* @param {React.EffectCallback} effect - Imperative function that can return a cleanup function.
*/
function useEffectOnce(effect){
useEffect(effect, __emptyArray);
}
export { useEffectAlways, useEffectOnChanges, useEffectOnce };
The dependency mechanism is the same for all hooks with dependencies, not just useEffect. If this change is to be accepted, it should also apply to useMemo et al.
I'm not a team member, but I feel safe to say that minimal API surface is part of the design principals of React.
And you have outright demonstrated in the code above how lower-level API can be spread into more elaborate higher-level API, with all the pros and cons attached.
Defining higher-level API typically involves more discussion, which is natively opinionated. For instance, why shouldn't React just provide:
export const ALWAYS = undefined;
export const ONCE = [];
And simply let you provide these to the deps in the useEffect hook?
You should also consider things beyond your specifics. There are so many possible good additions to the API - the two that immediately pop to mind are useForkRef and useForceUpdate. But if React is to take on all these (undoubtably reasonable in their own right) suggestions, the API will become many-fold, which will make it harder to maintain and document.
Honestly, having no useForkRef is just an example how even essential utilities don't make it to the API. It's the land of bare essentials.
@Izhaki I agree with you, if it's React philosophy to keep API surface as small as possible, then this suggestion simply goes against it.
However, if you have dense code in the effect:
useEffet(() => {
...
}, ONCE);
useEffect(() => {
...
}, ALWAYS);
Scanning just left part with useEffect won't immediately reveal the intent due to the distance between useEffect and ONCE|ALWAYS.
@voroninp I agree - but it's just part of much larger pros/cons discussion really.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!
Most helpful comment
The dependency mechanism is the same for all hooks with dependencies, not just
useEffect. If this change is to be accepted, it should also apply touseMemoet al.I'm not a team member, but I feel safe to say that minimal API surface is part of the design principals of React.
And you have outright demonstrated in the code above how lower-level API can be spread into more elaborate higher-level API, with all the pros and cons attached.
Defining higher-level API typically involves more discussion, which is natively opinionated. For instance, why shouldn't React just provide:
And simply let you provide these to the deps in the
useEffecthook?You should also consider things beyond your specifics. There are so many possible good additions to the API - the two that immediately pop to mind are
useForkRefanduseForceUpdate. But if React is to take on all these (undoubtably reasonable in their own right) suggestions, the API will become many-fold, which will make it harder to maintain and document.Honestly, having no
useForkRefis just an example how even essential utilities don't make it to the API. It's the land of bare essentials.