Feature proposal: Give useEffect cleanup an option to run on page unload

[TimurTimergalin]

Currently cleanup functions returned by useEffect hook are not executed on page unload, and for a good reason - most cleanups do not need to be called when the page is closing. But there are some situations, when it is useful. To name but a few:

• Calling close() method on WebSocket or EventSource instances;
• Performing requests with Beacon API or fetch with keepalive = true.

Currently, React does not have a built-in solution for this. It is possible to write something like:

function useReliableEffect(f: EffectCallback, deps: unknown[]) {
    useEffect(() => {
        const cleanup = f()
        const newCleanup = (typeof cleanup === "function") ? () => {
            window.removeEventListener("unload", newCleanup!)  // If cleanup is executed already, it should not run on page unload
            cleanup()
        } : undefined

        if (typeof newCleanup === "function") {
            window.addEventListener("unload", newCleanup)
        }
        return newCleanup

    }, deps);  // ESLint react-hooks/exhaustive-deps
}

but this solution does not have an advantage of statically checked dependencies. The alternative would be:

function useReliableEffect(f: EffectCallback) {  // Take only callback
    useEffect(() => {
        // Same code as above
    }, [f]);
}

// Caller
const f = useCallback(  // Delegate dependency management to caller
    () => {/*effect code*/}, [/*appropriate deps*/]
)
useReliableEffect(f)

but there is no guarantee that the caller will not forget to use useCallback

My proposal is to add an optional flag to the hook:

function useEffect(callback, deps?, cleanupOnUnload? = false)

that would make cleanup run on page onload.

[rjgotten]

Should that be on the actual page unload event?
Or on the beforeunload event?
Or maybe on the pagehide event, which is fired when the application goes into BF-cache for fast-resume?
Should it happen only on forward navigations? Or also on history navigations? And how about forward history navigations?

There's so--- many variants at play here, that this is just a bad idea.
It doesn't generalize well enough. All of the above mean very different things and all have viable arguments and use cases for being the thing you should be doing 'on unload'.

Imho, if you want this type of behavior you should really just create a custom usePageUnloadEffect or usePageHideEffect hook, or similar.

[KaleriKhan]

Hi @rjgotten,

Thank you for your insights on this proposal. I’m curious about how the proposed useEffect cleanup behavior on page unload might interact with Single Page Application (SPA) navigations or redirects within a React app. Specifically, could it help address issues such as lingering styles from the previous route when navigating between pages in a client-side rendered application?

Would you recommend handling such cleanup with the proposed useEffect flag, or do you think a custom hook (e.g., usePageHideEffect) would be more suitable for this type of scenario?

Thanks and best wishes,
KaleriKhan"

[guillaumebrunerie]

I had the same problem and I’ve been using the following workaround:

export const runOnPageHide = (cleanUp: () => void) => {
    const newCleanUp = () => {
        cleanUp();
        window.removeEventListener("pagehide", newCleanUp);
    };
    window.addEventListener("pagehide", newCleanUp);
    return newCleanUp;
};

Then to use it you just have wrap the clean up function of any regular effect with runOnPageHide:

useEffect(() => {
    // do stuff
    return runOnPageHide(() => {
        // do clean up
    }),
}, [some, dependencies])

Seems to work fine. Note that in my case, the React app runs in a popup window where I don’t have to worry about the BF-cache or about pagehide triggering when changing tabs, so you may have to tweak it to fit your needs. Just wanted to show another solution which uses the regular useEffect (so it doesn’t have issues with static dependencies) and doesn’t rely on remembering to use useCallback.

[rjgotten]

@KaleriKhan
Would you recommend handling such cleanup with the proposed useEffect flag, or do you think a custom hook (e.g., usePageHideEffect) would be more suitable for this type of scenario?

I would either build an application-specific custom hook, or would go for the full bespoke solution that @guillaumebrunerie brings up, which is essentially the same thing - but hidden behind less abstraction.

Adding an additional flag parameter to useEffect which tells it to cleanupOnUnload is bad because - as I wrote before: defining what 'on unload' means is hard; and is different for different use cases. It's not something that can be captured in a simple boolean toggle.

Also; there's the whole thing where flag parameters that alter implementation specifics can be considered a code smell. For good reason, because it leads to the anti-pattern of "configuration-over-composition" where over time you expand one particular unit of code with additional configuration again and again, bloating it and violating the single-responsibility principle.

Case in point: if you'd say: "Well, okay-- so a simple boolean toggle isn't enough. Let's pass an options object instead," then you're already headed for this territory because you'll have to specify the type of DOM event. As unload, beforeunload, and pagehide all behave differently. For the pagehide case you'd also need to specify the types of navigation on which the cleanup logic should run.

You'd also still need a way to set apart the page-unload cleanup logic from the regular component cleanup logic that's run when the effect dependencies change or the component unmounts. And where multiple different types of pagehide navigation are valid for the cleanup logic to run, you'll need a way to tell the cleanup logic which particular type of navigation it is dealing with. Which means the useEffect cleanup function will need to be changed to take parameters as well.

And this doesn't even address the elephant in the room of setting aside real page navigations that cause actual document (re)loads from a server, with virtual pushState or replaceState navigations. Where the latter also cross-connects to relevant subject matter such as which router framework is being used and how things should connect with it -- since it might be doing things on top of pushState and replaceState that make for the alignment between those and actual router navigations to be slightly off.

In short:
The whole idea just brings in too much baggage, no matter which way you slice it, to allow for it to be bolted onto the existing general-purpose useEffect. And the answer is to use composition instead: build your own hook which can use useEffect.

@guillaumebrunerie
(so it doesn’t have issues with static dependencies)

That's more of a problem with the eslint-plugin-react-hooks package.
There are ample different ways it could support custom hooks with dependency list parameters properly.
Where typescript-eslint is in play, the linting rule could verify the type of a hook parameter to be the proper DependencyList from React's typings; but it doesn't have to be that complicated. The rule could also check simply for the parameter name deps as is used by the built-in hooks. Whether something is a hook or not is already a name convention: if it starts with use the linting rule considers it a hook.