fix(clerk-react): Fix forceRedirectUrl and fallbackRedirectUrl when p…

…assed to button components (#3508) Co-authored-by: Lennart <lekoarts@gmail.com>
clerk · Jun 4, 2024 · 35a0015 · 35a0015
1 parent 94438d4
commit 35a0015
Show file tree

Hide file tree

Showing 9 changed files with 203 additions and 14 deletions.
diff --git a/.changeset/metal-foxes-raise.md b/.changeset/metal-foxes-raise.md
@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-react': patch
+---
+
+Update `SignUpButton` and `SignInButton` to respect `forceRedirect` and `fallbackRedirect` props. Previously, these were getting ignored and successful completions of the flows would fallback to the default redirect URL.
diff --git a/.changeset/sixty-ears-rest.md b/.changeset/sixty-ears-rest.md
@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-js': patch
+---
+
+Fixed a bug where Clerk components rendered in modals were wrapped with `aria-hidden`.
diff --git a/integration/templates/next-app-router/src/app/buttons/page.tsx b/integration/templates/next-app-router/src/app/buttons/page.tsx
@@ -0,0 +1,35 @@
+import { SignInButton, SignUpButton } from '@clerk/nextjs';
+
+export default function Home() {
+  return (
+    <main>
+      <SignInButton
+        mode='modal'
+        forceRedirectUrl='/protected'
+      >
+        Sign in button (force)
+      </SignInButton>
+
+      <SignInButton
+        mode='modal'
+        fallbackRedirectUrl='/protected'
+      >
+        Sign in button (fallback)
+      </SignInButton>
+
+      <SignUpButton
+        mode='modal'
+        forceRedirectUrl='/protected'
+      >
+        Sign up button (force)
+      </SignUpButton>
+
+      <SignUpButton
+        mode='modal'
+        fallbackRedirectUrl='/protected'
+      >
+        Sign up button (fallback)
+      </SignUpButton>
+    </main>
+  );
+}
diff --git a/integration/tests/redirects.test.ts b/integration/tests/redirects.test.ts
@@ -0,0 +1,130 @@
+import { test } from '@playwright/test';
+
+import { appConfigs } from '../presets';
+import type { FakeUser } from '../testUtils';
+import { createTestUtils, testAgainstRunningApps } from '../testUtils';
+
+testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('redirect props @nextjs', ({ app }) => {
+  test.describe.configure({ mode: 'serial' });
+
+  let fakeUser: FakeUser;
+
+  test.beforeAll(async () => {
+    const u = createTestUtils({ app });
+    fakeUser = u.services.users.createFakeUser({
+      fictionalEmail: true,
+      withPhoneNumber: true,
+      withUsername: true,
+    });
+    await u.services.users.createBapiUser(fakeUser);
+  });
+
+  test.afterAll(async () => {
+    await fakeUser.deleteIfExists();
+    await app.teardown();
+  });
+
+  test.afterEach(async ({ page, context }) => {
+    const u = createTestUtils({ app, page, context });
+    await u.page.signOut();
+    await u.page.context().clearCookies();
+  });
+
+  test.describe('SignInButton', () => {
+    test('sign in button respects forceRedirectUrl', async ({ page, context }) => {
+      const u = createTestUtils({ app, page, context });
+
+      await u.page.goToRelative('/buttons');
+      await u.page.waitForClerkJsLoaded();
+      await u.po.expect.toBeSignedOut();
+
+      await u.page.getByText('Sign in button (force)').click();
+
+      await u.po.signIn.waitForMounted();
+      await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password });
+
+      await u.page.waitForAppUrl('/protected');
+
+      await u.po.expect.toBeSignedIn();
+    });
+
+    test('sign in button respects fallbackRedirectUrl', async ({ page, context }) => {
+      const u = createTestUtils({ app, page, context });
+
+      await u.page.goToRelative('/buttons');
+      await u.page.waitForClerkJsLoaded();
+      await u.po.expect.toBeSignedOut();
+
+      await u.page.getByText('Sign in button (fallback)').click();
+
+      await u.po.signIn.waitForMounted();
+      await u.po.signIn.signInWithEmailAndInstantPassword({ email: fakeUser.email, password: fakeUser.password });
+
+      await u.page.waitForAppUrl('/protected');
+
+      await u.po.expect.toBeSignedIn();
+    });
+  });
+
+  test.describe('SignUpButton', () => {
+    test('sign up button respects forceRedirectUrl', async ({ page, context }) => {
+      const u = createTestUtils({ app, page, context });
+      const fakeUser = u.services.users.createFakeUser({
+        fictionalEmail: true,
+        withPhoneNumber: true,
+        withUsername: true,
+      });
+
+      await u.page.goToRelative('/buttons');
+      await u.page.waitForClerkJsLoaded();
+
+      await u.page.getByText('Sign up button (force)').click();
+
+      // Fill in sign up form
+      await u.po.signUp.signUpWithEmailAndPassword({
+        email: fakeUser.email,
+        password: fakeUser.password,
+      });
+
+      // Verify email
+      await u.po.signUp.enterTestOtpCode();
+
+      await u.page.waitForAppUrl('/protected');
+
+      // Check if user is signed in
+      await u.po.expect.toBeSignedIn();
+
+      await fakeUser.deleteIfExists();
+    });
+
+    test('sign up button respects fallbackRedirectUrl', async ({ page, context }) => {
+      const u = createTestUtils({ app, page, context });
+      const fakeUser = u.services.users.createFakeUser({
+        fictionalEmail: true,
+        withPhoneNumber: true,
+        withUsername: true,
+      });
+
+      await u.page.goToRelative('/buttons');
+      await u.page.waitForClerkJsLoaded();
+
+      await u.page.getByText('Sign up button (fallback)').click();
+
+      // Fill in sign up form
+      await u.po.signUp.signUpWithEmailAndPassword({
+        email: fakeUser.email,
+        password: fakeUser.password,
+      });
+
+      // Verify email
+      await u.po.signUp.enterTestOtpCode();
+
+      await u.page.waitForAppUrl('/protected');
+
+      // Check if user is signed in
+      await u.po.expect.toBeSignedIn();
+
+      await fakeUser.deleteIfExists();
+    });
+  });
+});
diff --git a/packages/clerk-js/src/ui/elements/Modal.tsx b/packages/clerk-js/src/ui/elements/Modal.tsx
@@ -50,15 +50,13 @@ export const Modal = withFloatingTree((props: ModalProps) => {
     >
       <ModalContext.Provider value={modalCtx}>
         <Flex
-          aria-hidden
           ref={overlayRef}
           elementDescriptor={descriptors.modalBackdrop}
           sx={[
             t => ({
               animation: `${animations.fadeIn} 150ms ${t.transitionTiming.$common}`,
               zIndex: t.zIndices.$modal,
               backgroundColor: t.colors.$modalBackdrop,
-              // ...common.centeredFlex(),
               alignItems: 'flex-start',
               justifyContent: 'center',
               overflow: 'auto',

diff --git a/packages/react/src/components/SignInButton.tsx b/packages/react/src/components/SignInButton.tsx
@@ -1,3 +1,4 @@
+import type { SignInProps } from '@clerk/types';
 import React from 'react';
 
 import type { SignInButtonProps, WithClerkProp } from '../types';
@@ -11,21 +12,27 @@ export const SignInButton = withClerk(({ clerk, children, ...props }: WithClerkP
   const child = assertSingleChild(children)('SignInButton');
 
   const clickHandler = () => {
-    const opts = {
+    const opts: SignInProps = {
+      forceRedirectUrl,
+      fallbackRedirectUrl,
       signUpFallbackRedirectUrl,
       signUpForceRedirectUrl,
-      signInForceRedirectUrl: forceRedirectUrl,
-      signInFallbackRedirectUrl: fallbackRedirectUrl,
     };
 
     if (mode === 'modal') {
       return clerk.openSignIn(opts);
     }
-    return clerk.redirectToSignIn(opts);
+    return clerk.redirectToSignIn({
+      ...opts,
+      signInFallbackRedirectUrl: fallbackRedirectUrl,
+      signInForceRedirectUrl: forceRedirectUrl,
+    });
   };
 
   const wrappedChildClickHandler: React.MouseEventHandler = async e => {
-    await safeExecute((child as any).props.onClick)(e);
+    if (child && typeof child === 'object' && 'props' in child) {
+      await safeExecute(child.props.onClick)(e);
+    }
     return clickHandler();
   };
 

diff --git a/packages/react/src/components/SignUpButton.tsx b/packages/react/src/components/SignUpButton.tsx
@@ -1,3 +1,4 @@
+import type { SignUpProps } from '@clerk/types';
 import React from 'react';
 
 import type { SignUpButtonProps, WithClerkProp } from '../types';
@@ -19,9 +20,9 @@ export const SignUpButton = withClerk(({ clerk, children, ...props }: WithClerkP
   const child = assertSingleChild(children)('SignUpButton');
 
   const clickHandler = () => {
-    const opts = {
-      signUpFallbackRedirectUrl: fallbackRedirectUrl,
-      signUpForceRedirectUrl: forceRedirectUrl,
+    const opts: SignUpProps = {
+      fallbackRedirectUrl,
+      forceRedirectUrl,
       signInFallbackRedirectUrl,
       signInForceRedirectUrl,
       unsafeMetadata,
@@ -31,11 +32,17 @@ export const SignUpButton = withClerk(({ clerk, children, ...props }: WithClerkP
       return clerk.openSignUp(opts);
     }
 
-    return clerk.redirectToSignUp(opts);
+    return clerk.redirectToSignUp({
+      ...opts,
+      signUpFallbackRedirectUrl: fallbackRedirectUrl,
+      signUpForceRedirectUrl: forceRedirectUrl,
+    });
   };
 
   const wrappedChildClickHandler: React.MouseEventHandler = async e => {
-    await safeExecute((child as any).props.onClick)(e);
+    if (child && typeof child === 'object' && 'props' in child) {
+      await safeExecute(child.props.onClick)(e);
+    }
     return clickHandler();
   };
 

diff --git a/packages/react/src/components/__tests__/SignInButton.test.tsx b/packages/react/src/components/__tests__/SignInButton.test.tsx
@@ -52,7 +52,7 @@ describe('<SignInButton/>', () => {
     const btn = screen.getByText('Sign in');
     await userEvent.click(btn);
 
-    expect(mockRedirectToSignIn).toHaveBeenCalledWith({ signInForceRedirectUrl: url });
+    expect(mockRedirectToSignIn).toHaveBeenCalledWith({ forceRedirectUrl: url, signInForceRedirectUrl: url });
   });
 
   it('handles fallbackRedirectUrl prop', async () => {
@@ -62,6 +62,7 @@ describe('<SignInButton/>', () => {
     await userEvent.click(btn);
 
     expect(mockRedirectToSignIn).toHaveBeenCalledWith({
+      fallbackRedirectUrl: url,
       signInFallbackRedirectUrl: url,
     });
   });

diff --git a/packages/react/src/components/__tests__/SignUpButton.test.tsx b/packages/react/src/components/__tests__/SignUpButton.test.tsx
@@ -53,7 +53,7 @@ describe('<SignUpButton/>', () => {
     const btn = screen.getByText('Sign up');
     userEvent.click(btn);
     await waitFor(() => {
-      expect(mockRedirectToSignUp).toHaveBeenCalledWith({ signUpForceRedirectUrl: url });
+      expect(mockRedirectToSignUp).toHaveBeenCalledWith({ forceRedirectUrl: url, signUpForceRedirectUrl: url });
     });
   });
 
@@ -63,6 +63,7 @@ describe('<SignUpButton/>', () => {
     userEvent.click(btn);
     await waitFor(() => {
       expect(mockRedirectToSignUp).toHaveBeenCalledWith({
+        fallbackRedirectUrl: url,
         signUpFallbackRedirectUrl: url,
       });
     });