-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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
Extending useState result with getState #21220
Comments
I am sorry, I cannot get the point to getState(). Since state and setState() should only be used in and between Components in Render Phase and if state is not latest which means the latest render hasn't been run yet and that state should not been deemed as reliable, I am afraid. |
@jacty |
I forgot to mention I am working also with useEffect hook and adding event listeners manually, because I need active listeners (to be able to prevent default behavior). React creates all events listeners as passive and gives no way to make it active. Here is a demo of the problem being discussed and possible solution I am currently using: import React from "react";
import "./styles.css";
import { useRef, useEffect } from "react";
import useGetState from "./useGetState";
export default function App() {
const ref1 = useRef(null);
const ref2 = useRef(null);
const [counter, setCounter, getCounter] = useGetState(0);
useEffect(() => {
if (ref1.current && !ref1.current.subscribed) {
ref1.current.subscribed = true;
ref1.current.addEventListener("pointerup", () => alert(counter), {
passive: false
});
}
}, [counter, ref1]);
useEffect(() => {
if (ref2.current && !ref2.current.subscribed) {
ref2.current.subscribed = true;
ref2.current.addEventListener("pointerup", () => alert(getCounter()), {
passive: false
});
}
}, [getCounter, ref2]);
return (
<div className="App">
<button onClick={() => setCounter(counter + 1)}>
Increment Counter: {counter}
</button>
<br />
<button ref={ref1}>Alert counter state on not passive event</button>
<button ref={ref2}>Alert counter getState() on not passive event</button>
</div>
);
} Maybe I am doing something wrong and we still can use current state value directly as given by useState, but I found no better way. I guess the fact, that there is such a form of setState, that is accepting callback and providing current state value is already proof of problem existence, otherwise why react supports this? So what I suggest - just to expose already existing functionality of getting current state value directly, without wasting resources on wrapping it into setState method. I pretty often know for sure I will not change the state, so why should I make react doublecheck if new state in setState method arg is actually old one. |
@YuriGor, one of the ways to achieve what you are trying to do is using the cleanup state of useEffect(() => {
const eventHandler = () => {
alert(counter);
}
const node = ref1.current;
if (node) {
node.addEventListener("pointerup", eventHandler, {
passive: false
});
}
return () => {
node.removeEventListener("pointerup", eventHandler, {
passive: false
});
};
}, [counter]); In addition, I'm not sure how complex your case is than the example in codesandbox but I want to note that you can also use the |
Thank you @eusbolh, do you think adding / removing listeners is cheaper from performance perspective, comparing to just taking fresh state from In my real app I am using mouse wheel and touch events to make zoom and pan, so I need to prevent default scrolling/zooming behavior, in order to apply my own. I have "scale" "offsetX" and "offsetY" (and some others like minScale which is recalculating depending on window size) states, and to calculate matrix properly I need to consider all of them, but state variable itself is useless in my case and I have to take it from 'setState'. Now you can imagine how many events is produced during simple "pinch" gesture. Do you see any hidden drawbacks in the approach I am suggesting? |
I believe that what you are doing is a workaround rather than a solution. You might be getting away with your specific case, but it will get more complex as the codebase grows. Let's assume you have a prop that needs to be used in that event handler. How would you manage to make that work? I created a sandbox for you to work on. For this example, you might think that you can come up with a similar workaround as you did with states (providing getters for props with the help of setState). How about the fetched data in redux (or any other state management library if you are using any)? You will not be able to use In theory, you can keep everything using the
The most common case for functional update syntax is the case that the new state is computed using the previous state. For example, if you are incrementing a counter, the new state is calculated as
I don't have specific performance data related to these two approaches but I believe that adding and removing 4-5 event listeners on each render shouldn't cause any major performance problems. On each render, lots of other things happen besides the event listeners you add. It shouldn't matter that much if the number of event listeners doesn't get out of hand. If it does get out of hand, then that means you have bigger problems. In my opinion, you should try to optimize your code when you experience performance problems because working with stale closures will only get worse. |
Good point about other props, thank you. I had an idea ,instead of adding custom native listeners directly, add one generic listener in some hook so this generic listener will route events to another layer of custom handlers declared inside render method via useEffect, It would be kind of getting action back to component, instead of floating away from it into separate independent entity with it's own life cycle. |
Finally I just made a better use active listeners hook which is supporting deps but recreates only handlers, without reattaching listeners to dom node (I just feel safer by not doing this while, for example, touchmove is in progress) import isEqual from 'lodash/isEqual';
const elements = new WeakMap();
export default function useActiveListeners({ current: element }, handlers, ...deps) {
if (!element) return;
if (!elements.has(element)) {
const cfg = { deps };
Object.entries(handlers).forEach(([name, handler]) => {
const listener = function (e) {
return listener.handler(e);
};
listener.handler = handler;
cfg[name] = listener;
element.addEventListener(name, listener, { passive: false });
});
elements.set(element, cfg);
} else {
const cfg = elements.get(element);
if (!isEqual(cfg.deps, deps)) {
Object.entries(handlers).forEach(([name, handler]) => {
cfg[name].handler = handler;
});
}
}
} So a lot of redundant code was removed from my component and there is no more use cases for getState. |
Hi, it's an often case when we cannot rely on current state value given by useState, e.g. if we need this current value inside some function - it will be 'stale'.
One of the ways to really get latest state is to call setState method given by useState and pass callback as an argument to the setState, so this callback will receive up to date state value.
For myself I created a wrapper for useState, which is giving third result array element, and this element is getState function:
usage example:
As you see this getState function is just non-mutating call of setState just to grab the fresh state value.
It would be great to extend the original useState hook in a similar, but, maybe, a bit more efficient way,
because I believe original setState makes some extra stuff not needed if we only need to get state and nothing more.
The text was updated successfully, but these errors were encountered: