Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/breezy-flowers-float.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
9 changes: 0 additions & 9 deletions packages/shared/customJSDOMEnvironment.ts

This file was deleted.

28 changes: 0 additions & 28 deletions packages/shared/jest.config.js

This file was deleted.

45 changes: 0 additions & 45 deletions packages/shared/jest.setup.ts

This file was deleted.

7 changes: 3 additions & 4 deletions packages/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,9 @@
"lint:attw": "attw --pack . --profile node16",
"lint:publint": "publint",
"publish:local": "pnpm yalc push --replace --sig",
"test": "jest && vitest",
"test:cache:clear": "jest --clearCache --useStderr",
"test:ci": "jest --maxWorkers=70%",
"test:coverage": "jest --collectCoverage && open coverage/lcov-report/index.html"
"test": "vitest",
"test:ci": "vitest --maxWorkers=70%",
"test:coverage": "vitest --collectCoverage && open coverage/lcov-report/index.html"
},
"dependencies": {
"@clerk/types": "workspace:^",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { describe, expect, test } from 'vitest';

import { apiUrlFromPublishableKey } from '../apiUrlFromPublishableKey';

describe('apiUrlFromPublishableKey', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { inBrowser, isValidBrowser, isValidBrowserOnline, userAgentIsRobot } from '../browser';

describe('inBrowser()', () => {
afterEach(() => {
jest.restoreAllMocks();
vi.restoreAllMocks();
});

it('returns true if window is defined', () => {
expect(inBrowser()).toBe(true);
});
it('returns false if window is undefined', () => {
const windowSpy = jest.spyOn(global, 'window', 'get');
const windowSpy = vi.spyOn(global, 'window', 'get');
// @ts-ignore - Test
windowSpy.mockReturnValue(undefined);
expect(inBrowser()).toBe(false);
Expand All @@ -21,16 +23,23 @@ describe('isValidBrowser', () => {
let webdriverGetter: any;

beforeEach(() => {
userAgentGetter = jest.spyOn(window.navigator, 'userAgent', 'get');
webdriverGetter = jest.spyOn(window.navigator, 'webdriver', 'get');
userAgentGetter = vi.spyOn(window.navigator, 'userAgent', 'get');
// Define webdriver property if it doesn't exist
if (!('webdriver' in window.navigator)) {
Object.defineProperty(window.navigator, 'webdriver', {
configurable: true,
get: () => false,
});
}
webdriverGetter = vi.spyOn(window.navigator, 'webdriver', 'get');
Comment on lines +27 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Good defensive coding for JSDOM compatibility.

The code checks if the webdriver property exists on window.navigator before defining it. This suggests that Vitest's JSDOM environment may not include this property by default, unlike Jest's environment.

However, verify whether this defensive check is necessary. If Vitest consistently lacks this property, consider defining it unconditionally in the global test setup (vitest.setup.mts) rather than in individual test files to reduce duplication.

Run the following script to check if this pattern appears in other test files:


🏁 Script executed:

#!/bin/bash
# Search for similar defensive property definitions in test files
rg -A 5 "if \(!'.*' in window\.navigator\)" --type ts --glob '**/__tests__/**'

Length of output: 0


I need to try different search approaches since the regex pattern didn't match. Let me search more broadly:


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for webdriver property usage in test files
rg -n "webdriver" --type ts --glob '**/__tests__/**' -B 2 -A 2

Length of output: 9467


🏁 Script executed:

#!/bin/bash
# Search for Object.defineProperty usage in test files
rg -n "Object\.defineProperty" --type ts --glob '**/__tests__/**' -B 2 -A 2

Length of output: 39064


🏁 Script executed:

#!/bin/bash
# Look for vitest setup files
fd -e mts -e ts --glob '**/vitest*' -o --glob '**/setup*' 2>/dev/null | head -20

Length of output: 0


Consider extracting the repeated webdriver property setup into a helper function to avoid duplication.

The defensive webdriver property check appears twice in the same file:

  • Lines 28–33 (in isValidBrowser describe block)
  • Lines 113–118 (in isValidBrowserOnline describe block)

Since the logic is identical and JSDOM appears to consistently lack this property in both test suites, extract this into a shared setup helper to reduce duplication and improve maintainability:

const setupWebdriverProperty = () => {
  if (!('webdriver' in window.navigator)) {
    Object.defineProperty(window.navigator, 'webdriver', {
      configurable: true,
      get: () => false,
    });
  }
};

Then call setupWebdriverProperty() in each beforeEach instead of repeating the full block.

🤖 Prompt for AI Agents
In packages/shared/src/__tests__/browser.spec.ts around lines 27 to 34, the
defensive webdriver property check is duplicated elsewhere; extract the repeated
block into a small helper function (e.g., setupWebdriverProperty) that checks
for 'webdriver' on window.navigator and defines it when missing, then call that
helper from each beforeEach instead of repeating the Object.defineProperty
block; update imports/locals as needed and ensure the vi.spyOn call still
references window.navigator.webdriver after the helper runs.

});

afterEach(() => {
jest.restoreAllMocks();
vi.restoreAllMocks();
});

it('returns false if not in browser', () => {
const windowSpy = jest.spyOn(global, 'window', 'get');
const windowSpy = vi.spyOn(global, 'window', 'get');
// @ts-ignore - Test
windowSpy.mockReturnValue(undefined);

Expand Down Expand Up @@ -99,11 +108,27 @@ describe('isValidBrowserOnline', () => {
let connectionGetter: any;

beforeEach(() => {
userAgentGetter = jest.spyOn(window.navigator, 'userAgent', 'get');
webdriverGetter = jest.spyOn(window.navigator, 'webdriver', 'get');
onLineGetter = jest.spyOn(window.navigator, 'onLine', 'get');
userAgentGetter = vi.spyOn(window.navigator, 'userAgent', 'get');
// Define webdriver property if it doesn't exist
if (!('webdriver' in window.navigator)) {
Object.defineProperty(window.navigator, 'webdriver', {
configurable: true,
get: () => false,
});
}
webdriverGetter = vi.spyOn(window.navigator, 'webdriver', 'get');
onLineGetter = vi.spyOn(window.navigator, 'onLine', 'get');
// Define connection property if it doesn't exist
// @ts-ignore
if (!('connection' in window.navigator)) {
// @ts-ignore
Object.defineProperty(window.navigator, 'connection', {
configurable: true,
get: () => ({ downlink: 10, rtt: 100 }),
});
}
// @ts-ignore
connectionGetter = jest.spyOn(window.navigator, 'connection', 'get');
connectionGetter = vi.spyOn(window.navigator, 'connection', 'get');
});

it('returns TRUE if connection is online, navigator is online, has disabled webdriver, and not a bot', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { expect, test } from 'vitest';

import { buildAccountsBaseUrl } from '../buildAccountsBaseUrl';

test.each([
Expand Down
72 changes: 72 additions & 0 deletions packages/shared/src/__tests__/color.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import type { Color } from '@clerk/types';
import { describe, expect, it } from 'vitest';

import { colorToSameTypeString, hexStringToRgbaColor, stringToHslaColor, stringToSameTypeColor } from '../color';

describe('stringToHslaColor(color)', function () {
const hsla = { h: 195, s: 1, l: 0.5 };
const cases: Array<[string, Color | null]> = [
['', null],
['transparent', { h: 0, s: 0, l: 0, a: 0 }],
['#00bfff', hsla],
['00bfff', hsla],
['rgb(0, 191, 255)', hsla],
['rgba(0, 191, 255, 0.3)', { ...hsla, a: 0.3 }],
];

it.each(cases)('.stringToHslaColor(%s) => %s', (a, expected) => {
expect(stringToHslaColor(a)).toEqual(expected);
});
});

describe('hexStringToRgbaColor(color)', function () {
const cases: Array<[string, Color | null]> = [
['#00bfff', { r: 0, g: 191, b: 255 }],
['00bfff', { r: 0, g: 191, b: 255 }],
];

it.each(cases)('.hexStringToRgbaColor(%s) => %s', (a, expected) => {
expect(hexStringToRgbaColor(a)).toEqual(expected);
});
});

describe('stringToSameTypeColor(color)', function () {
const cases: Array<[string, Color | null]> = [
['', ''],
['invalid', ''],
['12ff12', '#12ff12'],
['#12ff12', '#12ff12'],
['1ff', '#1ff'],
['transparent', 'transparent'],
['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
Comment on lines +41 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Remove duplicate test cases.

Lines 43-44 are exact duplicates of lines 41-42 (testing the same rgb/rgba inputs). Remove the redundant entries.

Apply this diff:

     ['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
     ['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
-    ['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
-    ['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
     ['hsl(244,66%,33%)', { h: 244, s: 0.66, l: 0.33, a: undefined }],
📝 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.

Suggested change
['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
['rgb(100,100,100)', { r: 100, g: 100, b: 100, a: undefined }],
['rgba(100,100,100,0.5)', { r: 100, g: 100, b: 100, a: 0.5 }],
🤖 Prompt for AI Agents
In packages/shared/src/__tests__/color.spec.ts around lines 41 to 44 there are
duplicated test entries for 'rgb(100,100,100)' and 'rgba(100,100,100,0.5)';
remove the redundant lines (the second occurrence on lines 43-44) so each input
case appears only once in the test array.

['hsl(244,66%,33%)', { h: 244, s: 0.66, l: 0.33, a: undefined }],
['hsla(244,66%,33%,0.5)', { h: 244, s: 0.66, l: 0.33, a: 0.5 }],
['hsl(244,66%,33)', ''],
['hsla(244,66%,33,0.5)', ''],
];

it.each(cases)('.stringToSameTypeColor(%s) => %s', (a, expected) => {
expect(stringToSameTypeColor(a)).toEqual(expected);
});
});

describe('colorToSameTypeString(color)', function () {
const cases: Array<[Color, string]> = [
['', ''],
['invalid', ''],
['#12ff12', '#12ff12'],
['#12ff12', '#12ff12'],
['#1ff', '#1ff'],
Comment on lines +57 to +62
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix type annotation and remove duplicate test case.

Two issues:

  1. Type mismatch: The type annotation Array<[Color, string]> indicates the first element should be a Color object, but lines 58-62 use string literals. If colorToSameTypeString accepts both Color objects and strings, update the type annotation to reflect this (e.g., Array<[Color | string, string]>).

  2. Duplicate test case: Lines 60-61 both test '#12ff12'.

Apply this diff to fix the type annotation and remove the duplicate:

-  const cases: Array<[Color, string]> = [
+  const cases: Array<[Color | string, string]> = [
     ['', ''],
     ['invalid', ''],
     ['#12ff12', '#12ff12'],
-    ['#12ff12', '#12ff12'],
     ['#1ff', '#1ff'],
📝 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.

Suggested change
const cases: Array<[Color, string]> = [
['', ''],
['invalid', ''],
['#12ff12', '#12ff12'],
['#12ff12', '#12ff12'],
['#1ff', '#1ff'],
const cases: Array<[Color | string, string]> = [
['', ''],
['invalid', ''],
['#12ff12', '#12ff12'],
['#1ff', '#1ff'],
🤖 Prompt for AI Agents
In packages/shared/src/__tests__/color.spec.ts around lines 57 to 62, the test
cases array is annotated as Array<[Color, string]> but uses string literals and
contains a duplicate '#12ff12' entry; update the type annotation to Array<[Color
| string, string]> (or Array<[string | Color, string]>) so strings are allowed,
and remove the duplicate '#12ff12' test case so each case is unique.

[{ r: 100, g: 100, b: 100, a: undefined }, 'rgb(100,100,100)'],
[{ r: 100, g: 100, b: 100, a: 0.5 }, 'rgba(100,100,100,0.5)'],
[{ h: 100, s: 0.55, l: 0.33, a: undefined }, 'hsl(100,55%,33%)'],
[{ h: 100, s: 1, l: 1, a: 0.5 }, 'hsla(100,100%,100%,0.5)'],
];

it.each(cases)('.colorToSameTypeString(%s) => %s', (a, expected) => {
expect(colorToSameTypeString(a)).toEqual(expected);
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { describe, expect, it } from 'vitest';

import type { RelativeDateCase } from '../date';
import { addYears, dateTo12HourTime, differenceInCalendarDays, formatRelative } from '../date';

Expand Down
Loading