react: useCallback() invalidates too often in practice

This is related to https://github.com/facebook/react/issues/14092, https://github.com/facebook/react/issues/14066, https://github.com/reactjs/rfcs/issues/83, and some other issues.

The problem is that we often want to avoid invalidating a callback (e.g. to preserve shallow equality below or to avoid re-subscriptions in the effects). But if it depends on props or state, it’s likely it’ll invalidate too often. See https://github.com/facebook/react/issues/14092#issuecomment-435907249 for current workarounds.

useReducer doesn’t suffer from this because the reducer is evaluated directly in the render phase. @sebmarkbage had an idea about giving useCallback similar semantics but it’ll likely require complex implementation work. Seems like we’d have to do something like this though.

I’m filing this just to acknowledge the issue exists, and to track further work on this.

About this issue

  • Original URL
  • State: open
  • Created 6 years ago
  • Reactions: 170
  • Comments: 115 (24 by maintainers)

Commits related to this issue

Most upvoted comments

@sokra An alternate would be:

function useEventCallback(fn) {
  let ref = useRef();
  useLayoutEffect(() => {
    ref.current = fn;
  });
  return useCallback(() => (0, ref.current)(), []);
}

This doesn’t require the args like yours has. But again, you can’t call this in the render phase and the use of mutation is dicey for concurrent.

We’ve submitted a proposal to solve this. Would appreciate your input!

https://github.com/reactjs/rfcs/pull/220

the rfc sounds nice. In almost any react app, I end up using a useLatest hook to ref-ify non-reactive values I only need in response to some event. I wonder if all usages of useLatest are related to event handling and hence covered by the proposed hook for creating event handlers.

On a different note, the name useEvent confused me in the beginning and I see many people are also talking about it. 🟢 useState => returns state (and setter) 🟢 useRef => returns ref 🟢 useCallback => returns callback 🟢 useMemo => returns memorized value 🔴 useEvent => returns event handler

I get that having a short name is nice, but confusing “event” and “event handler” is a much more considerable drawback IMO. I think useEventHandler or useHandler (if having a short name is deemed very important) are better choices.

Stuff like this is why Vue or Angular are still alive.

On Sat, Jan 28, 2023, 7:57 AM ZZYZX @.***> wrote:

I bumped into this thread while already having a solution and trying to understand if I’m reinventing the wheel 😃

Looks like I’m not, as my solution to the problem is different than existing options:

