Skip to content
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

When used with react strict mode, the realtime database does not subscribe properly. #169

Closed
Jannchie opened this issue Jul 20, 2022 · 17 comments
Labels
bug Something isn't working

Comments

@Jannchie
Copy link

Jannchie commented Jul 20, 2022

Bug report

Describe the bug

When we develop with the React strict mode, we can't build subscriptions correctly.

To Reproduce

Turn on strict mode:

ReactDOM.createRoot(document.getElementById("root") as HTMLElement).render(
  <React.StrictMode>
    <App />
  </React.StrictMode>
);

Using subscriptions in components:

function App() {
  useEffect(() => {
    const subs = supabase
      .from("objects")
      .on("UPDATE", (payload) => {
        dispatch(updateGameObject(payload.new.data));
      })
      .subscribe(console.log);
    subs.onClose(() => {
      console.log("on closed");
      supabase.removeAllSubscriptions();
    });
    return () => {
      subs.unsubscribe();
    };
  }, []);
  return <div />
}

Expected behavior

In strict mode, useEffect is triggered twice. In theory, it should quickly establish and close a connection once, and then establish it again.

However, in practice, it is not possible to subscribe properly. In strict mode, subscribe() will receive the CLOSED signal directly at the first time.

Screenshots

The output in the console is as follows:

image

System information

  • OS: Windows
  • Browser (if applies): Edge
  • Version of supabase-js: ^1.35.4
  • Version of Node.js: 16
  • Version of react: 18

Additional context

In supabase/supabase#7771 it is suggested that turning on strict mode can cause problems.

@Jannchie Jannchie added the bug Something isn't working label Jul 20, 2022
@kristianeboe
Copy link

Is this with React 18? React 18's Strict mode has been nothing but trouble for me since it started running useEffect twice in localhost

@Jannchie
Copy link
Author

Yes, I'm using React 18. The above code should work fine if useEffect is called twice. But in reality, it does not.

@liambrewer
Copy link

Same issue here, Vite with React 18 and a brand new project. Strict Mode is enabled.

useEffect(() => {
    const sub = supabase
      .from('message')
      .on('*', (message) => {
        console.log('New Message');
        handleNewMessage(message.new);
      })
      .subscribe();
    return () => {
      supabase.removeSubscription(sub);
    };
  }, []);

@kiwicopple
Copy link
Member

got it! Sending this to the realtime-js repo 👍

@kiwicopple kiwicopple transferred this issue from supabase/supabase Jul 21, 2022
@ru88s
Copy link

ru88s commented Jul 27, 2022

In my case, I upgraded to Next.js v12.2.3 and it solved the problem.
Release v12.2.3 · vercel/next.js

@w3b6x9
Copy link
Member

w3b6x9 commented Aug 12, 2022

Closing this as it's no longer an issue with a later version of next js.

@w3b6x9 w3b6x9 closed this as completed Aug 12, 2022
@w3b6x9 w3b6x9 unpinned this issue Aug 12, 2022
@edgarsilva
Copy link

This is still an issue with NextJS v12.2.5 and supabase-js v1.35.6

@w3b6x9 w3b6x9 reopened this Aug 17, 2022
@w3b6x9 w3b6x9 pinned this issue Aug 17, 2022
@edgarsilva
Copy link

Using this workaround, for the time being, solves the issue =>

useEffect(() => {
  // This is a hack for React >= v18 in Strict mode
  // ensures to always call on the last  mount and
  // correctly remove subscription on unmount
  let subscription = null;
  const timer = setTimeout(() => subscription = subscribeToTable(), 1000);

  function subscribeToTable() {
    return (
      supabase
        .from("my_table")
        .on("*", (payload) => {
          console.log('Change received!', payload)
        })
        .subscribe((msg) => {
          if (msg === 'SUBSCRIBED') {
            isSubscribed.current = true;
          }
        })
    );
  }

  return () => {
    if (!subscription) {
      return clearTimeout(timer);
    }

    supabase.removeSubscription(subscription);
    isSubscribed.current = false;
  };
}, []);

@pixtron
Copy link

pixtron commented Aug 20, 2022

The issue seems to not be a react specific issue and can be observed in a vanilla typescript setup. See this minimal reproduction repro with supabase-js@2.0.0-rc.4.

As others noted useEffect gets called twice in React 18 development mode.

useEffect(() => {
  const channel = supabase
    .channel("db-changes")
    .on("postgres_changes", { event: "*", schema: "*" }, (payload: any) =>
      console.log("Postgres change", payload)
    );

  channel.subscribe();
	
  return () => {
    channel.unsubscribe()
  };
}, []);

