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

useCallback() invalidates too often in practice #14099

Open
gaearon opened this issue Nov 5, 2018 · 119 comments
Open

useCallback() invalidates too often in practice #14099

gaearon opened this issue Nov 5, 2018 · 119 comments
Labels
Component: Hooks React Core Team Opened by a member of the React Core Team

Comments

@gaearon
Copy link
Collaborator

gaearon commented Nov 5, 2018

This is related to #14092, #14066, reactjs/rfcs#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 #14092 (comment) 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.

@edkalina
Copy link

edkalina commented Nov 7, 2018

@gaearon thank you for your answer in reactjs/rfcs#83. I've look at sources of useReducer. But I can't understand how it is related to useCallback. What issues has "mutation of ref during rendering"? Can you explain me in brief?

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 7, 2018

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".

@strayiker
Copy link

strayiker commented Nov 10, 2018

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)

@sokra
Copy link
Contributor

sokra commented Nov 19, 2018

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.

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 19, 2018

Yes, that's the approach from reactjs/rfcs#83 and https://reactjs.org/docs/hooks-faq.html#how-to-read-an-often-changing-value-from-usecallback. We don't want it to be default because it's easier to introduce bugs in concurrent mode this way.

@sophiebits
Copy link
Collaborator

@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.

@sokra
Copy link
Contributor

sokra commented Nov 20, 2018

@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.

@strayiker
Copy link

strayiker commented Nov 20, 2018

@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]
);

@sokra
Copy link
Contributor

sokra commented Nov 20, 2018

@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.

@muqg
Copy link

muqg commented Nov 21, 2018

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?

@gaearon
Copy link
Collaborator Author

gaearon commented Nov 21, 2018

@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.

@strayiker
Copy link

strayiker commented Nov 21, 2018

If I understand the problem correctly...

What if useCallback will return a special callable object with two different states of an our callback function? The first is a current callback, that will changes on each render. The second is a commited callback, that changes within a commit phase.
By default call to that object will trigger the current value, so it can be used in render phase.
But internally, React will pass the commited value to the dom, which prevents the callback from being modified until the next commit.

function Callback(cb) {
  function callback(...args) {
    return callback.current(...args);
  }

  callback.commited = cb;
  callback.current = cb;
  callback.SPECIAL_MARKER_FOR_REACT = true;

  return callback;
}

function useCallback(cb) {
  const callback = useMemo(() => new Callback(cb), []);

  callback.current = cb;
  
  useLayoutEffect(() => {
    callback.commited = cb;
  });

  return callback;
}
function Component(counter) {
  const handler = useCallback(() => {
    console.log(counter);
  });

  handler(); // call to handler.current

  // pass handler.commited to the dom
  return <button onClick={handler} />
}

@Volune
Copy link

Volune commented Nov 22, 2018

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?

@strayiker
Copy link

strayiker commented Nov 22, 2018

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.

@Voronar
Copy link

Voronar commented Dec 14, 2018

Hi.
According to @sophiebits useEventCallback implementation why is it
function uses useLayoutEffect and not useEffect for ref updating?
And is it normal due to current limitations use useEventCallback for all internal regular functions with some logic (which wants be memoized for
using in expensive pure components tree or/and has closured variables from outer hook function) inside custom hook?

@jonnyasmar
Copy link

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.

@Bazzer588
Copy link

I have a problem with converting this kind of thing to use functional components and the useCallback hook...

export class TestForm extends React.Component {

    onChangeField = (name,value) => {
        this.setState({ [name]: value });
    };

    render () {
        const state = this.state;
        return (
            <>
                <PickField name="gender"      value={state.gender}      onChangeField={this.onChangeField} />
                <TextField name="firstName"   value={state.firstName}   onChangeField={this.onChangeField} />
                <TextField name="lastName"    value={state.lastName}    onChangeField={this.onChangeField} />
                <DateField name="dateOfBirth" value={state.dateOfBirth} onChangeField={this.onChangeField} />
            </>
        );
    }

The PickField, TextField and DateField components can be implemented with React.PureComponent or React.memo(...). Basically they just display an input field and a label - they have their own onChange handler which calls the onChangeField prop passed in. They only redraw if their specific value changes

onChangeField as above works just fine - but if I try this using a functional component for TestForm and useCallback for onChangeField I just can't get it to 'not' redraw everything on a single field change

@jonnyasmar
Copy link

@Bazzer588 Are you using React.memo on your functional component? What do your attempts using hooks/functional look like? Your problem may or may not be related to this issue; it's hard to tell without seeing your code.

@Bazzer588
Copy link

Bazzer588 commented Jan 6, 2019

Here's the full code - just drop a <TestForm/> or a <HookTestForm/> somewhere on a page

Using a React.Component, only the field being editted does a full render

import React from 'react';
import TextField from "./TextField";

export default class TestForm extends React.Component {

    onChangeField = (name,value) => {
        this.setState({ [name]: value });
    };

