-
Notifications
You must be signed in to change notification settings - Fork 252
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
feat(test): Add Clerk Elements E2E #3394
feat(test): Add Clerk Elements E2E #3394
Conversation
🦋 Changeset detectedLatest commit: ff3e23f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
edfefd2
to
4ddd59a
Compare
428a6d0
to
ab29eff
Compare
* Allowing 10^5 combinations should be enough entropy for e2e purposes. | ||
* @see https://clerk.com/docs/testing/e2e-testing#phone-numbers | ||
*/ | ||
function fakerPhoneNumber() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
}, | ||
waitForMounted: () => { | ||
return page.waitForSelector('.cl-signIn-root', { state: 'attached' }); | ||
waitForMounted: (selector = '.cl-signIn-root') => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
}, | ||
"dependencies": { | ||
"@clerk/elements": "file:../../../packages/elements", | ||
"@clerk/nextjs": "file:../../../packages/nextjs", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
☁️ I think we should drop the 2 lines above. We are not passing the deps added in presets in the package.json of the template in the other integration tests.
cc: @nikosdouvlis
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way you can run the template locally just fine without needing to install them first. With .addDependency
these entries are overwritten
"@types/node": "^18.17.0", | ||
"@types/react": "^18.3.1", | ||
"@types/react-dom": "^18.3.0", | ||
"next": "^14.2.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ shouldn't we use the 13.5.4
since it's the minimum supported version defined in elements peer dependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be handled with something like #3431
export default function RootLayout({ children }: { children: React.ReactNode }) { | ||
return ( | ||
<html lang='en'> | ||
<ClerkProvider clerkJSVariant='headless'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ Why headless
? Is this the suggested approach when using the elements package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now the integration tests don't need the AIO components and eventually people should use headless, yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice !
Description
This PR adds Playwright E2E tests for Clerk Elements. Included are:
next-sign-in.test.ts
- Mirrors the existingsign-in-flow.test.ts
file to assert that one can successfully sign innext-sign-up.test.ts
- Mirrors the existingsign-up-flow.test.ts
file to assert that one can successfully sign upotp.test.ts
- Assert that<Input type="otp" />
works correctlyvalidate-password.test.ts
- Assert that<Input type="password" validatePassword />
works correctlyMoreover, this PR also fixes this Faker.js deprecation message:
This issue explains how to use
replaceSymbolWithNumber
but that's also deprecated and its docs explain what to use instead.Fixes SDK-1721
Fixes SDK-1382
Checklist
npm test
runs as expected.npm run build
runs as expected.Type of change