markjaquith / clerk-sveltekit

Clerk adapter for SvelteKit
Other
128 stars 19 forks source link

[Bug] Regression since 0.3.0 - Clerk store needs to be export to allow for programatic e2e testing #36

Closed thebrianbug closed 5 months ago

thebrianbug commented 7 months ago

Background

To be able to be used in serious applications, users should be able to programmatically sign in. This can be useful both for customizing sign-in flows, but (more imporatantly for me) to also to allow for reliable e2e testing.

The way that Clerk suggests to support this is by exporting the clerk instance to the window object. I can achieve this in v0.2.0 with the code below:

hooks.client.ts

import type { HandleClientError } from '@sveltejs/kit';
import { PUBLIC_CLERK_PUBLISHABLE_KEY } from '$env/static/public';
import { initializeClerkClient, clerk } from 'clerk-sveltekit/client';

initializeClerkClient(PUBLIC_CLERK_PUBLISHABLE_KEY, {
  afterSignInUrl: '/',
  afterSignUpUrl: '/',
  signInUrl: '/sign-in',
  signUpUrl: '/sign-up'
});

clerk.subscribe((clerkInstance) => {
  if (clerkInstance) window.Clerk = clerkInstance;
});

export const handleError: HandleClientError = async ({ error, event }) => {
  console.error(error, event);
};

This allows me to write a programmatic sign-in helper like the one below. I wish that the Clerk team would provide a more reliable programmatic sign-in method, but for now, this is what they support.

import type { Page } from '@playwright/test';
import { expect, test } from '@playwright/test';

const DEFAULT_TEST_EMAIL_SUFFIX = // Omitted
const DEFAULT_TEST_CODE = '424242';

export async function signIn(page: Page) {
  await page.goto('/sign-in', {
    waitUntil: 'networkidle',
    timeout: 60000
  });

  await test.step('Check if Clerk is ready', async () => {
    await page.waitForFunction(
      () => {
        if (typeof window.Clerk === 'undefined') {
          throw new Error('Window does not have Clerk property.');
        }
        return window.Clerk.isReady();
      },
      { timeout: 7_000 }
    ); // Set the timeout as needed
  });

  // Clear cookies for the current domain
  await page.context().clearCookies();

  // Sign in using Clerk
  await test.step('Sign in with Clerk', async () => {
    await expect(async () => {
      const sessionId = await page.evaluate(
        async (params: { email: string; passCode: string }) => {
          const signIn = window.Clerk?.client?.signIn;

          const signInResp = await signIn?.create({
            identifier: params.email
          });
          const firstFactorEmailCode = signInResp?.supportedFirstFactors?.find(
            (ff) => ff.strategy === 'email_code' && ff.safeIdentifier === params.email
          );

          if (!firstFactorEmailCode || firstFactorEmailCode.strategy !== 'email_code') {
            throw new Error('Email code login factor not found.');
          }

          await signIn?.prepareFirstFactor({
            strategy: 'email_code',
            emailAddressId: firstFactorEmailCode.emailAddressId
          });

          const attemptResponse = await signIn?.attemptFirstFactor({
            strategy: 'email_code',
            code: params.passCode
          });

          if (attemptResponse?.status !== 'complete') {
            throw new Error('Sign in code attempt failed');
          }

          return attemptResponse?.createdSessionId;
        },
        {
          email: `${DEFAULT_TEST_EMAIL_SUFFIX}`,
          passCode: DEFAULT_TEST_CODE
        }
      );

      if (!sessionId) {
        throw new Error('Sign in attempt failed');
      }

      await page.evaluate((sessionId) => {
        return window.Clerk?.setActive({
          session: sessionId
        });
      }, sessionId);
    }).toPass({
      // Probe, wait, probe...
      intervals: [7_000, 10_000, 15_000],
      timeout: 25_000
    });
  });
}

export async function signOutClearCookies(page: Page) {
  await page.context().clearCookies();
}

Let's bring back the exported clerk variable, and also consider adding a test that verifies that is is exported properly for use in e2e tests. I would also include the setting of the Clerk store in the window object in hooks.client.ts in the package documentation.

Request

Restore the external export of the Clerk store, which was removed here.

markjaquith commented 6 months ago

Yep, agree with this!

But related to programmatic sign-in, did you see how I implemented it in the e2e tests in this repo? Not sure if that is useful to you.

markjaquith commented 6 months ago

I would also include the setting of the Clerk store in the window object in hooks.client.ts in the package documentation.

Hmm. Maybe we should even do this ourselves... is there ever a case where you would not want window.Clerk to be available?

thebrianbug commented 6 months ago

No, I can't think of any. Others may come up with a situation, but I wouldn't go without it personally.

markjaquith commented 6 months ago

I don't think a change needs to be made here. I'm already setting window.Clerk() inside initializeClerkClient(), exactly the way you were.

thebrianbug commented 6 months ago

Interesting... it's not being set as a global for me. I'll try again and see what's going on, maybe there's a difference in access to window object from a package.

thebrianbug commented 5 months ago

Closing, I've confirmed that in 0.3.0 we are still exporting Clerk to the window object, so I don't need access to the store itself. Others might, but this works for my needs