feat(clerk-js): Additional vitest specs #5716
Conversation
🦋 Changeset detectedLatest commit: 684c7bf The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis update migrates all Clerk.js tests and supporting utilities from the Jest testing framework to Vitest. It replaces Jest-specific mocks, spies, and test utilities with Vitest equivalents across all test files, updates test utility imports, and introduces new Vitest-based fixture and mock helper modules. The Vitest configuration is enhanced with SVG mocks, coverage reporting, and global constants. Several new utility modules are added for generating fixtures, mocking Clerk methods, and running fake timers within Vitest. Test assertions are updated to reflect a default localhost port of 3000. Environment setup is improved with locale, timezone, and browser API mocks to ensure consistent and reliable test execution. Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
jacekradko
force-pushed
the
feat/clerkjs-vitest
branch
from
076cc81 to
f5d7686
Compare
jacekradko
force-pushed
the
feat/clerkjs-vitest
branch
from
9a82b66 to
ef039ae
Compare
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
jacekradko
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🧹 Nitpick comments (18)
packages/clerk-js/src/core/resources/__tests__/Image.spec.ts (1)
15-20: Consider restoring the originalBaseResource._fetchafter tests. Overwriting the static_fetchmethod directly can leak into other tests. You may usevi.spyOn(BaseResource, '_fetch')along withvi.restoreAllMocks()in anafterEachhook, or capture and reassign the original method.packages/clerk-js/src/core/resources/__tests__/ExternalAccount.spec.ts (1)
15-15: Mocking_fetchonBaseResourcecould be scoped. Consider usingvi.spyOn(BaseResource, '_fetch')and restoring it post-test (vi.restoreAllMocks()or manual reassignment) to avoid cross-test side effects.packages/clerk-js/src/__tests__/mocks/svgMock.tsx (1)
4-9: Improve type safety and add displayName for better debugging.The component has a type mismatch - it accepts
React.SVGProps<SVGSVGElement>but renders as aspan. This could lead to type issues when SVG-specific props are passed.Apply this diff to improve type safety and debugging:
-const SvgMock = React.forwardRef<HTMLSpanElement, React.SVGProps<SVGSVGElement>>((props, ref) => ( +const SvgMock = React.forwardRef<HTMLSpanElement, React.HTMLAttributes<HTMLSpanElement>>((props, ref) => ( <span ref={ref} {...props} /> )); + +SvgMock.displayName = 'SvgMock';packages/clerk-js/src/ui/utils/vitest/runFakeTimers.ts (2)
1-1: Remove commented Jest import.The commented Jest import should be removed as it's no longer needed.
-// import { jest } from '@jest/globals';
6-11: Improve type safety in withAct function.The withAct function uses
anytypes which reduces type safety. Consider improving the typing.-const withAct = ((fn: any) => - (...args: any) => { +const withAct = <T extends (...args: any[]) => void>(fn: T): T => + ((...args: Parameters<T>) => { act(() => { fn(...args); }); - }) as WithAct; + }) as T;packages/clerk-js/src/ui/utils/vitest/mockHelpers.ts (2)
1-1: Remove commented out Jest import.Clean up the leftover Jest import comment since the migration to Vitest is complete.
-// import { jest } from '@jest/globals';
59-60: Clarify the purpose of the comment about touch mock.The comment "Ensure touch mock matches expected signature if available, otherwise basic mock" seems incomplete or unclear about what the expected signature should be.
Consider updating the comment to be more specific:
- // Ensure touch mock matches expected signature if available, otherwise basic mock + // Mock the touch method to return a resolved promise with the sessionpackages/clerk-js/vitest.config.mts (1)
33-49: Consider adding more specific exclusions to coverage configuration.The coverage exclusions list is comprehensive, but you might want to exclude additional patterns like mock files and test utilities.
Add these exclusions to prevent test utilities from affecting coverage metrics:
exclude: [ 'src/**/*.d.ts', 'src/**/index.ts', 'src/**/index.browser.ts', 'src/**/index.headless.ts', 'src/**/index.headless.browser.ts', 'src/**/coverage/**', 'src/**/dist/**', 'src/**/node_modules/**', 'src/(ui|utils|core)/__tests__/**', + 'src/**/*.test.{ts,tsx}', + 'src/**/*.spec.{ts,tsx}', + 'src/**/vitest/**', + 'src/**/__mocks__/**' ],packages/clerk-js/vitest.setup.mts (1)
69-84: Consider making IntersectionObserver implementation more complete.The current mock is minimal. Consider implementing the basic callback functionality for more realistic testing.
Enhance the IntersectionObserver mock:
- //@ts-expect-error - Mocking class - globalThis.IntersectionObserver = class IntersectionObserver { - constructor() {} - disconnect() { - return null; - } - observe() { - return null; - } - takeRecords() { - return []; // Return empty array as per spec - } - unobserve() { - return null; - } - }; + globalThis.IntersectionObserver = vi.fn().mockImplementation((callback) => ({ + root: null, + rootMargin: '0px', + thresholds: [0], + disconnect: vi.fn(), + observe: vi.fn(), + unobserve: vi.fn(), + takeRecords: vi.fn(() => []), + })) as any;packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/vitestUtils.ts (2)
24-54: Avoid unnecessary optional‐chaining on required params
paramsis a required argument, yet several fields are accessed with optional-chaining (e.g.params?.maxAllowedMemberships).
Using?.here hides genuine typing mistakes and prevents the compiler from warning if the field is missing. Replace with direct access.- maxAllowedMemberships: params?.maxAllowedMemberships, - createdAt: params?.createdAt || new Date(), + maxAllowedMemberships: params.maxAllowedMemberships, + createdAt: params.createdAt ?? new Date(),
65-88: Leverage Partial & Pick to avoid widespreadas anycastsAll mocked methods are typed as
vi.fn() as any, defeating the purpose of strong typing.
You can keep type-safety by returningvi.fn<ReturnType<typeof someRealMethod>, Parameters<typeof someRealMethod>>(), or by declaring a reusable helper:const mockFn = <T extends (...a: any[]) => any>(): vi.Mock<ReturnType<T>, Parameters<T>> => vi.fn() as any;Then:
- accept: vi.fn() as any, + accept: mockFn<OrganizationInvitationResource['accept']>(),This preserves IntelliSense while keeping the code concise.
packages/clerk-js/src/ui/utils/vitest/createFixtures.tsx (2)
84-90: Variable shadowing obscures outerpropshelper
const props = { … }is overshadowed by thepropsparameter ofMockClerkProvider, reducing readability and making accidental misuse easy.-const props = { +const componentCtxPropsHelper = { setProps: … } … const MockClerkProvider = ({ children }: { children: React.ReactNode }) => {
94-104: Hard-coded component exclusion list will silently stale
componentsWithoutContextis an ad-hoc string array. If new components that require skipping are added, forgetting to update this list will break tests.Consider deriving the information from types (e.g. a discriminated union) or exposing a helper
componentNeedsContext(name)to centralize the logic.packages/clerk-js/src/vitestUtils.ts (1)
1-6: Use named import foruserEventto avoid future breaking changes
@testing-library/user-event’s default export is not guaranteed; the docs recommend the named import:-import UserEvent from '@testing-library/user-event'; +import { userEvent as userEventLib } from '@testing-library/user-event';and inside
render:-const userEvent = UserEvent.setup({ delay: null }); +const user = userEventLib.setup({ delay: null });This prevents surprises if the library switches to ESM with no default export.
packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts (2)
295-301:withMultiSessionModemutates state but ignores requested modeThe helper always flips
single_session_modetofalse, giving no way to re-enable the flag and making intent unclear. Accept a boolean parameter (defaulttrue) for symmetry.- const withMultiSessionMode = () => { - // TODO: - ac.single_session_mode = false; - }; + const withMultiSessionMode = (enable = true) => { + ac.single_session_mode = !enable; + };
487-496: Avoid// @ts-expect-error– model social strategies properlyRelying on a blanket suppression hides real typing issues and can break after type upgrades.
Extend theUserSettingsJSON['social']index signature instead of forcing an error.packages/clerk-js/src/core/vitest/fixtures.ts (2)
111-116:idinitialised with provider name
ExternalAccountJSON.idshould be a unique identifier (ext_...), not the OAuth provider string.- id: params?.provider || '', + id: params?.id || 'ext_test',
260-270:mockFetchnever restores the original globalOverriding
global.fetchwithout teardown leaks between tests. Capture the original and return a restore helper.-export const mockFetch = (ok = true, status = 200, responsePayload = {}) => { - // @ts-ignore - global.fetch = vi.fn(() => { - return Promise.resolve<RecursivePartial<Response>>({ +export const mockFetch = (ok = true, status = 200, responsePayload = {}) => { + // @ts-ignore + const original = global.fetch; + global.fetch = vi.fn(() => + Promise.resolve<RecursivePartial<Response>>({ status, statusText: status.toString(), ok, json: () => Promise.resolve(responsePayload), - }); - }); + }), + ); + return () => (global.fetch = original); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
packages/clerk-js/src/core/resources/__tests__/__snapshots__/Client.spec.ts.snapis excluded by!**/*.snappackages/clerk-js/src/core/resources/__tests__/__snapshots__/Environment.spec.ts.snapis excluded by!**/*.snappackages/clerk-js/src/core/resources/__tests__/__snapshots__/Session.spec.ts.snapis excluded by!**/*.snappackages/clerk-js/src/core/resources/__tests__/__snapshots__/UserSettings.spec.ts.snapis excluded by!**/*.snappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
.changeset/chatty-wombats-rest.md(1 hunks)eslint.config.mjs(1 hunks)package.json(1 hunks)packages/clerk-js/package.json(3 hunks)packages/clerk-js/src/__tests__/headless.spec.ts(1 hunks)packages/clerk-js/src/__tests__/mocks/svgMock.tsx(1 hunks)packages/clerk-js/src/core/__tests__/clerk.redirects.spec.ts(2 hunks)packages/clerk-js/src/core/__tests__/fapiClient.spec.ts(5 hunks)packages/clerk-js/src/core/__tests__/tokenCache.spec.ts(6 hunks)packages/clerk-js/src/core/auth/__tests__/cookieSuffix.spec.ts(2 hunks)packages/clerk-js/src/core/auth/__tests__/devBrowser.spec.ts(3 hunks)packages/clerk-js/src/core/auth/__tests__/getCookieDomain.spec.ts(5 hunks)packages/clerk-js/src/core/auth/__tests__/getSecureAttribute.spec.ts(1 hunks)packages/clerk-js/src/core/fraudProtection.spec.ts(9 hunks)packages/clerk-js/src/core/resources/__tests__/AuthConfig.spec.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Base.spec.ts(3 hunks)packages/clerk-js/src/core/resources/__tests__/Client.spec.ts(5 hunks)packages/clerk-js/src/core/resources/__tests__/Environment.spec.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/ExternalAccount.spec.ts(3 hunks)packages/clerk-js/src/core/resources/__tests__/Image.spec.ts(2 hunks)packages/clerk-js/src/core/resources/__tests__/Session.spec.ts(4 hunks)packages/clerk-js/src/core/resources/__tests__/Token.spec.ts(3 hunks)packages/clerk-js/src/core/resources/__tests__/User.spec.ts(9 hunks)packages/clerk-js/src/core/resources/__tests__/UserSettings.spec.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Web3Wallet.spec.ts(5 hunks)packages/clerk-js/src/core/vitest/fixtures.ts(1 hunks)packages/clerk-js/src/ui/common/__tests__/redirects.spec.ts(13 hunks)packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/vitestUtils.ts(1 hunks)packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpStart.spec.tsx(4 hunks)packages/clerk-js/src/ui/elements/__tests__/PlainInput.spec.tsx(2 hunks)packages/clerk-js/src/ui/elements/__tests__/RadioGroup.spec.tsx(2 hunks)packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.spec.tsx(1 hunks)packages/clerk-js/src/ui/hooks/__tests__/useDevMode.spec.tsx(1 hunks)packages/clerk-js/src/ui/hooks/__tests__/useSupportEmail.spec.tsx(1 hunks)packages/clerk-js/src/ui/localization/__tests__/applyTokensToString.spec.ts(2 hunks)packages/clerk-js/src/ui/router/__mocks__/RouteContext.tsx(1 hunks)packages/clerk-js/src/ui/router/__tests__/PathRouter.spec.tsx(1 hunks)packages/clerk-js/src/ui/router/__tests__/Switch.spec.tsx(2 hunks)packages/clerk-js/src/ui/router/__tests__/VirtualRouter.spec.tsx(1 hunks)packages/clerk-js/src/ui/utils/__tests__/normalizeColorString.spec.ts(4 hunks)packages/clerk-js/src/ui/utils/test/createFixtures.tsx(0 hunks)packages/clerk-js/src/ui/utils/vitest/createFixtures.tsx(1 hunks)packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts(1 hunks)packages/clerk-js/src/ui/utils/vitest/fixtures.ts(1 hunks)packages/clerk-js/src/ui/utils/vitest/mockHelpers.ts(1 hunks)packages/clerk-js/src/ui/utils/vitest/runFakeTimers.ts(1 hunks)packages/clerk-js/src/utils/__tests__/captcha.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/completeSignUpFlow.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/date.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/dynamicParamParser.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/errors.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/ignoreEventValue.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/instance.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/jwt.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/localStorage.spec.ts(4 hunks)packages/clerk-js/src/utils/__tests__/memoizeStateListenerCallback.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/organization.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/passkeys.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/path.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/queryStateParams.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/querystring.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/redirectUrls.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/resourceParams.spec.ts(1 hunks)packages/clerk-js/src/utils/__tests__/url.spec.ts(6 hunks)packages/clerk-js/src/vitestUtils.ts(1 hunks)packages/clerk-js/vitest.config.mts(1 hunks)packages/clerk-js/vitest.setup.mts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/clerk-js/src/ui/utils/test/createFixtures.tsx
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/clerk-js/src/ui/router/__mocks__/RouteContext.tsx (1)
packages/shared/src/utils/index.ts (1)
noop(5-5)
packages/clerk-js/src/utils/__tests__/localStorage.spec.ts (1)
packages/clerk-js/src/utils/localStorage.ts (1)
SafeLocalStorage(17-66)
packages/clerk-js/src/core/resources/__tests__/Session.spec.ts (1)
packages/clerk-js/src/core/events.ts (1)
eventBus(20-20)
packages/clerk-js/src/core/__tests__/clerk.redirects.spec.ts (1)
packages/clerk-js/src/core/resources/Environment.ts (1)
Environment(16-93)
packages/clerk-js/src/core/resources/__tests__/Client.spec.ts (1)
packages/clerk-js/src/core/vitest/fixtures.ts (1)
createSession(174-199)
packages/clerk-js/src/ui/utils/vitest/createFixtures.tsx (9)
packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts (2)
createEnvironmentFixtureHelpers(25-32)createClientFixtureHelpers(34-40)packages/clerk-js/src/ui/utils/vitest/mockHelpers.ts (2)
mockRouteContextValue(85-103)mockClerkMethods(46-83)packages/clerk-js/src/ui/types.ts (2)
AvailableComponentName(147-147)AvailableComponentProps(39-52)packages/clerk-js/src/ui/utils/vitest/fixtures.ts (2)
createBaseEnvironmentJSON(13-23)createBaseClientJSON(209-211)packages/clerk-js/src/core/resources/Environment.ts (1)
Environment(16-93)packages/types/src/clerk.ts (1)
LoadedClerk(1825-1827)packages/clerk-js/src/ui/contexts/ClerkUIComponentsContext.tsx (1)
ComponentContextProvider(22-102)packages/clerk-js/src/ui/customizables/index.ts (1)
AppearanceProvider(9-9)packages/clerk-js/src/ui/elements/contexts/index.tsx (1)
FlowMetadataProvider(116-120)
🪛 Biome (1.9.4)
packages/clerk-js/src/ui/common/__tests__/redirects.spec.ts
[error] 3-3: Declarations inside of a import declaration may not have duplicates
a second declaration of buildVerificationRedirectUrl is not allowed
buildVerificationRedirectUrl is first declared here
(parse)
🪛 Gitleaks (8.26.0)
packages/clerk-js/src/ui/hooks/__tests__/useSupportEmail.spec.tsx
11-11: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts
88-88: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
packages/clerk-js/src/ui/utils/vitest/createFixtures.tsx
67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (23)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 13)
- GitHub Check: Integration Tests (tanstack-react-router, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (quickstart, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Static analysis
- GitHub Check: Unit Tests (18, --filter=@clerk/astro --filter=@clerk/backend --filter=@clerk/express --filter=@c...
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (133)
packages/clerk-js/src/utils/__tests__/errors.spec.ts (1)
1-1: Migrate to Vitest importsExplicitly importing
describe,expect, anditfrom Vitest is correct and aligns with the broader test migration.packages/clerk-js/src/core/resources/__tests__/UserSettings.spec.ts (1)
2-2: Switch to Vitest functionsImporting
describe,expect, anditfrom Vitest properly replaces Jest globals in this test suite.packages/clerk-js/src/utils/__tests__/passkeys.spec.ts (1)
7-7: Update imports for VitestCorrectly importing core test functions from Vitest ensures consistency with the new testing framework.
package.json (1)
86-86: Bump @vitest/coverage-v8 versionUpdating the coverage-v8 plugin to
3.0.5to match the Vitest version is appropriate. Ensure this version is available on npm and aligns with your Vitest config.packages/clerk-js/src/utils/__tests__/jwt.spec.ts (1)
1-2: Use explicit Vitest imports
The test now importsdescribe,expect, anditfrom Vitest, aligning with the Jest→Vitest migration. No other logic changes.packages/clerk-js/src/utils/__tests__/querystring.spec.ts (1)
2-2: Use explicit Vitest imports
Addingdescribe,expect, anditfrom Vitest ensures consistency with the new test runner. The existing assertions remain unchanged.packages/clerk-js/src/utils/__tests__/captcha.spec.ts (1)
1-2: Use explicit Vitest imports
Importing Vitest'sdescribe,expect, anditsupports the transition from Jest; theit.eachsyntax is preserved.packages/clerk-js/src/core/resources/__tests__/Environment.spec.ts (1)
2-2: Use explicit Vitest imports
The test now explicitly pullsdescribe,expect, anditfrom Vitest. No snapshots or logic were altered.packages/clerk-js/src/utils/__tests__/ignoreEventValue.spec.ts (1)
1-2: Use explicit Vitest imports
Swapped in Vitest's testing functions for Jest globals. Test behavior remains intact.packages/clerk-js/src/utils/__tests__/organization.spec.ts (1)
1-1: Migrate test imports to Vitest
Imports fordescribe,expect, anditfrom Vitest align perfectly with the Jest→Vitest migration in this suite.packages/clerk-js/src/utils/__tests__/queryStateParams.spec.ts (1)
1-1: Migrate test imports to Vitest
Switching todescribe,expect, anditfrom Vitest is correct and consistent with the other converted tests.packages/clerk-js/src/utils/__tests__/resourceParams.spec.ts (1)
2-2: Migrate test imports to Vitest
The explicit import ofdescribe,expect, anditfrom Vitest correctly replaces the previous Jest imports.packages/clerk-js/src/utils/__tests__/memoizeStateListenerCallback.spec.ts (1)
3-3: Migrate test imports to Vitest
Importingdescribeanditfrom Vitest is sufficient for the skipped tests here—noexpectorviusage needed.packages/clerk-js/src/utils/__tests__/redirectUrls.spec.ts (1)
3-3: Migrate test imports to Vitest
The addition ofafterAll,beforeEach,describe,expect, anditfrom Vitest correctly updates the test harness.packages/clerk-js/src/utils/__tests__/instance.spec.ts (1)
1-1: Correct Vitest import for test functionsThe Vitest globals (
describe,expect,it) are properly imported, replacing Jest equivalents.packages/clerk-js/src/utils/__tests__/path.spec.ts (1)
1-1: Correct Vitest import for test functionsVitest globals (
describe,expect,it) are correctly imported to replace Jest references. The existingit.eachusage aligns with Vitest's API.packages/clerk-js/src/__tests__/headless.spec.ts (2)
2-2: Update test environment directiveThe JSDoc directive has been updated to
@vitest-environment node, ensuring the test runs in the correct environment under Vitest.
5-5: Add Vitest imports for test functionsThe test functions (
describe,expect,it) are properly imported from Vitest, replacing the default Jest globals.packages/clerk-js/src/utils/__tests__/completeSignUpFlow.spec.ts (2)
2-2: Import Vitest lifecycle hooks and mocking utilitiesThe import brings in
beforeEach,describe,expect,it, andvifrom Vitest, correctly replacing Jest equivalents.
6-8: Replace Jest mocks with Vitestvi.fn()Mocks
mockHandleComplete,mockNavigate, andmockAuthenticateWithRedirectare correctly instantiated withvi.fn().packages/clerk-js/src/utils/__tests__/dynamicParamParser.spec.ts (1)
1-1: Correct Vitest import for test functionsThe Vitest globals (
describe,expect,it) are properly imported, completing the migration from Jest.packages/clerk-js/package.json (2)
84-84: Jest-specific plugin added.
The@emotion/jestdependency is appropriate for Jest-based tests. Ensure it’s only applied to the Jest runner configuration so Vitest tests aren’t inadvertently extended by it.
93-94: SVG and DOM mocking dependencies added.
Addingjsdomandvite-plugin-svgrensures the test environment supports DOM APIs and SVG imports when running Vitest.packages/clerk-js/src/core/resources/__tests__/Base.spec.ts (3)
1-1: Switched to Vitest testing API.
Importingdescribe,expect,it, andvifrom Vitest correctly replaces the Jest API for this test file.
25-25: Mocked request withvi.fn().mockResolvedValue.
Replacingjest.fn()withvi.fn()maintains the same mocking behavior under Vitest.
33-33: Mocked internal callback withvi.fn().
The__internal_setCountrymock is correctly converted to use Vitest’svi.fn()API.packages/clerk-js/src/core/resources/__tests__/User.spec.ts (2)
2-2: Updated test imports to Vitest.
Switching imports from Jest to Vitest (describe,expect,it,vi) aligns with the overall test migration.
18-18: Migrated resource fetch mocks to Vitest’svi.fn.
All instances ofBaseResource._fetch = jest.fn()have been correctly updated to usevi.fn()under Vitest.Also applies to: 53-53, 152-152, 180-180, 207-207, 274-274, 292-292
packages/clerk-js/src/ui/hooks/__tests__/useSupportEmail.spec.tsx (3)
2-2: Adopted Vitest test utilities.
Updating imports to usedescribe,expect,test, andvifrom Vitest ensures compatibility with the new test runner.
6-7: Initialized mocks with Vitest'svi.fn().
Replacing Jest mock functions withvi.fn()formockUseOptionsandmockUseEnvironmentmaintains consistent mock behavior.
14-18: Mocked application context with Vitest'svi.mock.
TheuseOptionsanduseEnvironmentmocks are correctly converted to Vitest’s API.packages/clerk-js/src/core/auth/__tests__/cookieSuffix.spec.ts (5)
1-1: Migrated imports to Vitest.
Switching the test utilities import from Jest to Vitest (afterEach,beforeEach,describe,expect,test,vi) aligns with the migration.
3-5: Mocked shared keys module withvi.mock.
Converting the Jest mock ofgetCookieSuffixto Vitest’svi.mockpreserves the intended behavior.
6-8: Mocked logger module withvi.mock.
Replacing the Jest mock forlogger.logOncewithvi.fn()under Vitest is correct.
16-16: Set up rejected mock for insecure context.
UsingmockRejectedValueon the Vitestvi.Mockensures the code path for errors is tested appropriately.
20-21: Reset mocks after each test.
Resetting bothgetSharedCookieSuffixandlogger.logOnceviamockReset()avoids state leakage across tests.packages/clerk-js/src/ui/router/__tests__/PathRouter.spec.tsx (3)
4-4: Replace Jest globals with Vitest importsThe global hooks and mocking functions have been correctly switched to Vitest’s API.
9-9: Usevi.fn()for spy creationMigrating from
jest.fn()tovi.fn()ensures compatibility with Vitest’s mocking utilities.
11-26: Switchjest.mock()tovi.mock()for module mockingThe module mock for
@clerk/shared/reactis correctly updated to use Vitest’svi.mock().packages/clerk-js/src/ui/elements/__tests__/RadioGroup.spec.tsx (3)
3-3: Update imports to VitestReplacing Jest globals with Vitest’s
describe,expect, anditis accurate.
6-6: Adjust fixture helper import pathSwitching to the Vitest-specific
createFixturesmodule aligns with the new test infrastructure.
170-170: Capitalize error expectationThe assertion now matches the updated error message casing (
Some Error).packages/clerk-js/src/ui/localization/__tests__/applyTokensToString.spec.ts (2)
1-1: Replace Jest hooks with VitestMigrating
beforeEach,describe,expect,it, andviimports to Vitest is correct.
39-39: Usevi.spyOn()for console warningsReplacing
jest.spyOnwithvi.spyOnensures consistent mocking under Vitest.packages/clerk-js/src/core/auth/__tests__/getSecureAttribute.spec.ts (2)
1-1: Switch test imports to VitestUpdating Jest imports to Vitest (
afterEach,beforeEach,describe,expect,it,vi) is correctly applied.
9-9: Usevi.spyOn()forwindowgetterThe spy on
window.windowis properly migrated to Vitest’svi.spyOn.packages/clerk-js/src/ui/elements/__tests__/PlainInput.spec.tsx (3)
3-3: Replace Jest globals with Vitest importsSwitching to Vitest’s
describe,expect, anditis correctly implemented.
6-6: Update fixture helper import to Vitest pathChanging to the Vitest-specific
createFixturesmodule aligns with the new test setup.
138-138: Capitalize error expectationThe test assertion now expects
Some Errorinstead of lowercase, matching the updated UI behavior.packages/clerk-js/src/core/resources/__tests__/Image.spec.ts (1)
1-1: Migration to Vitest imports looks correct. The import ofdescribe,expect,it, andvifrom Vitest properly replaces Jest globals.packages/clerk-js/src/ui/hooks/__tests__/useDevMode.spec.tsx (4)
3-3: Framework import updated correctly. Switching from Jest to Vitest’sdescribe,expect,test, andviis appropriate.
7-9: Hook mocks declared usingvi.fn(). Your replacement ofjest.fn()withvi.fn()formockUseEnvironment,mockUseOptions, andmockUseAppearanceis spot on.
11-16: Context mock conversion is valid. Thevi.mock('../../contexts', …)implementation maintains behavior from Jest’sjest.mock.
18-22: Appearance mock conversion is valid. Replacingjest.mockwithvi.mockforuseAppearanceretains original logic.packages/clerk-js/src/core/resources/__tests__/ExternalAccount.spec.ts (1)
1-1: Vitest import changes are correct. Importing Vitest’s APIs replaces Jest globals appropriately.packages/clerk-js/src/ui/utils/__tests__/normalizeColorString.spec.ts (4)
1-1: Switched to Vitest lifecycle & assertion APIs. The import ofafterEach,beforeEach,describe,expect,test, andviis correct.
7-7:vi.spyOnused correctly. Theconsole.warnspy with a noop implementation properly replacesjest.spyOn.
11-11:vi.clearAllMocks()inafterEachis appropriate. This cleans up all spy/mock state between tests.
39-39: Manualconsole.warnresets are valid. Clearing the mock mid-test isolates call counts as intended.Also applies to: 83-83, 87-87, 96-96
packages/clerk-js/src/ui/router/__tests__/VirtualRouter.spec.tsx (3)
4-4: Vitest import usage is correct. Switching toafterAll,beforeAll,beforeEach,describe,expect,it, andvimatches Vitest conventions.
8-8:vi.fn()formockNavigateis correct. Replacingjest.fn()maintains the same mocking behavior.
12-19:vi.mockforuseClerknavigation is accurate. The mock implementation usingvi.fncorrectly emulatesnavigate.packages/clerk-js/src/core/resources/__tests__/Web3Wallet.spec.ts (1)
2-2: LGTM! Clean Jest to Vitest migration.The migration is well-executed with consistent replacement of Jest utilities with Vitest equivalents:
- Import statement properly updated to use Vitest utilities
- All mock functions correctly changed from
jest.fn()tovi.fn()- Test logic and assertions remain unchanged, preserving test integrity
Also applies to: 14-14, 37-37, 60-60, 85-85
packages/clerk-js/src/ui/common/__tests__/redirects.spec.ts (2)
1-2: LGTM! Proper Jest to Vitest migration.The import statement has been correctly updated to use Vitest testing utilities instead of Jest.
9-9: Verify the localhost port change aligns with test environment.All localhost URLs have been updated to include port 3000. Ensure this change aligns with your test environment configuration and doesn't break existing test setups.
Also applies to: 18-18, 26-26, 38-38, 50-50, 62-62, 74-74, 86-86, 98-98, 110-110, 122-122, 158-158, 170-170, 177-177, 178-178, 180-180, 184-184, 186-186, 191-191, 202-202, 221-221
eslint.config.mjs (1)
372-379: LGTM! Proper ESLint configuration for Vitest migration.The new configuration block correctly:
- Targets Vitest test files (
.spec.{ts,tsx}) in the clerk-js package- Disables Jest-specific ESLint rules that would conflict with Vitest
- Follows the established naming and structure conventions
This configuration is essential for preventing ESLint errors during the Jest to Vitest migration.
packages/clerk-js/src/utils/__tests__/date.spec.ts (1)
1-2: LGTM! Correct timer mock migration to Vitest.The timer mocking has been properly migrated from Jest to Vitest:
- Import statement correctly updated to include Vitest utilities
jest.useFakeTimers('modern')→vi.useFakeTimers()jest.setSystemTime()→vi.setSystemTime(Date.now())jest.useRealTimers()→vi.useRealTimers()The test logic remains unchanged and the timer mocking functionality is preserved.
Also applies to: 7-8, 12-12
packages/clerk-js/src/core/__tests__/tokenCache.spec.ts (4)
2-2: Migration: Switch test imports to VitestThe import now explicitly brings in Vitest’s globals (
describe,it,expect,vi, etc.) instead of relying on Jest’s environment.
8-14: Migration: Replace jest.mock with vi.mockThe
vi.mockcall correctly replaces the Jest mock forBaseResourcewith Vitest’s equivalent.
21-22: Migration: Replace Jest timer functions with Vitest equivalentsThe
useFakeTimers/useRealTimerscalls have been updated tovi.useFakeTimers()andvi.useRealTimers(), aligning with Vitest’s API.Also applies to: 25-26
91-91: Migration: Replace jest.advanceTimersByTime with vi.advanceTimersByTimeAll calls to advance the fake timers have been updated to
vi.advanceTimersByTime().Also applies to: 102-102, 129-129, 133-133, 155-155, 158-158, 162-162
packages/clerk-js/src/ui/components/SignUp/__tests__/SignUpStart.spec.tsx (4)
3-3: Migration: Switch test imports to VitestThe top-level import now uses
describe,it,expect, andvifrom Vitest instead of Jest.
7-7: Migration: Import rendering utilities from vitestUtilsThe rendering helpers (
render,screen,waitFor) have been switched to your new Vitest-specific utilities module.
10-10: Migration: Switch fixture helper import to Vitest helperThe
bindCreateFixturesimport now points to the Vitest fixture creator instead of the Jest-based one.
302-302: Migration: Replace jest.fn with vi.fn for window.history mockingThe
replaceStatemock is now created withvi.fn(), matching Vitest’s mocking API.Also applies to: 338-338
packages/clerk-js/src/core/resources/__tests__/AuthConfig.spec.ts (2)
1-1: Migration: Switch test imports to VitestImports now pull
describe,it,expect, andvifrom Vitest rather than using Jest’s globals.
6-8: Migration: Replace jest.mock with vi.mockThe module mock for
unixEpochToDateis correctly rewritten withvi.mockandvi.fn.packages/clerk-js/src/utils/__tests__/localStorage.spec.ts (4)
1-1: Migration: Switch test imports to VitestThe test now imports all lifecycle hooks and mock utilities from Vitest.
31-31: Add Vitest mock cleanup in afterEachIncluding
vi.restoreAllMocks()ensures all mocks are reset between tests, mirroring Jest’srestoreAllMocks.
58-65: Migration: Replace Jest timer functions with Vitest equivalentsUpdated the
useFakeTimers()/useRealTimers()calls for the expiration test insetItemto Vitest’s API.
110-118: Migration: Replace Jest timer functions with Vitest equivalentsSimilarly, the expiration scenario in
getItemnow usesvi.useFakeTimers(),vi.advanceTimersByTime(), andvi.useRealTimers().packages/clerk-js/src/core/auth/__tests__/getCookieDomain.spec.ts (5)
1-1: Migration: Switch test imports to VitestThe initial import now explicitly sources Vitest’s test functions and mocks.
11-11: Migration: Reset modules using vi.resetModulesReplacing
jest.resetModules()withvi.resetModules()correctly ensures a fresh module cache for each test.
25-33: Migration: Replace jest.fn mocks with vi.fnThe chained
mockReturnValueOnceusages onget,set, andremovehandlers have been updated to Vitest’svi.fn().
58-61: Migration: Replace jest.fn mocks with vi.fnThe handler mock in the undefined-domain test uses
vi.fn(), maintaining the original behavior.
70-73: Migration: Replace jest.fn mocks with vi.fnThe cached-value test mocks are now implemented with
vi.fn(), ensuring the call count assertion works as expected.packages/clerk-js/src/ui/router/__tests__/Switch.spec.tsx (4)
3-3: Migrate to Vitest test API
Importing Vitest globals (describe,it,vi, etc.) is correct and aligns with the new test framework.
7-7: Capture navigation calls with a dedicated mock
DefiningmockNavigateseparately ensures you can assert on navigation arguments. Good extraction.
9-19: Replace Jest module mock with Vitest mock
Thevi.mocksetup mirrors the previousjest.mockbehavior and correctly delegates tomockNavigate.
33-33: Usevi.clearAllMocksto reset mocks between tests
Replacingjest.clearAllMockswithvi.clearAllMocksensures no residual state across tests.packages/clerk-js/src/ui/hooks/__tests__/useCoreOrganizationList.spec.tsx (4)
2-2: Switch test globals to Vitest
Importingdescribe,it, andexpectfrom Vitest aligns with the overall migration.
4-4: Leverage custom Vitest utilities
Confirm thatvitestUtilsexportsact,renderHook, andwaitForwith equivalent behavior to the previous Jest utilities.
9-9: Update organization suggestion fixtures import
Importing from the Vitest-specific fixtures path is consistent with other test migrations.
10-10: Bind Vitest fixtures helper
UsingbindCreateFixturesfor organization fixtures maintains test consistency and eliminates Jest dependencies.packages/clerk-js/src/core/auth/__tests__/devBrowser.spec.ts (4)
1-1: Import Vitest lifecycle hooks and mocks
Switching from Jest globals to Vitest equivalents is correctly handled.
13-13: Mockfetchwith Vitest
Settingglobal.fetch = vi.fn()preserves the original test logic under Vitest.
34-34: Clearfetchmock after each test
Usingvi.mocked(global.fetch)?.mockClear()ensures no cross-test pollution of the mock.
40-46: Replace Jest mock factories with Vitestvi.fn()
Migratingjest.fn()calls (buildUrl,onAfterResponse,onBeforeRequest) tovi.fn()retains the behavior of the FAPI client mock.packages/clerk-js/src/ui/router/__mocks__/RouteContext.tsx (2)
2-2: Use Vitest for mocks
Importingviinstead ofjestaligns with Vitest conventions in mock modules.
5-11: Convert mock methods tovi.fn()
Replacingjest.fn()withvi.fn()forresolve,matches, andnavigateensures consistent mocking under Vitest.packages/clerk-js/src/core/__tests__/fapiClient.spec.ts (4)
2-2: Adopt Vitest globals
Importing lifecycle hooks and mock utilities from Vitest is correct for the migration.
57-59: Clear thefetchmock between tests
Using(global.fetch as vi.Mock).mockClear()inbeforeEachcorrectly resets call history.
188-192: Update inlinefetchmock resolution
Switchingjestmocks tovi.fn()inside the array response test keeps behavior consistent.
205-210: Handle 204 response mock with Vitest
Migrating the 204 response scenario to usevi.Mockretains the original test intent.packages/clerk-js/src/core/resources/__tests__/Client.spec.ts (3)
2-2: LGTM: Vitest import correctly updated.The import statement has been properly migrated from Jest to Vitest testing utilities.
4-4: LGTM: Fixture import path updated for Vitest.The import path has been correctly updated to use the new Vitest-specific fixtures module.
25-25: LGTM: Mock functions correctly migrated to Vitest.All Jest mock functions have been properly replaced with their Vitest equivalents (
vi.fn(),vi.fn().mockResolvedValueOnce(),vi.fn().mockReturnValue()).Also applies to: 55-55, 108-108
packages/clerk-js/src/core/fraudProtection.spec.ts (4)
1-2: LGTM: Vitest imports and types correctly updated.The import statement has been properly migrated from Jest to Vitest, including the correct
Mocktype import.
12-12: LGTM: Mock type updated to Vitest.The type annotation has been correctly changed from
jest.Mockto Vitest'sMocktype.
27-27: LGTM: Mock functions correctly migrated to Vitest.All Jest mock functions have been properly replaced with their Vitest equivalents using
vi.fn()and its chaining methods.Also applies to: 34-34, 50-50, 69-69, 78-79, 98-99, 120-121, 152-156
71-71: LGTM: Async rejection expectations properly handled.The addition of
awaitfor promise rejection expectations is correct for Vitest's async handling requirements.Also applies to: 168-168
packages/clerk-js/src/utils/__tests__/url.spec.ts (2)
3-3: LGTM: Vitest import correctly updated.The import statement has been properly migrated from Jest to Vitest testing utilities.
503-503: LGTM: Spy function correctly migrated to Vitest.The spy function has been properly replaced with Vitest's
vi.spyOn().packages/clerk-js/src/core/resources/__tests__/Session.spec.ts (4)
2-2: LGTM: Vitest imports and types correctly updated.The import statement has been properly migrated from Jest to Vitest, including the correct
Mocktype import.
7-7: LGTM: Fixture import path updated for Vitest.The import path has been correctly updated to use the new Vitest-specific fixtures module, consistent with other test files.
27-27: LGTM: Spy functions correctly migrated to Vitest.All Jest spy functions have been properly replaced with Vitest's
vi.spyOn().Also applies to: 173-173
225-225: LGTM: Mock type annotation updated to Vitest.The type casting has been correctly changed from
jest.Mock<any>to Vitest'sMock<any>type.packages/clerk-js/src/core/resources/__tests__/Token.spec.ts (4)
2-2: LGTM! Clean Jest to Vitest import migration.The import statement correctly migrates from Jest to Vitest testing utilities.
6-6: LGTM! Fixture imports updated for Vitest.The import correctly switches to the vitest-specific fixtures module.
21-21: LGTM! Mock type correctly updated for Vitest.The Mock type is now properly imported from Vitest instead of Jest.
53-53: LGTM! Spy utility correctly migrated to Vitest.The change from
jest.spyOntovi.spyOnis correct for the Vitest migration.packages/clerk-js/src/core/__tests__/clerk.redirects.spec.ts (5)
1-2: LGTM! Clean Jest to Vitest import migration.The import statement correctly migrates all testing utilities from Jest to Vitest.
8-9: LGTM! Mock functions properly migrated to Vitest.The mock functions are correctly updated to use
vi.fn()instead ofjest.fn().
11-12: LGTM! Module mocking correctly updated for Vitest.The module mocks are properly migrated to use
vi.mock()instead ofjest.mock().
15-22: LGTM! Mock implementation correctly migrated.The DevBrowser mock implementation properly uses
vi.fn()for all mock functions.
25-30: LGTM! Mock implementations correctly updated for Vitest.The Client and Environment mock implementations properly use
vi.fn().mockImplementation().packages/clerk-js/src/ui/utils/vitest/fixtures.ts (1)
93-106: Good use of type safety withcontainsAllOfTypehelper.The frozen arrays with the
containsAllOfTypeutility ensure compile-time validation that all required attribute keys are included, preventing runtime errors.packages/clerk-js/src/ui/utils/vitest/mockHelpers.ts (1)
46-83: Good systematic mocking approach for Clerk resources.The recursive mocking of nested resources (user, email addresses, phone numbers, etc.) provides comprehensive test coverage while maintaining the object structure.
packages/clerk-js/vitest.config.mts (2)
34-36: Verify if coverage should be enabled by default.Coverage is enabled by default (
enabled: true), which might slow down test execution during development.Consider making coverage opt-in for better developer experience:
coverage: { provider: 'v8', - enabled: true, + enabled: process.env.CI === 'true' || process.env.COVERAGE === 'true', reporter: ['text', 'json', 'html'],
9-21:❌ Incorrect review comment
Verify SVG mock plugin behavior in test environment.
The SVG mock plugin only transforms files when
NODE_ENV === 'test', but Vitest might not always set this environment variable. This could lead to SVG imports not being mocked properly.
🏁 Script executed:
#!/bin/bash # Check if NODE_ENV is set in test-related scripts rg -A 5 "NODE_ENV.*test" package.json scripts/ --type json # Check if there are any other references to NODE_ENV in test files fd -e ts -e tsx -e js -e jsx --exec grep -l "process.env.NODE_ENV" {} \; | grep -E "(test|spec)" | head -20Length of output: 376
Incorrect concern — Vitest already sets NODE_ENV to “test”
Vitest automatically setsNODE_ENV(andVITE_MODE) to"test"when running, so your SVG‐mock transform will always fire as expected. No changes required.Likely an incorrect or invalid review comment.
packages/clerk-js/vitest.setup.mts (2)
12-13: Good approach to preserve the original method.Storing the original
toLocaleDateStringmethod before overriding it ensures proper cleanup and prevents test pollution.
87-95:✅ Verification successful
Consider implementing Chrome API mocks if needed.
The commented Chrome API mock suggests it might be needed. Verify if any tests require these APIs.
🏁 Script executed:
#!/bin/bash # Check if any test files reference Chrome APIs rg -A 3 "chrome\.(runtime|storage|tabs)" --type ts --type tsx | grep -E "(test|spec)" # Check for any Chrome extension related code fd -e ts -e tsx --exec grep -l "chrome\." {} \; | grep -v node_modules | head -10Length of output: 489
No Chrome API references in clerk-js tests—mocks not needed
I searched the entire repo for
chrome.and only found usages in the browser-extension and chrome-extension packages (e.g.playground/browser-extension/...,packages/chrome-extension/...). There are no references tochrome.*in any files underpackages/clerk-js, including its Vitest tests.You can leave the Chrome mock snippet in
packages/clerk-js/vitest.setup.mtscommented out for future use, but no action is required now.packages/clerk-js/src/vitestUtils.ts (1)
32-52:mockNativeRuntimemanipulates global getters – guard against non-existent descriptors
globalThis.documentmay already beundefinedin js-dom-less environments, makingvi.spyOn(globalThis, 'document', 'get')throw.
Add an existence check or fallback toObject.definePropertyonly when the descriptor exists to avoid crashing the test suite in Node-only runs.
| export const mockJwt = | ||
| 'eyJhbGciOiJSUzI1NiIsImtpZCI6Imluc18yR0lvUWhiVXB5MGhYN0IyY1ZrdVRNaW5Yb0QiLCJ0eXAiOiJKV1QifQ.eyJhenAiOiJodHRwczovL2FjY291bnRzLmluc3BpcmVkLnB1bWEtNzQubGNsLmRldiIsImV4cCI6MTY2NjY0ODMxMCwiaWF0IjoxNjY2NjQ4MjUwLCJpc3MiOiJodHRwczovL2NsZXJrLmluc3BpcmVkLnB1bWEtNzQubGNsLmRldiIsIm5iZiI6MTY2NjY0ODI0MCwic2lkIjoic2Vzc18yR2JEQjRlbk5kQ2E1dlMxenBDM1h6Zzl0SzkiLCJzdWIiOiJ1c2VyXzJHSXBYT0VwVnlKdzUxcmtabjlLbW5jNlN4ciJ9.n1Usc-DLDftqA0Xb-_2w8IGs4yjCmwc5RngwbSRvwevuZOIuRoeHmE2sgCdEvjfJEa7ewL6EVGVcM557TWPW--g_J1XQPwBy8tXfz7-S73CEuyRFiR97L2AHRdvRtvGtwR-o6l8aHaFxtlmfWbQXfg4kFJz2UGe9afmh3U9-f_4JOZ5fa3mI98UMy1-bo20vjXeWQ9aGrqaxHQxjnzzC-1Kpi5LdPvhQ16H0dPB8MHRTSM5TAuLKTpPV7wqixmbtcc2-0k6b9FKYZNqRVTaIyV-lifZloBvdzlfOF8nW1VVH_fx-iW5Q3hovHFcJIULHEC1kcAYTubbxzpgeVQepGg'; | ||
|
|
There was a problem hiding this comment.
Real-looking JWT committed to VCS
Same concern as in fixtureHelpers.ts. Replace with a clearly fake, very short token.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/vitest/fixtures.ts around lines 18 to 20, the
mockJwt constant contains a real-looking JWT token which should not be committed
to version control. Replace this token with a clearly fake, very short JWT
string that does not resemble a real token to avoid any security or privacy
concerns.
| object: 'email_address', | ||
| id: params?.email_address || '', | ||
| email_address: '[email protected]', | ||
| reserved: false, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
id is set to an e-mail address
EmailAddressJSON.id should be the Clerk-generated ID (e.g. idn_...), not the address itself.
Use params.id (or generate one) and keep the address in email_address.
- id: params?.email_address || '',
+ id: params?.id || 'idn_test_email',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| object: 'email_address', | |
| id: params?.email_address || '', | |
| email_address: '[email protected]', | |
| reserved: false, | |
| object: 'email_address', | |
| - id: params?.email_address || '', | |
| + id: params?.id || 'idn_test_email', | |
| email_address: '[email protected]', | |
| reserved: false, |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/vitest/fixtures.ts around lines 77 to 80, the `id`
field is incorrectly set to the email address string. Update the `id` to use
`params.id` if available or generate a Clerk-style ID (e.g., starting with
`idn_`), and keep the actual email address in the `email_address` field
unchanged.
| user: createUser({}), | ||
| public_user_data: { | ||
| first_name: user.first_name, | ||
| last_name: user.last_name, | ||
| image_url: user.image_url, | ||
| has_image: user.has_image, | ||
| identifier: user.email_addresses?.find(e => e.id === user.primary_email_address_id)?.email_address || '', | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
public_user_data and user point at different users
createSession always injects a brand-new user, while public_user_data reflects the caller-provided user param, leading to inconsistent fixtures.
- user: createUser({}),
+ const fullUser = createUser(user);
+ ...
+ user: fullUser,
+ public_user_data: {
+ first_name: fullUser.first_name,
+ last_name: fullUser.last_name,
+ image_url: fullUser.image_url,
+ has_image: fullUser.has_image,
+ identifier: fullUser.email_addresses[0]?.email_address || '',
+ },Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/vitest/fixtures.ts around lines 184 to 191, the
public_user_data object is constructed from a different user instance than the
one created by createUser, causing inconsistency. To fix this, ensure that
public_user_data is derived from the same user object returned by createUser, so
both reference the identical user data and maintain fixture consistency.
| object: 'phone_number', | ||
| id: params?.phone_number || '', | ||
| phone_number: '+30 691 1111111', |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same bug for phone numbers
PhoneNumberJSON.id is not a phone number. Mirror the fix above.
- id: params?.phone_number || '',
+ id: params?.id || 'idn_test_phone',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| object: 'phone_number', | |
| id: params?.phone_number || '', | |
| phone_number: '+30 691 1111111', | |
| object: 'phone_number', | |
| id: params?.id || 'idn_test_phone', | |
| phone_number: '+30 691 1111111', |
🤖 Prompt for AI Agents
In packages/clerk-js/src/core/vitest/fixtures.ts around lines 94 to 96, the id
field of PhoneNumberJSON is incorrectly set to a phone number string. Update the
id field to use a unique identifier or a non-phone number value, mirroring the
fix applied previously to ensure id does not contain a phone number.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts (1)
87-89: This is a duplicate of a previously flagged issue.The hardcoded JWT token has already been identified as triggering secret-scanning tools. The previous reviewer suggested replacing this with a clearly fake token.
🧹 Nitpick comments (6)
packages/clerk-js/src/core/__tests__/fapiClient.spec.ts (1)
65-65: Consider replacing delete operator for better performance.The static analysis tool suggests avoiding the
deleteoperator due to performance implications. While the impact is minimal in test cleanup, consider using undefined assignment for consistency.- delete window.Clerk; + window.Clerk = undefined;🧰 Tools
🪛 Biome (1.9.4)
[error] 65-65: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts (5)
151-151: Consider improving type safety by avoidingas anycasts.The use of
as anybypasses TypeScript's type checking and could mask potential runtime errors. Consider creating proper type definitions or using more specific type assertions.- user_data: { ...(createUserFixture() as any) }, + user_data: createUserFixture(),Apply similar changes to lines 179 and 217 by ensuring
createUserFixture()returns the correct type that matches the expecteduser_datastructure.Also applies to: 179-179, 217-217
298-299: TODO comment indicates incomplete implementation.The TODO comment suggests that multi-session mode implementation might be incomplete or needs attention.
Would you like me to help investigate what needs to be implemented for multi-session mode or create an issue to track this task?
349-566: Consider breaking down this large function for better maintainability.The
createUserSettingsFixtureHelpersfunction is quite large (217 lines) and handles many different user settings configurations. Consider breaking it into smaller, more focused helper functions grouped by functionality (e.g., authentication methods, user attributes, sign-up settings).Example refactor approach:
const createUserSettingsFixtureHelpers = (environment: EnvironmentJSON) => { return { ...createAuthenticationHelpers(environment), ...createUserAttributeHelpers(environment), ...createSignUpHelpers(environment), }; };
489-490: String concatenation for strategy could be error-prone.The manual string concatenation
'oauth_' + providercould lead to inconsistencies. Consider using a more type-safe approach or template literals.- const strategy = 'oauth_' + provider; + const strategy = `oauth_${provider}`;
545-545: TODO comment indicates incomplete implementation.The comment references
pkg/generate/auth_config.go, suggesting there might be additional user settings that need to be implemented.Would you like me to help identify what additional user settings might be missing or create an issue to track completing this implementation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/clerk-js/src/core/__tests__/fapiClient.spec.ts(5 hunks)packages/clerk-js/src/ui/common/__tests__/redirects.spec.ts(13 hunks)packages/clerk-js/src/ui/utils/vitest/fixtureHelpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/ui/common/tests/redirects.spec.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/clerk-js/src/core/__tests__/fapiClient.spec.ts
[error] 65-65: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
packages/clerk-js/src/core/__tests__/fapiClient.spec.ts (6)
2-2: LGTM! Clean import migration to Vitest.The import statement correctly replaces Jest utilities with their Vitest equivalents, maintaining all necessary testing functions.
28-29: Excellent addition! Addresses previous review feedback.Capturing the original
fetchreference before mocking prevents test pollution, directly addressing the previous review comment about mock leakage.
31-31: Correct migration from Jest to Vitest mocks.The mock functions are properly converted from
jest.fn()tovi.fn()for Vitest compatibility.Also applies to: 34-34
60-60: Proper type assertion update for Vitest.The cast to
vi.Mockcorrectly replaces the Jest equivalent, ensuring type safety with Vitest mocks.
66-66: Essential cleanup prevents test pollution.Restoring the original
fetchinafterAllcompletes the proper mock lifecycle management, ensuring tests don't interfere with each other.
191-191: Consistent mock type casting throughout.All mock type assertions are consistently updated to use
vi.Mockinstead ofjest.Mock, completing the Vitest migration.Also applies to: 194-194, 208-208
Co-authored-by: Tom Milewski <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/clerk-js/vitest.config.mts (4)
6-23: Consider relocating the mock plugin beforevite-plugin-svgr.
viteSvgMockPluginoverrides whatever is produced by previous transformers for.svgfiles. Keeping it aftervite-plugin-svgrworks but causes each SVG to go throughsvgr’s (un-needed) transform first, then be discarded.
Moving the mock plugin to the front of the array will skip that extra work and speed up test startup a bit.- plugins: [react({ jsxRuntime: 'automatic', jsxImportSource: '@emotion/react' }), svgr(), viteSvgMockPlugin()], + plugins: [ + react({ jsxRuntime: 'automatic', jsxImportSource: '@emotion/react' }), + viteSvgMockPlugin(), // run first for tests + svgr(), + ],
10-17: Useimport.meta.env.MODEinstead ofprocess.env.NODE_ENV.Under Vite/Vitest,
process.env.NODE_ENVis shimmed but not officially documented;import.meta.env.MODEis the recommended API and is statically replaced at build time.- if (id.endsWith('.svg') && process.env.NODE_ENV === 'test') { + if (id.endsWith('.svg') && import.meta.env.MODE === 'test') {
28-31: Avoid hard-coding version to"test"– derive it frompackage.json.Hard-coding may go stale and leaks into the bundle when
vitestis run with--reporter html. You can read it once at config time:-import { defineConfig } from 'vitest/config'; +import { defineConfig } from 'vitest/config'; +import { readFileSync } from 'fs'; +const pkgVersion = JSON.parse( + readFileSync(new URL('./package.json', import.meta.url), 'utf8'), +).version; ... - __PKG_VERSION__: JSON.stringify('test'), + __PKG_VERSION__: JSON.stringify(pkgVersion),
33-51: Exclude generated declaration maps to keep coverage noise down.If
declarationMapis enabled intsconfig,*.d.ts.mapfiles end up insrc/**. They are currently counted as uncovered. Add them to the exclusion list:exclude: [ 'src/**/*.d.ts', + 'src/**/*.d.ts.map',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
package.json(1 hunks)packages/clerk-js/src/core/resources/__tests__/Client.spec.ts(5 hunks)packages/clerk-js/src/core/resources/__tests__/Environment.spec.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Session.spec.ts(4 hunks)packages/clerk-js/src/core/resources/__tests__/UserSettings.spec.ts(1 hunks)packages/clerk-js/vitest.config.mts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/clerk-js/src/core/resources/tests/Session.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/clerk-js/src/core/resources/tests/Environment.spec.ts
- package.json
- packages/clerk-js/src/core/resources/tests/UserSettings.spec.ts
- packages/clerk-js/src/core/resources/tests/Client.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: Analyze (javascript-typescript)
Co-authored-by: Tom Milewski <[email protected]>
Description
Some additional test conversions from jest to vitest now that we have both testing frameworks supported in
clerk-jsChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit