yardz commented on a change in pull request #13220:
URL: https://github.com/apache/superset/pull/13220#discussion_r578892154
##########
File path: superset-frontend/src/explore/components/ExploreViewContainer.jsx
##########
@@ -146,11 +147,11 @@ const getWindowSize = () => ({
function useWindowSize({ delayMs = 250 } = {}) {
const [size, setSize] = useState(getWindowSize());
- useEffect(() => {
+ useComponentDidMount(() => {
const onWindowResize = debounce(() => setSize(getWindowSize()), delayMs);
window.addEventListener('resize', onWindowResize);
return () => window.removeEventListener('resize', onWindowResize);
- }, []);
+ });
Review comment:
@nytai If you need behavior similar to a "ComponentDidMount" it is a
``useEffect( effect, [] )``. But the ``[]`` will **always** generate an error
with the rule and forces you to use a ``// eslint-disable-next-line
react-hooks/exhaustive-deps``.
In **this case** it is not a technical debt, it is a design decision... I am
extremely against "hiding" lint errors (and I hate use "eslint-disable"). But
in this case, this is exactly the desired behavior, there is no escaping for it
(I already tried, and a lot).
See the "FlashProvider", this is the desired behavior (apply the effect
**exclusively** once).
The cases where I subsisted useEffect were the cases that it was being
applied in this way. The hook is not wrong, it is not a technical debt and it
is not an "anti pattern". If I applied it to a file that already contained the
same logic and it was wrong it is not a problem with the hook or with this PR,
it is a problem with the PR that accepted the file with the "bad implementation"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
