auth-js icon indicating copy to clipboard operation
auth-js copied to clipboard

auth.onAuthStateChanged triggers on tab switch

Open noty-nick opened this issue 3 years ago • 4 comments

Bug report

Describe the bug

auth.onAuthStateChanged triggers on tab switch

To Reproduce

  1. Create starter project using this guide https://supabase.com/docs/guides/with-vue-3

  2. add lines to src/main.js

import { supabase } from "./supabase";
supabase.auth.onAuthStateChange(console.log);
  1. run npm run dev
  2. open http://localhost:3000 in browser
  3. open dev console
  4. Switch to another tab
  5. Switch back to previous tab

Expected behavior

1 SIGNED_IN event in console

Actuall behaviour

2 SIGNED_IN event in console

Screenshots

https://www.loom.com/share/56993404e5f348d2bb23b68b0ad15096

System information

  • OS: macOS
  • Browser (if applies) brave
  • Version of supabase-js: 1.35.3
  • Version of Node.js: 16.14.0

noty-nick avatar May 14 '22 00:05 noty-nick

We've added functionality to recover the session when the page becomes visible. This was needed for example for mobile devices that have been in the background and need to recover the session and refresh the token when the page becomes visible.

Can you outline why receiving a SIGNED_IN event is problematic for you?

thorwebdev avatar May 17 '22 05:05 thorwebdev

When i receive this event in my app I show loader and fetch user data from backend. That's why it was problematic for me.

I've fixed it with createClient(supabaseUrl, supabaseKey, { ...otherOptions, multiTab: false })

My understanding is that recovering session does not mean that auth state changed, so it's confusing to receive SIGNED_IN event on tab change just because session was recovered.

@thorwebdev Thanks, and if this is expected behaviour please close the issue

noty-nick avatar May 17 '22 10:05 noty-nick

Same issue for me with another context.

To reproduce

  1. I've created a basic project with Next.js.
  2. I've added on top of _app.tsx a context like this one:
export const AuthProvider = ({ supabase, ...props }: any) => {
  const [session, setSession] = useState(supabase.auth.session());
  const [user, setUser] = useState(supabase.auth.user());
  const router = useRouter();

  useEffect(() => {
    const { data: authListener } = supabase.auth.onAuthStateChange(
      (event: any, currentSession: any) => {
        setSession(currentSession);
        setUser(currentSession?.user ?? null);
        handleAuthChange(event, currentSession);

        if (event === EVENTS.SIGNED_IN) {
          router.push("/courses");
        }
      }
    );

    return () => {
      authListener.unsubsribe();
    };
  }, []);

  const handleAuthChange = async (event: any, session: any) => {
    await fetch("/api/auth", {
      method: "POST",
      headers: { "Content-Type": "application/json" },
      mode: "same-origin",
      body: JSON.stringify({ event, session }),
    });
  };

  return (
    <AuthContext.Provider
      value={{
        session,
        user,
        signOut: () => supabase.auth.signOut(),
        signIn: () => supabase.auth.signIn({ provider: "google" }),
      }}
      {...props}
    />
  );
};

It was problematic for me because for each SIGNED_IN event I had a redirection.

I understand your point @thorwebdev and I've just moved my redirection out of this context. But I agree with @noty-nick. Having a SIGNED_IN event for this edge case with mobile and recovered session, it's a bit confusing.

Hope this feedback can help! Thanks!

JulClt avatar May 29 '22 11:05 JulClt

This fix did not work for me because I need to sync session between tabs (and I think most apps as well want this behaviour)

So my current workaround is a wrapper function that filters out unwanted events

export function onAuthStateChange(callback: (u: User | null) => void) {
  let currentSession: Session | null;
  supabase.auth.onAuthStateChange((event, session) => {
    if (session?.user?.id == currentSession?.user?.id) return;
    currentSession = session;
    callback(session);
  });
}

@thorwebdev I'd be happy to add this ability to gotrue-js, Just let me know if this is smth desirable and what should interface look like

noty-nick avatar May 30 '22 18:05 noty-nick

This is intentional behavior since tabs can be forgotten for a very long time in certain cases, and when you come back to the tab the user is effectively "signed-in" again.