export default function useConstCallback<T extends CallableFunction>(func: T, deps?: React.DependencyList): T { const ref = useRef<T>(func)

useMemo(() => { ref.current = func // eslint-disable-next-line react-hooks/exhaustive-deps }, deps)

return useCallback((…params) => ref.current(…params), []) as unknown as T}

The main difference compared to useEvent above is that I’m abusing useMemo() (which runs instantly during hook call) instead of using useLayoutEffect() or useEffect() which calls only after the render;

Considering useCallback(x, deps) is functionally the same as useMemo(() => x, deps) (see implementation in React: https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L2242), this works exactly like useCallback() would, with the exception of having a constant reference.

— Reply to this email directly, view it on GitHub https://github.com/facebook/react/issues/14099#issuecomment-1407393353, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACR376PUOJWLDN3ZRZEVPF3WUUJR7ANCNFSM4GB2QADA . You are receiving this because you were mentioned.Message ID: @.***>

The useEvent proposal definitely goes in the right direction, as the misuse of useCallback is something we’ve all seen countless times across teams and organizations.

I do want to express some strong concerns around the naming as useEvent could potentially be very confusing for less experienced engineers or those who are coming to React from other frameworks. The dangerous naming correlation to DOM events makes it quite misleading at first glance and seems to hide its true purpose.

I much prefer useStableCallback as someone else suggested, or useHandler as I feel it makes it easier to document and explain its usage moving forward.

I’ve look at sources of useReducer. But I can’t understand how it is related to useCallback

useCallback lets you memoize the callback to avoid a different function being passed down every time. But you have to specify everything it depends on in the second array argument. If it’s something from props or state, your callback might get invalidated too often.

useReducer doesn’t suffer from this issue. The dispatch function it gives you will stay the same between re-renders even if the reducer itself closes over props and state. This works because the reducer runs during the next render (and thus has natural ability to read props and state). It would be nice if useCallback could also do something like this but it’s not clear how.

What issues has “mutation of ref during rendering”? Can you explain me in brief?

In concurrent mode (not yet released), it would “remember” the last rendered version, which isn’t great if we render different work-in-progress priorities. So it’s not “async safe”.

@sophiebits That’s clever and would have none of the problems with args, etc. It even doesn’t require a dependencies list.

One nitpick: return useCallback((...args) => (0, ref.current)(...args), []); to pass along i. e. event argument.

Does that have any drawbacks besides unnecessary effects?

Late to the party here, but after scanning the above thread- there is another potential problem in the useEventCallback example and other example uses of useLayoutEffect above:

function Example({ callback }) {
  const stableCallback = useEventCallback(callback);
  return <Child callback={stableCallback} />;
}

function Child({ callback }) {
  useLayoutEffect(() => {
    // Callback has not yet been updated!
    // This is because child effects are run before parent effects,
    // So the callback ref has not yet been updated (and still points to the old value).
    callback();
  }, [callback]);
}

It’s very, very different behaviour

With a ES6 class I can pass onChange={this.handleChange} to the child component and it does not redraw every time (because it sends the same handleChange method every time)

It’s frustrating that dispatch works independent of state (I mean the dispatch method does not change even if the underlying state does) but useCallback does not (if your callback needs to do something with state that is)

I see a lot of code where the whole form redraws when the use types into a field

A lot of devs don’t understand this until users complain ‘it’s slow’

Would be nice if second argument of useCallback was injected as dependencies to callback function.

  function useCallback(cb, deps) => {
    lastDeps = deps; // save current deps and cb deep in somewhere
    lastCb = cb;

    if (!cached) {
      cached = (...args) => lastCb(...lastDeps)(...args); // memoize that forevere
    }

    return cached; // never invalidates
  }

  const myCallback = useCallback(
    (state, props) => (a, b) => a + b + state + props,
    [state, props]
  );

  myCallback(1, 2)

Tbh this is making me super skeptical of concurrent mode, given that function memoization is the number one tedious thing I deal with daily in React since hooks were introduced (everything else about hooks being fantastic) and concurrent mode wouldn’t benefit me personally right now AFAIK, yet sounds quite dangerous to use.

Gentlemen: useGranularCallback from npm i granular-hooks BTW, I use their useGranularEffect() a lot because I don’t want useEffect style triggering when only certain vars should trigger updates. It takes two lists of arguments, primary and secondary deps as arrays. primary deps trigger calling the hook or changing the callback, secondaries don’t.

Workarounds above (especially relying more on useReducer) seem sufficient for most cases. There are cases when they’re not, but we’ll likely revisit this again in a few months.

I recently made a duplicate issue and was asked to check this one. What I proposed there was very similar to @sophiebits’ approach, but still looks a bit simpler to me:

function useStatic(cb) {
  const callback = useRef(cb)
  callback.current = cb

  const mem = useRef((...args) => callback.current(...args))
  return mem.current
  
  // We could, of course still, use the useCallback hook instead of a second reference.
  // return useCallback((...args) => callback.current(...args), [])
  // Although I think that the one above is better since it avoids the need to compare anything at all.
}

This way it is guaranteed to update where the hook is called since it does not directly use any side effect and instead it only updates a reference. It seems to me that it should be callable during the render phase and should not be dicey with concurrent mode (unless references don’t meet these two conditions). Wouldn’t this approach be a little better or am I missing something?

One more argument against the “useEvent” naming. The following statement derived from the RFC doesn’t make sense in English:

[The RFC states that] the event created by useEvent will throw if called

How can an event throw? How can an event be called? An event is something that happens. Sometimes we say “an event” as a shorthand to “an event object” that not only mentions what happened, but also describes the details about what happened.

But it feels unintuitive to use “an event” as a shorthand for “an event handler function” that processes what happened during an event.

While writing the above, I think I understood one way of thinking where these can be equivalent: if we treat an event as something not instantaneous that has duration (which is not how events are usually treated in software engineering), and then treat the side effects as an unseparable part of an event, then the function that processes the event is the part of the event, hence “using an event” makes some sense.

It ensures that the receiver (this value) of the function isn’t ref.

Concurrent mode can produce two different representations of a component (the first one is that commited to the dom and the second one is that in memory). This representations should behaves accordingly with their props and state.

useEventCallback by @sophiebits mutates ref.current after all dom mutations is completed, so the current (in-memory) component can’t use the newest callback until the commit is done.

@muqg proposal mutate the callback on each render, so the commited component will lose the reference to the old callback.

The point of my proposal in the passing a separated callback reference, that will changes in commit phase, to the dom, while the in-memory (not commited) representation of a component can use the latest version of that callback.

const handler = () => {/* do something*/};
const callback = useCallback(handler);

In this case, you wont pass down the handler to other components because it always changes. You will pass the callback, but will face again the concurrent mode problem.

@muqg In concurrent mode, last render doesn’t necessarily mean “latest committed state”. So a low-priority render with new props or state would overwrite a reference used by current event handler.

const useCallback = (fn, args) => {
  const callback = useMemo(() => {
    if (__DEV__) {
      if (fn.length !== args.length) warning(...);
    }
    const callback = () => fn(...callback.args);
    return callback;
  });
  useEffect(() => callback.args = args, [args]);
  return callback;
}

Drawbacks:

It’s easy to forget the arguments list, which would result in hard to find bugs. In dev mode it would make sense to check fn.length for the correct length.

It’s still possible to forget arguments in the dependencies array, but this applies to other hooks too.

Definitely agree with @floroz regarding the naming and would prefer something like useStableCallback.

Had a use case for @sophiebits proposed useEventCallback (https://github.com/facebook/react/issues/14099#issuecomment-440013892). The problem with useLayoutEffect is that it issues warnings when server side rendering. You can trick React by switching to useEffect based on the environment (typeof window) but this won’t work if the SSR API is called in the browser (with-apollo).

Wouldn’t useImperativeHandle work as well?

Edit: WARNING This means that the callback cannot be called safely in the effect cleanup phase. When unmounting the ref will be nulled before the cleanup phase.

function useEventCallback(fn) {
  let ref = useRef();
-  useLayoutEffect(() => {
+  useImperativeHandle(ref, () => {
-    ref.current = fn;
+    return fn;
  });
  return useCallback(() => (0, ref.current)(), []);
}

Workarounds above (especially relying more on useReducer) seem sufficient for most cases. There are cases when they’re not, but we’ll likely revisit this again in a few months.

@gaearon then do you plan to release useCallback with the first release of the hooks? After all it is confusing (current issue) and it is not a primitive hook (is equivalent to useMemo(() => fn, inputs))


@carlosagsmendes you can probably use the workaround useEventCallback from above. You can see your example with it here

I put the useEventCallback (with couple changes) here again:

function useEventCallback(fn) {
  let ref = useRef();
  useLayoutEffect(() => {
    ref.current = fn;
  });
  return useMemo(() => (...args) => (0, ref.current)(...args), []);
}

@sokra With this you will not be able to access to state and props updates inside a callback.

I think with @sophiebits’ approach this will work. The latest function is always copied into the ref and only a trampoline function is returned. This will make sure that the latest function is called, which has the latest state in context.

to be honest, I’m not sure why the implementation of the useEvent has to be so complicated, why not just this?

function useEvent(callback) {
  const callbackRef = useRef(callback)
  callbackRef.current = callback
  
  return useCallback((...args) => {
    return callbackRef.current(...args)
  }, [])
}

below is the typescript version

function useEvent<ArgumentsType extends unknown[], ReturnType>(
  callback: (...args: ArgumentsType) => ReturnType,
) {
  const callbackRef = useRef(callback)
  callbackRef.current = callback

  return useCallback((...args: ArgumentsType): ReturnType => {
    return callbackRef.current(...args)
  }, [])
}

with this implementation, there is no such thing as when will the current is switched, because it will be switched every single time, and it can be used in rendering as well

@nikparo Nothing to be sorry for, I think that’s important and that proves my point. We use programming languages and naming conventions to communicate, but this particular (useEvent) naming introduces a lot of space for ambiguity. By the way, “event function” also doesn’t make a lot of sense in the context of “an event” as a marker for a point in time when something happened instantaneously.

EDIT: “An event handler function” (or, more grammatically correct “an event-handling function”) from my point of view makes more sense than “an event function” because the main keyword here is “handler”/“handling”, a function that that “handles” the event. English doesn’t make this easy because whether a word is a noun or an adjective depends on its position in a sequence of words.

@sokra With this you will not be able to access to state and props updates inside a callback.

const [state, setState] = useState(0);

const handleClick = useCallback((event) => {
  console.log(state); // always 0
  setState(s => s + 1);
});

return <button onClick={handleClick} />

So dependencies are required.

function useCallback(fn, deps) {
  const fnRef = useRef(fn);
  const depsRef = useRef(deps);

  useLayoutEffect(() => {
    fnRef.current = fn;
    depsRef.current = deps;
  });

  return useCallback((...args) => (0, ref.current)(...depsRef.current)(...args), []);
}
cons handleClick = useCallback(
  (state) => event => console.log(state), // up-to-date
  [state]
);

Given that the ‘useEvent’ hook has gone the way of the doodoo what are the plans to solve this issue?

I was a big fan of the ‘useEvent’ hook - and end up having to copy my own implementation in every other project (something like use-event-callback or the one @HansBrende mentioned in that issue).

To bring this issue to a final close why not encourage a simply “hook recipe” on the documentation?

Something like:

## Avoiding Invalidated Memoized Objects

Slap your function into a ref that's updated through useLayoutEffect.

<recipe here>

Caveats:

bla bla SSR bla bla

I understand that the team is currently investigating other options for the ‘rendering optimizations are cumbersome to type’ problem- but us every day developers don’t have access to these tools.

Indeed, should they be released we might not be able to use them because we’re stuck on an old version of React for some reason or another.

function getStateAsync(setter) {
  return new Promise(resolve => {
     setter(prevValue => {
        resolve(prevValue);
        return prevValue; // The value has not changed.
     });
  })
}

function Chat() {
  const [text, setText] = useState('');

  const onClick = useCallback(async () => {
    sendMessage(await getStateAsync(setText));
  }, []); // There is no dependency array.

  return <SendButton onClick={onClick} />;
}

This gist is written in typescript and allows you to get several states at once.

In this case the “event” is the inner curried part. In general I would not recommend currying React event handlers. It’s a pretty confusing pattern to read and deal with.

Can we have documented rules or principles to note this recommendation? Users can’t avoid getting trapped by using useEvent in ways out of the When useEvent should not be used chapter listed.

The RFC states that the event function created by useEvent will throw if called during render, which is what you did with your curried handler.

The useEvent proposal definitely goes in the right direction, as the misuse of useCallback is something we’ve all seen countless times across teams and organizations.

I do want to express some strong concerns around the naming as useEvent could potentially be very confusing for less experienced engineers or those who are coming to React from other frameworks. The dangerous naming correlation to DOM events makes it quite misleading at first glance and seems to hide its true purpose.

I much prefer useStableCallback as someone else suggested, or useHandler as I feel it makes it easier to document and explain its usage moving forward.

As a long time user of react, I also agree. useEvent makes me assume we are using an emitter and i would avoid handler for the same reason as it implies action in response to an emitted event. I like your suggestion of useStableCallback over useEvent

@zheeeng I’m not sure what’s the benefit of even using useCallback in your case. handleChangeValueByIndex(i) will always return a new function even though the handleChangeValueByIndex itself is memoized.

Having said that, there definitely are cases where useCallback should still be used even after introducing useEvent. One example is memoized components that use render props:

const renderItem = useCallback(i => <Item data={items[i]} />, [items])
return <List renderItem={renderItem} count={items.length} />

Here, if List is memoized, renderItem reference has to change along with items, otherwise the changes to items won’t be reflected on screen

I wrote a hook that achieves exactly the same some time ago.

Would you please have a look, @gaearon? It might be every so slightly more efficient than yours, as it uses two hooks rather than three.

If you’re interested, I also tried to start a discussion here.

/**
 * Returns a memoized version of your function that maintains a stable reference, but
 * also can read the latest scope (props and state) of the component in which it is used.
 */
function useStableCallback<Args extends any[], T>(
    callback: (...args: Args[]) => T
) {
    const callbackContainer = useRef(callback)
    callbackContainer.current = callback
    return useCallback(
        (...args: Args) => callbackContainer.current(...args),
        [callbackContainer]
    )
}

Adding more React specific concepts like useEvent seem troublesome on a first glance considering it’s more of Reactisms that kinda fight with the underlaying JS language principles. That being said, I don’t see a better solution for this specific issue at the moment, RFC seems very well thought through.

FYI: My snippet of useImperativeHandle solution https://github.com/facebook/react/issues/14099#issuecomment-569044797 is like this (it is TypeScript, remove type annotations for JS):

// Better useCallback() which always returns the same (wrapped) function reference and does not require deps array.
// Use this when the callback requires to be same ref across rendering (for performance) but parent could pass a callback without useCallback().
// useEventCallback(): https://github.com/facebook/react/issues/14099#issuecomment-499781277
// WARNING: Returned callback should not be called from useLayoutEffect(). https://github.com/facebook/react/issues/14099#issuecomment-569044797
export function useStableCallback<T extends (...args: any[]) => any>(fn: T): T {
  const ref = useRef<T>()
  useImperativeHandle(ref, () => fn) // Assign fn to ref.current (currentFunc) in async-safe way

  return useRef(((...args: any[]) => {
    const currentFunc = ref.current
    if (!currentFunc) {
      throw new Error('Callback retrieved from useStableCallback() cannot be called from useLayoutEffect().')
    }
    return currentFunc(...args)
  }) as T).current
}

That doesn’t sound right. Our docs list examples of using the ref for non-React values. The key is when to update the .current property. React updates this property during the “commit phase” (when effects are run) and we suggest you do the same1 to avoid problems like the one I mentioned above.

1 The one exception to this is using a ref to lazily initialize a value. This is only safe because it’s idempotent.

@Volune

I personally don’t see any benefits of the current useCallback implementation over the proposed useEventCallback

Current useCallback is quite helpful with render props pattern. One example is List from react-virtualized. It’s a PureComponent, so you can pass a memoized rowRenderer function to prevent rerenders. But if your rowRenderer doesn’t invalidate when its deps change (e.g. if it’s an instance property on a class component), you have to pass all those deps as List props to trigger rerenders when they’re actually needed.

Example on codesandbox: https://codesandbox.io/embed/staging-snow-sgh7v Note how I have to pass an extra value property to List to fix the class example while useCallback works as is because it invalidates whenever it would produce a different output.

@muqg @strayiker BTW this is why there’s no much sense in trying to make useEventCallback work in the render phase: If a child uses your callback to render something, you most likely want it to invalidate as often as its deps change. Also, this is why useEventCallback is a really good name. Basically, it says “only use me for event handlers”.

@vzaidman

The solution by @sophiebits does the same without a need for stating any deps at all. The callback from ref.current always has a fresh closure. So the usage looks like this

const handleClick= useEventCallback(() => {
  // do anything with props, state, or any other local variables
})

What issues has “mutation of ref during rendering”? Can you explain me in brief?

In concurrent mode (not yet released), it would “remember” the last rendered version, which isn’t great if we render different work-in-progress priorities. So it’s not “async safe”.

Aren’t class components also exposed to this? Seems React does the same thing to Class Components (mutating props in render phase)

https://github.com/facebook/react/blob/4c5698400f04bbc6d0b4bd766b0993d0bcb37609/packages/react-reconciler/src/ReactFiberClassComponent.js#L1042-L1045

also seems the committed and work-in-progress fibers share the same class instance

https://github.com/facebook/react/blob/4c5698400f04bbc6d0b4bd766b0993d0bcb37609/packages/react-reconciler/src/ReactFiber.js#L413

If React assigns pending props to instance.props in the render phase but the render work is interrupted for some reason, this.props will reflect the props of the WIP (outside of render phase) since both share the same instance. If an event handler fires in between and accesses this.props it’ll see the WIP props while it should rather seethe committed props.

I assume the solution is to switch this.props between phases, maybe this is already done but I couldn’t trace it in the sources so just wanted to make sure this is handled.

@Bazzer588 The general recommendation is to useReducer.

That said, your particular example can be solved like this but it is pretty unusual to not depend on any props so this doesn’t always work:

    const onChangeField = useCallback((name, value) => {
        changeData(oldData => ({ ...oldData, [name]: value }));
    }, []);

@sebmarkbage I tried the approach above of using the oldData argument, but it didn’t help me to avoid re-renders of the children components (that’s why I was using useCallback)

So I tried using useReducer and use dispatch inside the useCallback but all the children are still re-rendering.

Moreover, I’m passing the state to a callback after changing it, and it is always the previous state (off by one).

Can someone take a look and let me know what the recommended approach is?

Thanks in advance!

How/why is it that variables are dereferenced within useEffect?

Whether or not the effect is called again based on changes to state/reducer/etc (useEffect’s second param), shouldn’t have any implication on those variable’s references within useEffect, right?

This behavior seems unexpected and having to leverage “escape hatches” just feels broken to me.

I don’t think there is a point in saving the “current”, if you want to call it during rendering, just save it in advance out of the hook:

const handler = () => {/* do something*/};
const callback = useCallback(handler);
// I can call the current:
handler();

I personally don’t see any benefits of the current useCallback implementation over the proposed useEventCallback, will it become the new implementation? Also, can it warn when the callback is called during render in development mode?

@Moranilt

The main idea is to memoize a callback, which is passed to another component. In your example you’re trying to memoize a callback, which is passed into <input />. But there is no point to do it, cause, <input /> does not have any instruments to use that memoized callback.

So, in your example you could memoize set*** callback. But in your case they are memoized already)

But, I have a fix for your example:

type InputProps = {
  name: string;
  value: string;
  setValue: (value: string) => void;
  onClick: (value: string) => void;
};

// You have to use memo() here, to make memoization work
const Input = memo<InputPropsA>(({ name, value, setValue, onClick }) => {
  // You do not need any memoization here for onChange or onClick handler
  return <input name={name} onChange={(e) => e.taget.value} value={value} onClick={() => onClick(value)} />;
});

const Form = () => {
  const [firstname, setFirstname] = useState('');
  const [lastname, setLastname] = useState('');
  const [middlename, setMiddlename] = useState('');
  
  // And this is a correct memoization, cause onInputClick is passed into Input
  const onInputClick = useEvent((value: string) => {
    console.log(`value is: ${value}`);
  });

  return (
    <form>
      <Input name="firstname" setValue={setFirstname} value={firstname} onClick={onInputClick} />
      <Input name="lastname" setValue={setLastname} value={lastname} onClick={onInputClick} />
      <Input name="middlename" setValue={setMiddlename} value={middlename} onClick={onInputClick} />
    </form>
  );
};

Right now, <Input /> will be rerendered in case of the value prop changing.

It is clearly being included in what’s passed to the callback, along with the primary deps. It’s only running the equivalency check on the primary deps.

@BirdTho They are being passed to the hook* (not callback), sure, but they are only updated when the primary deps are updated. There is no magic connection between the values in the closure and the values in the dependency array. In other words, the callback closure remains stale until the primary deps change.

But this is getting off topic, so if you don’t believe me I suggest you simply test it yourself with and without the so called secondary deps.

const callbackContainer = useRef(callback) callbackContainer.current = callback

@mariusbrataas Updating a ref during render is an unsafe side effect.

Effects only run once a component is committed (a separate step to update the DOM after a component has been successfully rendered). But due to concurrent features such as suspense, there is no guarantee that a component that has started rendering will finish rendering, much less be committed.

I was aware of that. I actually meant “before or after committing children”. Sorry for the confusion.

In other words, useLayoutEffect is not a reliable way to tell whether it is still rendering.

Agree.

Another solution, that would work better, I think:

function useConcurrentModeSafeRef(initialValue) {

  const isRendering = () =>
    !!React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactCurrentOwner.current;

  const rawRef = useRef();

  const raw = rawRef.current ? rawRef.current : (
    rawRef.current = {
      comittedValue: initialValue,
      currentValue: initialValue,
      ref: {
        get current() {
          if (isRendering()) {
            return raw.currentValue;
          } else {
            return raw.comittedValue;
          }
        },
        set current(v) {
          if (!isRendering()) {
            raw.comittedValue = v;
          }
          raw.currentValue = v;
        }
      }
    }
  );

  raw.currentValue = raw.comittedValue;

  useLayoutEffect(() => {
    raw.comittedValue = raw.currentValue;
  });

  return raw.ref;

}

function useStatic(cb) {
  const callback = useRef(cb)
  callback.current = cb

  const mem = useConcurrentModeSafeRef((...args) => callback.current(...args))
  return mem.current
}

Of course, that’s bad because it’s using unsafe API (React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED). Perhaps something like useConcurrentModeSafeRef() could be added to React itself.

Might this approach work fine when combined with useConcurrentModeSafeRef()?

Hoping there will be some solution to this in React 18.

What if we had some kind of ref hook that safely keeps track of the last commited value on any given render thread?

function useEventCallback(fn) {
  let ref = React.useConcurrentRef(fn);
  // value could be updated like:
  //   ref.setCurrent(fn);
  return useCallback((...args) => (0, ref.getCurrent)(...args), []);
}

Maybe it could have a current property with getters/setters just like useRef; I just used setCurrent/getCurrent in my example to illustrate that magic would be happening under the hood.

@Hypnosphi the problem you describe is a general useEffect problem (how to write effects with dependencies that don’t need to trigger the effect), it is not specific to the the useCallback solution i described. there are number of approaches you can take to solve prop dependencies in effects.

i suggest taking a look here: https://overreacted.io/a-complete-guide-to-useeffect/#why-usereducer-is-the-cheat-mode-of-hooks

there is also a custom hook named “usePrevious”, you can use it to check if a specific dependency changed on useEffect. im not sure how much of a best practice it is.

here is one solution:

  const [{logReport}, handleClick] = useReducer((state) => ({
    counter: state.counter + 1,
    logReport: `${props.name}: child clicked ${state.counter + 1} times`
  }), {counter: 0, logReport: ''});

  useEffect(() => {
    logReport && console.log(logReport);
  }, [logReport]);

you can use props inside reducers if you put the reducer function inside the component (which you shouldn’t do by default, but you can). note that logReport does not change when props.name changes, but only when the user clicks.

@liorJuice how would you use props in side effects with this approach?

I found that you can use useReducer to mitigate this effectively. the bit that was hard to swallow is when i needed side effects to happen in the callback.

Side effects cant happen in the reducer as it is being called with past actions a lot, on re renders, for some reason. so the reducer must be pure (state, action) => resultState, with no side effects.

The solution was to use useEffect that listen to changes in the reduced state (resultState), to initiate side effects, which suddenly seemed obvious. 😅

for example, this code:

  const [counter, setCounter] = useState(0);

  const handleClick = useCallback(() => {
    console.log(`child clicked ${counter + 1} times`);
    setCounter((c) => c + 1);
  }, [counter]);

which will create a very problematic callback, could be perfectly replaced with:

  const [counter, handleClick] = useReducer((c) => c + 1, 0);

  useEffect(() => {
    counter && console.log(`child clicked ${counter} times`);
  }, [counter]);

which will have the same effect but with handler that will persist through the entire lifetime of the component.

i think this is the most complete solution to the useCallback problem. @gaearon can you confirm?

@mariusGundersen setCopy is guaranteed to remain the same through component’s lifetime, while createCopy changes each time when value does

This line in your above example could cause problems:

paramsRef.current = params;

Since you’re mutating the ref during render- and renders can be async- you can end up mutating it prematurely (so that a previously-rendered thing calls the callback and gets a not-yet-rendered params value.

To avoid this, you would want to update the ref in a layout effect (useLayoutEffect) although this has its own drawbacks (see this comment).

Edit for clarity.

@arnotes Looks like you are updating your ref during the render phase, i.e. before it has been commited, rather than in a useEffect call. To quote @gaearon :

In concurrent mode, last render doesn’t necessarily mean “latest committed state”. So a low-priority render with new props or state would overwrite a reference used by current event handler.

Afaik the drawback with updating the ref in a useEffect is mainly that you then can’t use it during the render.

I found myself doing this often:

function Foo({ someProp, otherProp }) {
  const somePropRef = useRef(someProp);
  useLayoutEffect(() => {  // or useEffect
    somePropRef.current = someProp;
  }, [someProp]);

  const otherPropRef = useRef(otherProp);
  useLayoutEffect(() => {  // or useEffect
    otherPropRef.current = otherProp;
  }, [otherProp]);

  const onSomething = useCallback(() => {
    handleSomething(somePropRef.current, otherPropRef.current);
  }, []);

  return <Bar onSomething={onSomething} />;

  // or...
  //
  // // This ref value never changes, so can use .current in render.
  // // WARNING: Safe to close over refs and outer scope consts only.
  // const onSomethingRef = useRef(() => {
  //   handleSomething(somePropRef.current, otherPropRef.current);
  // });
  //
  // return <Bar onSomething={onSomethingRef.current} />;
}

Does that have any drawbacks besides unnecessary effects?