Calling above useEffect twice translates to this code:

const channel1 = supabase
  .channel("db-changes")
  .on("postgres_changes", { event: "*", schema: "*" }, (payload: any) =>
    console.log("Postgres change channel 1", payload)
  );

channel1.subscribe();
channel1.unsubscribe();

const channel2 = supabase
  .channel("db-changes")
  .on("postgres_changes", { event: "*", schema: "*" }, (payload: any) =>
    console.log("Postgres change channel 2", payload)
  );

channel2.subscribe();

Obviously it does not make too much sense to write above code. But there are use cases where exactly this will happen. For example when there are two components "Top Upvoted Posts" and "Latest Posts" on a page. When each of these two components subscribe to the postgres-changes for the table posts we got exactly this scenario.

Running this code, the server still sends the change events but neither channel1 nor channel2 receive those events.
Also it does not make a difference if channel1.unsubscribe() is called or not. realtime-js always sends a phx-leave to the server.

Most of the time the phx-leave is acknowledged:

sequence-1

Sometimes the phx-leave is not acknowledged:

sequence-2

Regardless if the phx-leave is acknowledged or not, realtime-js stops to push the incoming events forward to the subscribers.

@w3b6x9
Copy link
Member

w3b6x9 commented Aug 22, 2022

Hey everyone, I have fixed this issue in supabase-js v2.0.0-rc.5. Please do a version bump.

Also, please use either client.removeChannel(chan) or client.removeChannels() for proper cleanup.

@w3b6x9 w3b6x9 closed this as completed Aug 22, 2022
@w3b6x9 w3b6x9 unpinned this issue Aug 22, 2022
@chiubaca
Copy link

chiubaca commented Aug 23, 2022

Hey @w3b6x9 , I tried upgrading to v2.0.0-rc.5 and I'm still facing the issue reported by OP.

I've described my personal implementation of how to handle the double useEffect firing and issues with cleanup in my discussion post here along with a stackblitz to repro.

@w3b6x9
Copy link
Member

w3b6x9 commented Aug 23, 2022

@chiubaca just wanted to check if you tried it with cleanup and removeChannel instead of unsubscribe?

@pixtron
Copy link

pixtron commented Aug 23, 2022

@chiubaca i forked your stackblitz and did some changes, now it's working. Problem was that the useEffect destructor got called immediately and the channel has been removed, but your ref has not been reset.
https://stackblitz.com/edit/nextjs-yqe7rd?file=pages%2Fsend-broadcast.tsx

The flow of the double called useEffect is this:

  1. enter useEffect
  2. exit (destructor gets called and removes the channel)
  3. enter useEffect again (channel is not re-created as channelRef.current is not null.

Keep in mind the double call to useEffect is only during development. Although this might happen in production for example if a user switches pages fast (eg: A->B->A).

@w3b6x9 i noticed another thing:
if i pass config: { broadcast: { ack: true }} the default self: false disappears.
image

Merging with spread operators does only a shallow merge. If config.broadcast is set it will override the defaults for broadcast.

this.params.config = {
...{
broadcast: { ack: false, self: false },
presence: { key: '' },
},
...params.config,
}

@w3b6x9
Copy link
Member

w3b6x9 commented Aug 23, 2022

if i pass config: { broadcast: { ack: true }} the default self: false disappears.

@pixtron that's fine as the Realtime server will take the absence to mean self is false but i'll go back and make the default config better. thanks for pointing this out.

@chiubaca
Copy link

thanks so much for taking the to fix my stackblitz example @pixtron. And your explanation reassured me that my mental model of the double useEffect in React 18 was correct.

I was wondering if the setInterval was really necessary in your fork and was able to refactor it to be even simpler. I completely removed the if (channelRef.current === null) check and the ref entirely and ensured to cleanup with removeChannel as per @w3b6x9 and it just works now. It's pretty much a copy and paste of the demo code in the broadcast guide now 🙌🏼

@pixtron
Copy link

pixtron commented Aug 24, 2022

@chiubaca i just cleared the interval in my fork. If you don't clear it. the interval runs forever. Eg if you go to "Send broadcast" and then after ~4s "back to home" you will still see an ok logged every two seconds even though the channel has been already removed from supabase client. This might end up in a memory leak as channel can't be garbage collected as long it is still used in the interval.

Maybe it makes sense to move this conversation over to discord - feel free to ping me there if you have any further questions.

@ricardosikic
Copy link

I have removed <ReactStrictMode> and problem solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

10 participants