    render () {
        const state = this.state || {};
        return (
            <>
                <TextField name="gender"      value={state.gender}      onChangeField={this.onChangeField} />
                <TextField name="firstName"   value={state.firstName}   onChangeField={this.onChangeField} />
                <TextField name="lastName"    value={state.lastName}    onChangeField={this.onChangeField} />
                <TextField name="dateOfBirth" value={state.dateOfBirth} onChangeField={this.onChangeField} />
            </>
        );
    }
}

Using Hooks, all the child elements are re-rendered every time - presumably as onChangeField changes every time the state data changes. Is there some way I can implement onChangeField so it behaves like the above example?

import React, {useState, useCallback} from 'react';
import TextField from "./TextField";

export default React.memo( () => {

    const [data, changeData] = useState({});

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

    return (
        <>
            <TextField name="gender"      value={data.gender}      onChangeField={onChangeField} />
            <TextField name="firstName"   value={data.firstName}   onChangeField={onChangeField} />
            <TextField name="lastName"    value={data.lastName}    onChangeField={onChangeField} />
            <TextField name="dateOfBirth" value={data.dateOfBirth} onChangeField={onChangeField} />
        </>
    );
});

This is my <TextField> component - you can see when it does a full render from the console or with the React dev tools

import React from 'react';

export default React.memo( ({ name, value, onChangeField }) => {

    console.log('RENDER TEXT FIELD',name,value);

    return (
        <div className="form-field">
            <label htmlFor={name}>{name}</label>
            <input
                type="text"
                onChange={ (ev) => onChangeField( name, ev.target.value ) }
                value={value || ''}
            />
        </div>
    );
});

@muqg
Copy link

muqg commented Jan 6, 2019

@Bazzer588 I think its due to the object value kept in state under the variable name of data. I don't know why but objects in state always invalidate memoization and thus your callback onChangeField is a new on on each render thus breaking the memoization of the components you're passing it to.

I've had a similar issue like you and noticed this as being its cause. I have no idea why the object in state does not keep its reference when it has not been explicitly set to a new object.

@Bazzer588
Copy link

Yes my problem is that the in first example (using React.Component) I can create a onChangeField callback which is bound to this, and never changes during renders

Using the new hook methods I can't seem to replicate the way the existing functionality works,

On the project I'm working on we often do this to have a hierachy of components and state:

