React组件中使用两个useEffect的代码评审与优化方案问询

First off, nice catch on that routing edge case—persistent layout components like your Header are tricky because they don’t re-mount when routes change, so leftover state from previous scrolls sticks around. Your current fix works, but we can make this more idiomatic React and cut down on redundant logic. Let’s walk through the improvements:

Key Issues in the Current Code #

• Direct DOM query (document.getElementById) ties your component to the DOM structure, which is brittle and not the React way.
• Tracking both scrollCurrent and scrollPrev as separate state variables creates unnecessary re-renders and complicates dependencies.
• Having two separate useEffect hooks can lead to race conditions or redundant state updates if not carefully managed.

Refactored Solution #

Here’s a cleaner approach that combines routing reset and scroll handling, uses React refs, and simplifies state:

import { NavLink, useLocation } from "react-router-dom";
import { useState, useEffect, useRef, useCallback } from "react";

function Header() {
  const [prevScrollY, setPrevScrollY] = useState(0);
  const headerRef = useRef(null);
  const { pathname } = useLocation();

  // Handle scroll behavior: hide/show header based on scroll direction
  const handleScroll = useCallback(() => {
    const currentScrollY = window.scrollY;
    const headerElement = headerRef.current;

    if (!headerElement) return;

    // Hide header when scrolling down (with small threshold to avoid jitter)
    if (currentScrollY > prevScrollY && currentScrollY > 100) {
      headerElement.classList.add("header-hidden");
    } else {
      headerElement.classList.remove("header-hidden");
    }

    setPrevScrollY(currentScrollY);
  }, [prevScrollY]);

  useEffect(() => {
    // Reset scroll state and show header immediately when route changes
    setPrevScrollY(0);
    headerRef.current?.classList.remove("header-hidden");
    
    // Bind scroll listener
    window.addEventListener("scroll", handleScroll);

    // Cleanup listener on unmount or route change
    return () => {
      window.removeEventListener("scroll", handleScroll);
    };
  }, [pathname, handleScroll]);

  return (
    <div 
      ref={headerRef}
      className="header flex w-full items-center justify-between p-2 border-b-2 border-(--sub) sticky"
    >
      <p>Header Content</p>
    </div>
  );
}

export default Header;

What’s Better About This Approach? #

• React Refs Instead of DOM Queries: Using useRef directly on the Header element eliminates the need for document.getElementById, making the component more self-contained and less dependent on global DOM IDs.
• Simplified State: We only track prevScrollY instead of two separate state variables, reducing the number of re-renders and simplifying dependencies.
• Combined Effect Logic: The single useEffect handles both route change resets and scroll listener setup/cleanup. This keeps related logic together and avoids potential conflicts between separate effects.
• useCallback for Scroll Handler: Wrapping handleScroll in useCallback ensures the function reference stays stable unless prevScrollY changes, preventing unnecessary listener re-registrations.
• Scroll Threshold: Adding a small threshold (currentScrollY > 100) prevents the header from flickering when scrolling near the top of the page.

Bonus: Polish with CSS Transitions #

To make the hide/show behavior feel smooth, add a transition to your CSS:

.header {
  transition: transform 0.3s ease-in-out;
}

.header-hidden {
  transform: translateY(-100%);
}

This way, the header slides in/out seamlessly instead of jumping abruptly.

内容的提问来源于stack exchange，提问作者Debabrata Adhikari