hf avatar Dec 30 '22 17:12 hf

Even if it is effectively "signed-in" again, wouldn't it be more clear if we name this as a different event? Or at least have some way to distinguish between signing in via user action vs recovering a session

izhan avatar Jan 02 '23 02:01 izhan

This fix did not work for me because I need to sync session between tabs (and I think most apps as well want this behaviour)

So my current workaround is a wrapper function that filters out unwanted events

export function onAuthStateChange(callback: (u: User | null) => void) {
  let currentSession: Session | null;
  supabase.auth.onAuthStateChange((event, session) => {
    if (session?.user?.id == currentSession?.user?.id) return;
    currentSession = session;
    callback(session);
  });
}

@thorwebdev I'd be happy to add this ability to gotrue-js, Just let me know if this is smth desirable and what should interface look like

Where should we add this code if we are using SessionContextProvider from supabase? Since SessionContextProvider is already doing:

  useEffect(() => {
    const {
      data: { subscription }
    } = supabaseClient.auth.onAuthStateChange((event, session) => {
      if (session && (event === 'SIGNED_IN' || event === 'TOKEN_REFRESHED')) {
        setSession(session);
      }

      if (event === 'SIGNED_OUT') {
        setSession(null);
      }
    });

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

How do you "wrap" this function? Thanks!

ToniRV avatar Apr 13 '23 23:04 ToniRV

Why was this closed?

Intentional ≠ correct. As mentioned by others, it's a horrible UX on desktop. If you're fixing something to work on mobile but it breaks on desktop in the meantime, you're doing it wrong. Now all the components that rely on useUser() to do fetching and data presentation are re-rendered on refocus.

imageck avatar Jun 24 '23 09:06 imageck

Agreed this is a massive pain - changing tabs re-directs the user as if they just signed in.

gclcodeguy avatar Aug 07 '23 13:08 gclcodeguy

@ToniRV you can put it like this. It worked for me and you can check the browser console to check.

export function SupabaseProvider({ children }: { children: React.ReactNode; }) {
    const supabase = createPagesBrowserClient<Database>();
    const router = useRouter();

    const onAuthStateChange = (callback: (event : AuthChangeEvent) => void) => {
        let currentSession: Session | null;
        return supabase.auth.onAuthStateChange((event, session) => {
            if (session?.user?.id == currentSession?.user?.id) return;
            currentSession = session;
            callback(event);
        });
    }


    useEffect(() => {
        const { data: { subscription } } = onAuthStateChange((event : AuthChangeEvent) => {
            console.log(event)
            switch (event) {
                case 'SIGNED_OUT':
                    router.push('auth/login')
                    break;
                case 'SIGNED_IN' :
                    router.push('dashboard/home')
                    break;
                default:
                    // router.refresh()
                    break;
            }
        });

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

    return (
        <Context.Provider value={{ supabase }}>
            <>{children}</>
        </Context.Provider>
    )
}

aissamChia avatar Aug 17 '23 10:08 aissamChia

@ToniRV you can put it like this. It worked for me and you can check the browser console to check.

export function SupabaseProvider({ children }: { children: React.ReactNode; }) {
    const supabase = createPagesBrowserClient<Database>();
    const router = useRouter();

    const onAuthStateChange = (callback: (event : AuthChangeEvent) => void) => {
        let currentSession: Session | null;
        return supabase.auth.onAuthStateChange((event, session) => {
            if (session?.user?.id == currentSession?.user?.id) return;
            currentSession = session;
            callback(event);
        });
    }


    useEffect(() => {
        const { data: { subscription } } = onAuthStateChange((event : AuthChangeEvent) => {
            console.log(event)
            switch (event) {
                case 'SIGNED_OUT':
                    router.push('auth/login')
                    break;
                case 'SIGNED_IN' :
                    router.push('dashboard/home')
                    break;
                default:
                    // router.refresh()
                    break;
            }
        });

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

    return (
        <Context.Provider value={{ supabase }}>
            <>{children}</>
        </Context.Provider>
    )
}

Adding Clarity - Session is imported from Supabase import { Session } from "@supabase/supabase-js";

benjamin94 avatar Sep 15 '23 06:09 benjamin94