<SignIn> Component Not Rendering After Signing Out When Component Has Appearance Props Added

brettabamonte commented 1 year ago

Preliminary Checks

[X] I have reviewed the documentation: https://clerk.com/docs
[X] I have searched for existing issues: https://github.com/clerkinc/javascript/issues
[X] I have not already reached out to Clerk support via email or Discord (if you have, no need to open an issue here)
[X] This issue is not a question, general help request, or anything other than a bug report directly related to Clerk. Please ask questions in our Discord community: https://clerk.com/discord.

Reproduction / Replay Link

https://github.com/brettabamonte/Clerk-Minimal_Reproduction-SignIn_Component/tree/main

Publishable key

pk_test_Y2hpZWYtc2N1bHBpbi0xMi5jbGVyay5hY2NvdW50cy5kZXYk

Description

Steps to reproduce:

See steps and screenshots in linked git repository README.md (https://github.com/brettabamonte/Clerk-Minimal_Reproduction-SignIn_Component).

Expected behavior: After clicking the the user should be redirected to the sign in page with a component displaying that has appearance props.

Actual behavior: After clicking the the user is redirected to the sign in page but the component does not render when it has the appearance prop set. If the appearance prop is set the component displays properly.

Environment

System:
    OS: macOS 14.0
    CPU: (8) arm64 Apple M2
    Memory: 238.50 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.18.1 - /usr/local/bin/node
    npm: 9.8.1 - /usr/local/bin/npm
  Browsers:
    Chrome: 118.0.5993.117
    Safari: 17.0
  npmPackages:
    @clerk/nextjs: ^4.26.1 => 4.26.1 
    @types/node: 20.1.4 => 20.1.4 
    @types/react: 18.2.6 => 18.2.6 
    @types/react-dom: 18.2.4 => 18.2.4 
    autoprefixer: 10.4.14 => 10.4.14 
    classnames: ^2.3.2 => 2.3.2 
    next: ^14.0.0 => 14.0.0 
    postcss: 8.4.23 => 8.4.23 
    react: 18.2.0 => 18.2.0 
    react-dom: 18.2.0 => 18.2.0 
    tailwindcss: 3.3.2 => 3.3.2 
    typescript: 5.0.4 => 5.0.4

tjl-jg commented 1 year ago

I have an issue that sounds very similar, but with the <UserButton />. After signing out, and signing back in, it renders at size 0x0.

Not sure if it's related or not, but the browser console is showing a couple of errors related to signin. The stack trace is to a packed vendors file, so the line numbers probably aren't relevant, but the error is "Uncaught TypeError: Cannot read properties of undefined (reading 'imageUrl')" and the line of code sets avatarUrl with l.userData.imageUrl, where l is a minified variable name.

All that to say that it looks like it is somehow trying to set the avatar with the imageUrl of a userData that hasn't been initialized yet.

IGassmann commented 1 year ago

I can confirm this also happens on @clerk/nextjs: 4.27.1/next: 14.0.2. However, it happens for us independently of the use of the appearance prop

LekoArts commented 12 months ago

Hello all!

Just a short update here to show that I/we haven't forgotten this issue 😅 Thanks for the reproduction, I was able to also see the issue. Important bit: This also happens without the appearance prop. After investigating I'm still not sure exactly what is causing it, might be a race condition or an issue with client side re-hydration.

We'll discuss internally a bit how we want to tackle this but I'd be interested in your opinion on this question (@brettabamonte @tjl-jg @IGassmann):

What is your expected behavior for the <SignOutButton /> as to where it should navigate? Should it stay on the same page? Navigate to the sign in page? Navigate to a custom page one has to define?

tjl-jg commented 12 months ago

My opinion about redirect options: I prefer full context to be in my code, so I'd probably use a property specifying the redirect destination even if my destination was the default. This way it shows up in searches of the project, and the action is clear later on (or to other developers) without anybody having to read documentation.

brettabamonte commented 11 months ago

@LekoArts I agree with @tjl-jg. I like the context to be within the component using a property. Having a default destination is nice if the property isn't declared on the component. 90% of the time, I like redirecting users to a default page like the sign in page but there are instances where it'd be nice to redirect them to a custom destination.

Thank you and the team for looking into this!

IGassmann commented 11 months ago

@LekoArts I agree with @tjl-jg. I like the context to be within the component using a property. Having a default destination is nice if the property isn't declared on the component. 90% of the time, I like redirecting users to a default page like the sign in page but there are instances where it'd be nice to redirect them to a custom destination.

Thank you and the team for looking into this!

I agree. Additionally, there may be situations where redirection is not desired. Consider an app that functions both when signed in and when signed out, where being signed in is not essential for using the app.

LekoArts commented 11 months ago

Thanks for your input!

Then you can unblock yourself by using the signOutCallback prop on the <SignOutButton /> component:

"use client"

import { useRouter } from "next/navigation"
import { SignOutButton } from "@clerk/nextjs";

export const Signout = () => {
  const router = useRouter()

  return (
    <SignOutButton signOutCallback={() => router.push('/your-path')}>
      <button>Sign Out</button>
    </SignOutButton>
  )
}

Or if you want to reload the page (you'd want to make that page public then though):

"use client"

import { SignOutButton } from "@clerk/nextjs";

export const Signout = () => {
  return (
    <SignOutButton signOutCallback={() => window.location.reload()}>
      <button>Sign Out</button>
    </SignOutButton>
  )
}

@brettabamonte What your reproduction shows is in some sense "expected" behavior that is described in this thread here. The blank page shows because it shows the unauthenticated state of the /dashboard route which will be empty. The behavior you want kicks in because the middleware redirects to the sign-in page because /dashboard is not a public route. With the callback prop above you can reliably do whatever you want after the signout 👍

We'll still look into the underlying issue and see if we'd need to change something DX-wise.

Sengulair commented 7 months ago

I have the same problem as @tjl-jg. Here is a minimal reproduction by the way: https://github.com/Sengulair/clerk-nextjs-app-issue/commit/f7a29db4ce18ac149714b72f91afff578ddeb677 It still appears on the 5.0.1 version. Do we have some workaround for that case?

panteliselef commented 7 months ago

@Sengulair mind giving this another try with 5.0.4

Sengulair commented 7 months ago

@panteliselef I've updated the reproduction to 5.0.4. Still failing with the same errors

Sengulair commented 7 months ago

@panteliselef Tested with 5.0.6 and seems the problem is resolved.

dimkl commented 6 months ago

Closing this as it seems resolved. @brettabamonte If the issue persists after using a version >= 5.0.6, feel free to re-open the issue.

clerk / javascript