Skip to content

Commit f4bd636

Browse files
joker23claude
andauthored
fix: correct typeof comparisons in browser SDK (#1301)
## Summary - Fix `typeof x !== undefined` (always true) to `typeof x !== 'undefined'` (correct) in 5 locations - `isDocument()` and `isWindow()` now correctly return `false` in service worker contexts - `getCrypto()` now correctly throws when crypto API is unavailable - `randomUuidV4()` now correctly falls back when `crypto.randomUUID` is missing - Re-enables the `valid-typeof` ESLint rule to prevent this class of bug ## Context These bugs were introduced in Sept-Oct 2024. Since `typeof` always returns a string, comparing against the value `undefined` (not the string `'undefined'`) made these checks always truthy. The bug was masked because the previous ESLint config did not flag this pattern. ## Affected files - `packages/sdk/browser/src/BrowserApi.ts` (3 locations) - `packages/sdk/browser/src/platform/randomUuidV4.ts` (1 location) - `packages/telemetry/browser-telemetry/src/randomUuidV4.ts` (1 location) ## Test plan - [ ] `yarn workspaces foreach -p run lint` passes with 0 errors - [ ] `yarn workspace @launchdarkly/js-client-sdk test` passes - [ ] `yarn workspace @launchdarkly/browser-telemetry test` passes Depends on #1299. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Fixes environment/crypto detection and UUID generation fallback logic in browser-facing SDK code; while small, it can change behavior in non-browser contexts (service workers/SSR) and affects ID generation paths. > > **Overview** > Corrects several invalid `typeof x !== undefined` checks to `typeof x !== 'undefined'` in the browser SDK and browser telemetry UUID helper, so non-browser contexts no longer incorrectly treat `document`, `window`, or `crypto` as available. > > Re-enables ESLint `valid-typeof` to prevent regressions, and adds Jest coverage to ensure `isDocument`/`isWindow` return `false` when globals are missing and `randomUuidV4` properly falls back when `crypto.randomUUID` is unavailable. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 074d3f3. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent eca74c5 commit f4bd636

6 files changed

Lines changed: 52 additions & 7 deletions

File tree

.eslintrc.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ module.exports = {
2828
'eqeqeq': ['error', 'always', { null: 'ignore' }],
2929
'prefer-const': ['error', { ignoreReadBeforeAssign: true }],
3030
'no-var': 'error',
31-
'valid-typeof': 'off',
31+
'valid-typeof': 'error',
3232
'no-restricted-syntax': [
3333
'error',
3434
{ selector: 'ForInStatement', message: 'Use Object.{keys,values,entries} instead.' },
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { isDocument, isWindow } from '../src/BrowserApi';
2+
3+
it('isDocument returns true when document is defined', () => {
4+
expect(isDocument()).toBe(true);
5+
});
6+
7+
it('isDocument returns false when document is not defined', () => {
8+
const original = Object.getOwnPropertyDescriptor(globalThis, 'document');
9+
Object.defineProperty(globalThis, 'document', { value: undefined, configurable: true });
10+
try {
11+
expect(isDocument()).toBe(false);
12+
} finally {
13+
Object.defineProperty(globalThis, 'document', original!);
14+
}
15+
});
16+
17+
it('isWindow returns true when window is defined', () => {
18+
expect(isWindow()).toBe(true);
19+
});
20+
21+
it('isWindow returns false when window is not defined', () => {
22+
const original = Object.getOwnPropertyDescriptor(globalThis, 'window');
23+
Object.defineProperty(globalThis, 'window', { value: undefined, configurable: true });
24+
try {
25+
expect(isWindow()).toBe(false);
26+
} finally {
27+
Object.defineProperty(globalThis, 'window', original!);
28+
}
29+
});

packages/sdk/browser/__tests__/platform/randomUuidV4.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { fallbackUuidV4, formatDataAsUuidV4 } from '../../src/platform/randomUuidV4';
1+
import randomUuidV4, { fallbackUuidV4, formatDataAsUuidV4 } from '../../src/platform/randomUuidV4';
22

33
it('formats conformant UUID', () => {
44
// For this test we remove the random component and just inspect the variant and version.
@@ -26,3 +26,19 @@ it('formats conformant UUID', () => {
2626
expect(specifierB).toEqual(0x8);
2727
expect(specifierC).toEqual(0x8);
2828
});
29+
30+
it('falls back when crypto.randomUUID is unavailable', () => {
31+
const original = globalThis.crypto;
32+
Object.defineProperty(globalThis, 'crypto', {
33+
value: { getRandomValues: original.getRandomValues.bind(original) },
34+
configurable: true,
35+
});
36+
37+
try {
38+
const uuid = randomUuidV4();
39+
expect(uuid).toHaveLength(36);
40+
expect(uuid[14]).toEqual('4');
41+
} finally {
42+
Object.defineProperty(globalThis, 'crypto', { value: original, configurable: true });
43+
}
44+
});

packages/sdk/browser/src/BrowserApi.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@
55
*/
66

77
export function isDocument() {
8-
return typeof document !== undefined;
8+
return typeof document !== 'undefined';
99
}
1010

1111
export function isWindow() {
12-
return typeof window !== undefined;
12+
return typeof window !== 'undefined';
1313
}
1414

1515
/**
@@ -91,7 +91,7 @@ export function getLocationHash(): string {
9191
}
9292

9393
export function getCrypto(): Crypto {
94-
if (typeof crypto !== undefined) {
94+
if (typeof crypto !== 'undefined') {
9595
return crypto;
9696
}
9797
// This would indicate running in an environment that doesn't have window.crypto or self.crypto.

packages/sdk/browser/src/platform/randomUuidV4.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export function fallbackUuidV4(): string {
9191
}
9292

9393
export default function randomUuidV4(): string {
94-
if (typeof crypto !== undefined && typeof crypto.randomUUID === 'function') {
94+
if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') {
9595
return crypto.randomUUID();
9696
}
9797

packages/telemetry/browser-telemetry/src/randomUuidV4.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export function fallbackUuidV4(): string {
9494
}
9595

9696
export default function randomUuidV4(): string {
97-
if (typeof crypto !== undefined && typeof crypto.randomUUID === 'function') {
97+
if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') {
9898
return crypto.randomUUID();
9999
}
100100

0 commit comments

Comments
 (0)