-
Notifications
You must be signed in to change notification settings - Fork 559
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
[hooks] useState - "setState" callback #98
Comments
@sandiiarov that won't do what @marcellomontemagno is asking, it just logs immediately, not in the callback. @marcellomontemagno you can use a |
Yes, there is not a second argument on the setState hook callback. +1'd because I also want to know the reason why. |
@marcellomontemagno agree with @jquense to use |
Could someone write an example? Imagine a list of items that a user can cursor through. Whenever we get a keydown event on an arrow we update the cursor position, and then in the callback we scroll the currently selected element into view. const itemCount = 10;
const [cursor, setCursor] = useState(0);
const handleKeyDown = e => {
if (e.key === 'ArrowUp') {
setCursor(cursor => Math.max(0, cursor - 1), () => {
// scroll element into view
});
} else if (e.key === 'ArrowDown') {
setCursor(cursor => Math.min(itemCount - 1, cursor + 1), , () => {
// scroll element into view
});
}
} Lets also say that arrow keys aren't the only reason that const itemCount = 10;
const [state, setState] = useState({ cursor: 0, scrollKey: 0 });
const handleKeyDown = e => {
if (e.key === "ArrowUp") {
setState(state => ({
cursor: Math.max(0, state.cursor - 1),
scrollKey: state.scrollKey + 1
}));
} else if (e.key === "ArrowDown") {
setState(state => ({
cursor: Math.min(itemCount - 1, state.cursor + 1),
scrollKey: state.scrollKey + 1
}));
}
};
useEffect(
() => {
if (state.scrollKey > 0) {
// scroll to current element
}
},
[state.scrollKey]
); Does that look about right? |
Same here, i find the callback very useful. Too i can't imaging a scenario why this should be removed. Or how it influence performance. I really want to know the why ! For me i just though it's a one call or a resolve of a promise. That will happen when the updates happen. I don't know but it's seem that there is more to it. facebook/react#14174 (comment) (this thread is treating that better). So i guess i've got to look at how the hooc model is implemented and work. However combining with useEffect. we can use a promise, that we resolve in useEffect. And keep more of the flexibility. Even thogh not too cool. Check this (a more lively thread, with a lot of context) #68 (comment) |
Because it invokes a thought pattern that may very well be an antipattern with hooks. With hooks, you are writing code more declarative than imparative. The thought of "do one thing THEN do another thing THEN do another thing" would be the imparative (here: callback- or promise-based) way. What actually should be done in the hooks model would rather be a "if this has been rendered, and this property is different than before, X should be done to reach the target state", which is more declarative. You don't designate when stuff happens, but just give a condition/dependency and let react pick the right point in time for it. |
Folks may already know all of/some of this, but I figured I'd post it just in case. The warning that's logged when trying to use the 2nd callback argument provides a hint as to why it isn't supported: Judging from the message, it seems that using Here's a CodeSandbox example of the And also just to clarify again, I know you may already know all of this @marcellomontemagno but since hooks are new-ish I figured I'd post this just in case! 😅 |
This reminds me of a game "guess what to do in componentWillReceiveProps by comparing N different things". I have a form library and I'm thinking of how to update it to the new API. I'm just getting myself familiar with hooks and got stuck pretty soon on the following case: user submits a form. In the previous class-based model, when In the hooks world I can't do that. I have a reducer that returns a state only and
Here's the first confusing point: what if re-render was triggered during submission for some unrelated reason (e.g. parent props was updated due to whatever). In fact, submission request was triggered and is ongoing but since re-render was triggered and form status is still This common case seems pretty tricky with hooks. But hopefully I just didn't grok it yet and there is clean way to handle it within the new model. |
Usually, I'd trigger the asynchronous side-effect directly from button.onClick or form.onSubmit and store the results back using a useReducer dispatch. Since that seems not to be an option for you, maybe do something along these lines? depend only on isSubmitting, pass state via Ref
The effect only triggers when state.isSubmitting changes and within it, you have access to the latest state using the ref. Though I would not recommend using this reference hack in every situation, in your case it seems fitting. But keep in mind: if you have a promise chain in your useEffect and access the stateRef later (with a few renders in between), you might access a later version of the state, not the state at the time the effect was triggered Or, you omit the reference here, that way you would have access to the state at the time of triggering the effect - but you would have eslint-plugin-react-hooks linter warning that you might need to ignore. depend on full state, keep track of last submit in a Refin this case, we assume a property
|
I spent some time today on this and ended up with using custom hook that brings back Looks like it works. All props to @bloodyowl for initial gist: https://gist.github.com/bloodyowl/64861aaf1f53cfe0eb340c3ea2250b47 |
Yes, you can use useEffect/useLayoutEffect to add your custom callback function after updating state. If you are looking for an out of the box solution, maybe this custom hook which is nothing else than useState with a callback function may be handy for you. |
@rwieruch your code is dangerous, everytime the callback changes (which does happen frequently if using closures) the callback will fire again. You need to "stabilize" your callbacks using refs (Dan Abramov has a nice blog post somewhere on that topic) |
I understand that from a general case point of view useEffect() will run our code on the place of the old setState() callback, however consider this case: ... onBlur() { So here we want to execute 2 different callbacks depending on the event. If we fire onChange() we want to fire its parent's onChange if provided in the component props and so on. In short every type of setState() needs to have a separate useEffect(). How can this be managed with hooks without the "removed" callback. The only way I could think of, is to have additional if / else logic inside the useEffect() hook ? |
Exactly that example is why you wouldn't want that callback. |
In my case, I wanted to wait for the re-render, because afterward I can operate on a |
The need for a way to "chain" setState is one that I find myself having to deal with. Consider the following code: https://text.evie.codes/igacuvadug.js . Here, my issue is that I need to do this on clicking my component :
With the lack of callback (which, to be fair, I'd immediately turn into a promise and use async/await, because c'mon, callback hell!), I find myself unable to chain these in any meaninful fashion. Since those operations become batched, it means my progress bar's position resets to zero without the Spring reset being triggered, so it doesn't just go to 0, it moves to 0. The suggestions above (and what I've seen previously in terms of useEffect solutions) account for replacing one callback but I need to replace 4 of them (I could batch some of these things, but not all of them). The useEffect condition hell I would put myself in sounds like it would be a nightmare to manage. |
@eslachance here's a working example without useEffect at all: https://codesandbox.io/s/weathered-tdd-794hs Here's what I did:
|
Damn that's why my reset wasn't working 😅 |
Hey all This is how I solve it in my apps. Just published it as an opensource package: https://github.com/slorber/use-async-setState import { useAsyncSetState, useGetState } from "use-async-setState";
const Comp = () => {
const [state,setStateAsync] = useAsyncSetState({ counter: 0 });
const getState = useGetState(state);
const incrementTwiceAndSubmit = async () => {
await setStateAsync({...getState(): counter: getState().counter + 1});
await setStateAsync({...getState(): counter: getState().counter + 1});
await submitState(getState());
}
return <div>...</div>
} |
Great thread! I was wondering about the same thing. |
There's a very good discussion on Stackoverflow: The My scenario is that I have some |
Do you want to open a PR for https://github.com/the-road-to-learn-react/use-state-with-callback to include the prevention of calling the callback on mount? Didn't think about it for my use case back then, but I think it should be only something along these lines:
|
+1 |
There is another scenario which one might want a to run a callback per state update, i think its the same or similar to what was mentioned in this comment by @kdzhambazov . If you want to run 2 different callbacks for the same data that was changed, you can't do that only with This can be a custom hook obviously: function useStateWithCallback(initialValue) {
const [state, setState] = useState(initialValue);
// we need this flag for 2 reasons:
// 1. To prevent the call on mount (first useEffect call)
// 2. To force the effect to run when the state wasn't really updated
// i.e next-state === previous-state.
const [shouldRunCBs, setRunCBs] = useState(false);
// tracking a queue because we may have more than 1 callback per update?
const cbQRef = useRef([]);
function customSetState(value, cb) {
if (typeof cb === "function") {
cbQRef.current.push(cb);
// we force the effect to run even if the state wasn't really updated
// i.e next-state === previous-state.
// this is how the callback in classes work as well
// we can opt-out from this behaviour though
setRunCBs(true);
}
setState(value);
}
useEffect(() => {
if (shouldRunCBs) {
// we must pass back the new value
//because the consumers can't get it via the closure of thier component
// and they don't have an instance like in classes.
cbQRef.current.forEach(cb => cb(state));
cbQRef.current = [];
setRunCBs(false);
}
}, [state, shouldRunCBs]);
return [state, useCallback(customSetState, [])];
} Here is a sandbox with a simple use case |
That almost works -- if using synchronous React ( This very specific nitpicking aside (which you can't reproduce in your sandbox as you're using React events), this pattern still looks really strange. If you want to guarantee a |
@Jessidhia Thanks for the input, Edit I managed to bypass this limitation by tracking last value in a function useStateWithCallback(initialValue) {
const [state, setState] = useState(initialValue);
// we need to track down last changes to support synchronous updates
// e.g, addEventListener handlers
const lastStateRef = useRef(initialValue);
// we need this flag for 2 reasons:
// 1. To prevent the call on mount (first useEffect call)
// 2. To force the effect to run when the state wasn't really updated
// i.e next-state === previous-state.
const [shouldRunCBs, setRunCBs] = useState(false);
// tracking a queue because we may have more than 1 callback per update?
const cbQRef = useRef([]);
function customSetState(value, cb) {
if (typeof cb === "function") {
cbQRef.current.push(cb);
// we force the effect to run even if the state wasn't really updated
// i.e next-state === previous-state.
// this is how the callback in classes work as well
// we can opt-out from this behaviour though
setRunCBs(true);
}
setState(value);
}
useEffect(() => {
if (shouldRunCBs && state !== lastStateRef.current) {
// we must pass back the new value
//because the consumers can't get it via the closure of thier component
// and they don't have an instance like in classes.
cbQRef.current.forEach(cb => cb(state));
cbQRef.current = [];
setRunCBs(false);
lastStateRef.current = state;
}
}, [state, shouldRunCBs]);
return [state, useCallback(customSetState, [])];
} |
I use it like this:
|
Most solutions here have flaws:
Take another look at https://github.com/slorber/use-async-setState please:
|
@sag1v your solution is probably the closest to mine except doing setState(1,cb) if state=1 will lead to an extra unnecessary re-render. Just look how I handled this more efficiently here: https://github.com/slorber/use-async-setState/blob/master/src/index.ts#L33 |
@slorber Thanks. It was just a naive implementation example. My point was to show there are use cases for a state update callback, it is doable in user-land but maybe it should be provided by the library as it is with classes. |
useEffect is the proper solution to consume the updated state, but useEffect is called everytime the state is updated, so I wouldn't want to call my setState callback everytime. Can return a promise instead and use the thenable callback on-demand.
|
@arkakkar this is what I do with https://github.com/slorber/use-async-setState You also need to handle case like await setState(s => s) (which does not trigger useEffect) + handle rejects etc |
Then you should have a useEffect on the boolean flag and scroll to the list if it changes. Something like:
Because useEffect runs after rendering, the ref will be available |
I don't understand why people are reluctant to add a second parameter to
Which is much more clearer and compact you propose to make a |
I think I'm happy with this reply #98 (comment) This is still something people will have to deal with so here a recipe to deal with the lack of Option 1: subscribe to the state change you made via useCallbackThis option might not be suitable for you if that very same state change can happen for many different reasons. Options 2: in the event callback where the first setState is happening, just invoke another setState right after the first oneThis works because, even though the first Option 2 might not be suitable for you if instead of invoking a second If you need to invoke an imperative API from the browser such as Option 2.1: hide the imperative API from the browser making react your only source of truth.Here an example of how to try to convert an imperative API from the browser in something react can control:
It might be desirable to have Options 2.2: reintroduce the setState callback by yourself.@slorber did a good job in making this something reusable via https://github.com/slorber/use-async-setState. To wrap this up, my personal opinion is that is good to think about Please feel free to follow up with any concern about this recipe or just leave a 👍 If you agree. if confirmed, I would be happy to formalize this better and add a FAQ to the react documentation about hooks at some point as this discussion had quite a lot of replies. |
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. |
Hi, I'm very hyped about the hooks api and I started experimenting with it on some small side project.
I've noticed that the "setState" function returned by the
useState
hook doesn't have an "afterRender" callback (the second argument ofthis.setState
in classes)I didn't find further informations about this in the doc. Is there any design reason why the "afterRender" callback is not present? Is there a plan to introduce it later on?
The reason I ask is that during the past years of react usage I found a few scenarios (very rarely though) were not having the "afterRender" callback would result in a quite tedious implementation of
componentDidUpdate
or pushed me to change the shape of my state to work around it (I can provide examples about this)In the meanwhile here the snippet I used to understand that no "afterRender" callback is used by the "setState" of hooks
Counter.js
Root.js
the log never appears in the console after after increasing and decreasing the counter
The text was updated successfully, but these errors were encountered: