CSFrequency / react-firebase-hooks

React Hooks for Firebase.
Apache License 2.0
3.55k stars 304 forks source link

`unsubscribe` method is not called if users reload the page #292

Closed 9sako6 closed 10 months ago

9sako6 commented 1 year ago

First off, thanks for awsome react-firebase-hooks!

In the current implementation, unsubscribe method is set as useEffect’s return value. So, the unsubscribe method will not be called if users reload the page using the browser's reload button or navigate to another page using the back button. Thus, unused subscriptions remain, and it seems that Firestore continues to send requests to them.

How about calling the unsubscribe method in beforeunload event handler? Here is an example of a naive implementation.

diff --git a/firestore/useCollection.ts b/firestore/useCollection.ts
index 48084d0..f46224e 100644
--- a/firestore/useCollection.ts
+++ b/firestore/useCollection.ts
@@ -8,6 +8,7 @@ import {
   Query,
   QuerySnapshot,
   SnapshotOptions,
+  Unsubscribe,
 } from 'firebase/firestore';
 import { useCallback, useEffect, useMemo } from 'react';
 import { useLoadingValue } from '../util';
@@ -26,6 +27,8 @@ import {
   Options,
 } from './types';

+const unsubscribes: Unsubscribe[] = [];
+
 export const useCollection = <T = DocumentData>(
   query?: Query<T> | null,
   options?: Options
@@ -50,11 +53,18 @@ export const useCollection = <T = DocumentData>(
         )
       : onSnapshot(ref.current, setValue, setError);

+    unsubscribes.push(unsubscribe)
     return () => {
       unsubscribe();
     };
   }, [ref.current]);

+  useEffect(() => {
+    window.addEventListener('beforeunload', () => {
+      unsubscribes.forEach(unsubscribe => unsubscribe())
+    });
+  }, [])
+
   return [value as QuerySnapshot<T>, loading, error];
 };
0x80 commented 10 months ago

I think you might have misunderstood the useEffect hook. The return function is meant to be called by React during cleanup. What makes you think that it won't be called when you navigate away?

And I think a page reload will toss out the whole javascript context, so everything including subscriptions is garbage collected.

9sako6 commented 10 months ago

@0x80

Thanks for the reply.

You are correct, the cleanup function is called when the component is unmounted. However, my understanding is that the cleanup function is not called when the page is reloaded or the tab is closed.

Below is a simple demonstration video to show this. https://github.com/9concepts/nextjs-cleanup-experiment

I could not immediately provide a demo that stays connected to Firestore when using react-firebase-hooks. I don't see any other similar reports and will close the issue until I can show an example where the connection is kept alive.

0x80 commented 10 months ago

AFAIK when a tab is closed or reloaded, nothing of the current javascript runtime survives. I would be surprised if javascript kept executing in the background. I think every tab has an isolated runtime environment. It's like closing your nodejs application on the command line, it kills everything in terms of processes and allocated memory.

I don't think technically javascript even can call the unsubscribe in such situations, because objects have no destructor like in c++.