    onChangeField = (fieldName,fieldValue) => {
        const newValue = { ...this.props.value, [fieldName]: fieldValue };
        this.props.onChangeField( this.props.name, newValue );
    };

So it passes props down the tree (ie value {})
And uses the callback to send new values up the tree

@dantman
Copy link
Contributor

dantman commented Jan 6, 2019

Using the new hook methods I can't seem to replicate the way the existing functionality works,

Use useReducer.

@jonnyasmar
Copy link

@Bazzer588 The data var your passing to the second parameter of useCallback is going to invalidate every time. useCallback doesn't do a deep comparison. You need to pass in a flat array for it to properly memoize.

@BirdTho
Copy link

BirdTho commented May 20, 2022 via email

@BirdTho
Copy link

BirdTho commented May 20, 2022

@Hypnosphi So basically, if I'm not going to use concurrent mode, this is fine then?

@nikparo
Copy link

nikparo commented May 21, 2022

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.

@Moranilt
Copy link

Moranilt commented Jun 6, 2022

Seems like it's useless with onChange events for input:

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

const Input: React.FC<InputPropsA> = ({ name, value, setValue }) => {
  const onChangeHandler = useEvent((e: React.ChangeEvent<HTMLInputElement>) => {
    setValue(e.target.value);
  });

  return <input name={name} onChange={onChangeHandler} value={value} />;
};

const Form = () => {
  const [firstname, setFirstname] = useState('');
  const [lastname, setLastname] = useState('');
  const [middlename, setMiddlename] = useState('');

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

Even if you put every setState into useCallback - it's not working for me right now with this hook

If I'm doing something wrong, pelase guide me.

@artem-malko
Copy link

artem-malko commented Jun 6, 2022

@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.

@kambing86
Copy link

kambing86 commented Oct 10, 2022

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

@jampy
Copy link

jampy commented Oct 10, 2022

@kambing86 this will not work properly in "concurrent mode", see #14099 (comment)

@sarakusha
Copy link

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.

@kfcaio
Copy link

kfcaio commented Oct 23, 2022

While it's not fully available, what could we use to achieve the closest behavior? https://github.com/vadzim/react-use-handler, https://github.com/gfox1984/granular-hooks, or some gist above? I'm using React 16

@KingSora
Copy link

KingSora commented Dec 13, 2022

@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.

@sophiebits wouldn't useImperativeHandle be even closer to the desired behavior:

function useEventCallback(fn) {
  const ref = useRef(fn);
  useImperativeHandle(ref, () => fn);
  return useCallback((...args) => ref.current?.(...args), []);
}

If thats not the case, can you explain why? - Just curious whether I'm understanding everything properly.

@HansBrende
Copy link

HansBrende commented Dec 16, 2022

I think we can save some overhead by writing it this way (avoiding what seems to be an unnecessary useCallback call in the above proposals):

export const useEvent = (fn) => {
  const ref = useRef([fn, (...args) => ref[0](...args)]).current;
  useLayoutEffect(() => {
    ref[0] = fn;
  });
  return ref[1];
}

@jewalky
Copy link

jewalky commented Jan 28, 2023

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.

At the first glance, it may have the same issues with concurrent mode as this comment: #14099 (comment)
But, by quickly checking, it looks like concurrent mode was abandoned altogether, and as such, any considerations about it are irrelevant.

@BirdTho
Copy link

BirdTho commented Jan 28, 2023 via email

@JoaquimEsteves
Copy link

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.

@troglotit
Copy link

troglotit commented Sep 25, 2023

Had a quite interaction heavy app, so had to optimize rendering quite heavily. So leaned into setState(updaterFn)

function App() {
  let [a, setA] = useState()
  let [b, setB] = useState()
  
  useEffect(() => {
    setA(a => {
      setB(b => {
        callApi(a,b)
        return b
      })
      return a
    })
  }, [])

  // which fundamentally can be changed to either adding custom hook, 
  // or extending React's useEffect/useMemo/useCallback via RFC
  useStateEffect((a,b) => {
    callApi(a,b)
  }, [], [setA, setB])
}

Which, I think, precisely solves all the problems: optimize useCallback, premature "useEvent all the things". But requires passing setStates instead of state itself as props.

Also, not sure about this concern in another issue, maybe I'm missing something:

You can useReducer or setState(updaterFn) to avoid closing over the variables. This doesn't solve all use cases (especially when you also need to perform side effects) but gets you pretty close.

@vzaidman
Copy link
Contributor

      setB(b => {
        callApi(a,b)
        return b
      })

Is this the correct code? To me it reads like-
"On mount, only once, callApi with the latest a and b values from the useState hook."

@troglotit
Copy link

troglotit commented Sep 25, 2023

@vzaidman you're reading correctly. But, I have to concede, the code is quite abstracted (i.e. you can think of useCallback/useMemo instead of useEffect)

UPD: After a bit of time, I think we don't really need setters:

function ComponentName({a}) {
  const [b, setB] = useState(0);

  const cb = useMemo((a,b) => 
    (event) => {callbackDeps1(event); callbackDeps2(a,b)},
    [callbackDeps1, callbackDeps2],
    [a,b]
  );
    
  return <Button onClick={cb} />
}

@Effanuel
Copy link

Had a quite interaction heavy app, so had to optimize rendering quite heavily. So leaned into setState(updaterFn)

function App() {
  let [a, setA] = useState()
  let [b, setB] = useState()
  
  useEffect(() => {
    setA(a => {
      setB(b => {
        callApi(a,b)
        return b
      })
      return a
    })
  }, [])

  // which fundamentally can be changed to either adding custom hook, 
  // or extending React's useEffect/useMemo/useCallback via RFC
  useStateEffect((a,b) => {
    callApi(a,b)
  }, [], [setA, setB])
}

Which, I think, precisely solves all the problems: optimize useCallback, premature "useEvent all the things". But requires passing setStates instead of state itself as props.

Also, not sure about this concern in another issue, maybe I'm missing something:

You can useReducer or setState(updaterFn) to avoid closing over the variables. This doesn't solve all use cases (especially when you also need to perform side effects) but gets you pretty close.

Can you make this code more complex, its too simple

@nvmnghia
Copy link

nvmnghia commented Aug 1, 2024

Had a quite interaction heavy app, so had to optimize rendering quite heavily. So leaned into setState(updaterFn)

function App() {
  let [a, setA] = useState()
  let [b, setB] = useState()
  
  useEffect(() => {
    setA(a => {
      setB(b => {
        callApi(a,b)
        return b
      })
      return a
    })
  }, [])

  // which fundamentally can be changed to either adding custom hook, 
  // or extending React's useEffect/useMemo/useCallback via RFC
  useStateEffect((a,b) => {
    callApi(a,b)
  }, [], [setA, setB])
}

Which, I think, precisely solves all the problems: optimize useCallback, premature "useEvent all the things". But requires passing setStates instead of state itself as props.

Also, not sure about this concern in another issue, maybe I'm missing something:

You can useReducer or setState(updaterFn) to avoid closing over the variables. This doesn't solve all use cases (especially when you also need to perform side effects) but gets you pretty close.

Looks really weird. Why didn't you just use a & b, leave the deps array empty then add some lint ignore & comment?

@tibbe
Copy link

tibbe commented Sep 16, 2024

@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.

Perhaps a stupid question. What is the 0 doing there? Why isnt' it just ref.current()?

@vadzim
Copy link

vadzim commented Sep 17, 2024

What is the 0 doing there? Why isnt' it just ref.current()?

@tibbe
It's a trick to call a function without setting this and an equivalent of that:

const f = ref.current;
f()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Hooks React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

No branches or pull requests