fix: correct typeof comparisons in browser SDK#1301
Conversation
877eb21 to
10ab2b8
Compare
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
There was a problem hiding this comment.
🟡 addWindowEventListener guards on isDocument() instead of isWindow(), now exposed by the typeof fix
The addWindowEventListener function at packages/sdk/browser/src/BrowserApi.ts:53 checks isDocument() instead of isWindow() before accessing window. Before this PR, this didn't matter because both isDocument() and isWindow() always returned true (due to the broken typeof x !== undefined comparison). Now that the typeof checks are fixed, these functions correctly distinguish between document and window availability. In an environment where window is available but document is not, addWindowEventListener would incorrectly return a no-op instead of registering the event. This function is used by BrowserStateDetector.ts:21 (for pagehide events) and LocationWatcher.ts:46 (for popstate events).
(Refers to line 53)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Will address separately (this is an existing bug)
|
|
||
| export default function randomUuidV4(): string { | ||
| if (typeof crypto !== undefined && typeof crypto.randomUUID === 'function') { | ||
| if (typeof crypto !== 'undefined' && typeof crypto.randomUUID === 'function') { |
There was a problem hiding this comment.
🟡 getRandom128bit uses bare crypto reference instead of typeof check, making Math.random fallback unreachable
In both packages/sdk/browser/src/platform/randomUuidV4.ts:39 and packages/telemetry/browser-telemetry/src/randomUuidV4.ts:42, getRandom128bit() checks if (crypto && crypto.getRandomValues) using a bare reference to crypto. If crypto is completely undefined (not just lacking getRandomValues), this throws a ReferenceError instead of falling through to the Math.random fallback. Before this PR, this code path was unreachable because the broken typeof crypto !== undefined in randomUuidV4() always evaluated to true, causing a crash earlier. Now that the typeof check in randomUuidV4() is fixed, the function correctly falls through to fallbackUuidV4() → getRandom128bit() when crypto is missing, but the bare crypto reference there will throw, making the Math.random fallback still unreachable in environments without crypto.
Prompt for agents
The fix in randomUuidV4() now correctly handles the case where crypto is completely undefined by falling through to fallbackUuidV4(). However, getRandom128bit() (line 39 in packages/sdk/browser/src/platform/randomUuidV4.ts and line 42 in packages/telemetry/browser-telemetry/src/randomUuidV4.ts) uses a bare reference to crypto (if (crypto && crypto.getRandomValues)) which throws a ReferenceError if crypto is not defined at all. The Math.random fallback on lines 44-49 is unreachable in this case. The fix should change the crypto check in getRandom128bit() to use typeof crypto !== 'undefined' && crypto.getRandomValues in both files. This ensures the Math.random fallback is actually reachable when crypto is unavailable.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
existing issue will address in another PR
Fix typeof comparisons that compared against the value `undefined` instead of the string `'undefined'`. Since typeof always returns a string, comparing to the value `undefined` made these checks always truthy. Affected functions: - isDocument() and isWindow() in BrowserApi.ts now correctly return false in service worker contexts where document/window are unavailable - 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 catch this class of bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- BrowserApi: verify isDocument() and isWindow() return true when globals exist - randomUuidV4: verify fallback path when crypto.randomUUID is unavailable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verify isDocument() and isWindow() return false when the globals are undefined, simulating a service worker environment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7cc1435 to
074d3f3
Compare
🤖 I have created a release *beep* *boop* --- <details><summary>browser: 0.1.16</summary> ## [0.1.16](browser-v0.1.15...browser-v0.1.16) (2026-04-21) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-client-sdk bumped from 4.6.0 to 4.6.1 </details> <details><summary>browser-telemetry: 1.0.32</summary> ## [1.0.32](browser-telemetry-v1.0.31...browser-telemetry-v1.0.32) (2026-04-21) ### Bug Fixes * correct typeof comparisons in browser SDK ([#1301](#1301)) ([f4bd636](f4bd636)) * **js-client-sdk:** better `undefined` handling ([#1303](#1303)) ([4818678](4818678)) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/js-client-sdk bumped from 4.6.0 to 4.6.1 </details> <details><summary>js-client-sdk: 4.6.1</summary> ## [4.6.1](js-client-sdk-v4.6.0...js-client-sdk-v4.6.1) (2026-04-21) ### Bug Fixes * correct typeof comparisons in browser SDK ([#1301](#1301)) ([f4bd636](f4bd636)) * **js-client-sdk:** better `undefined` handling ([#1303](#1303)) ([4818678](4818678)) </details> <details><summary>react-sdk: 0.2.2</summary> ## [0.2.2](react-sdk-v0.2.1...react-sdk-v0.2.2) (2026-04-21) ### Dependencies * The following workspace dependencies were updated * dependencies * @launchdarkly/js-client-sdk bumped from ^4.6.0 to ^4.6.1 </details> <details><summary>server-sdk-ai: 0.17.0</summary> ## [0.17.0](server-sdk-ai-v0.16.8...server-sdk-ai-v0.17.0) (2026-04-21) ### ⚠ BREAKING CHANGES * Flatten JudgeResponse and EvalScore into new LDJudgeResult ([#1284](#1284)) * Add per-execution runId, at-most-once tracking, and cross-process tracker resumption ([#1270](#1270)) ### Features * Add per-execution runId, at-most-once tracking, and cross-process tracker resumption ([#1270](#1270)) ([fc25ab7](fc25ab7)) * Flatten JudgeResponse and EvalScore into new LDJudgeResult ([#1284](#1284)) ([aba1221](aba1221)) * Implement agent graph definitions ([#1282](#1282)) ([e7d08e5](e7d08e5)) * simplify evaluation schema to flat score/reasoning shape ([#1286](#1286)) ([c132e9f](c132e9f)) ### Bug Fixes * Add support for graph metric tracking ([#1269](#1269)) ([034a89d](034a89d)) </details> <details><summary>server-sdk-ai-langchain: 0.5.5</summary> ## [0.5.5](server-sdk-ai-langchain-v0.5.4...server-sdk-ai-langchain-v0.5.5) (2026-04-21) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/server-sdk-ai bumped from ^0.16.8 to ^0.17.0 * peerDependencies * @launchdarkly/server-sdk-ai bumped from ^0.15.0 || ^0.16.0 to ^0.17.0 </details> <details><summary>server-sdk-ai-openai: 0.5.5</summary> ## [0.5.5](server-sdk-ai-openai-v0.5.4...server-sdk-ai-openai-v0.5.5) (2026-04-21) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/server-sdk-ai bumped from ^0.16.8 to ^0.17.0 * peerDependencies * @launchdarkly/server-sdk-ai bumped from ^0.15.0 || ^0.16.0 to ^0.17.0 </details> <details><summary>server-sdk-ai-vercel: 0.5.5</summary> ## [0.5.5](server-sdk-ai-vercel-v0.5.4...server-sdk-ai-vercel-v0.5.5) (2026-04-21) ### Dependencies * The following workspace dependencies were updated * devDependencies * @launchdarkly/server-sdk-ai bumped from ^0.16.8 to ^0.17.0 * peerDependencies * @launchdarkly/server-sdk-ai bumped from ^0.15.0 || ^0.16.0 to ^0.17.0 </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Primarily a version/changelog bump, but it publishes `@launchdarkly/server-sdk-ai` `0.17.0` with documented breaking API changes that can impact downstream consumers and provider peer dependency resolution. > > **Overview** > Bumps release versions across the monorepo via `.release-please-manifest.json`, updating `@launchdarkly/server-sdk-ai` to `0.17.0`, `@launchdarkly/js-client-sdk` to `4.6.1`, and related packages (`@launchdarkly/browser`, `@launchdarkly/react-sdk`, `@launchdarkly/browser-telemetry`, and AI provider packages) accordingly. > > Updates package metadata, changelogs, examples, and embedded SDK/wrapper version strings (e.g., `BrowserInfo` and `LDReactClient`) to reflect the new releases, including `server-sdk-ai`’s `0.17.0` breaking-change notes and provider peer dependency bumps to `^0.17.0`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e7f8c09. 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: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: jsonbailey <jbailey@launchdarkly.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
typeof x !== undefined(always true) totypeof x !== 'undefined'(correct) in 5 locationsisDocument()andisWindow()now correctly returnfalsein service worker contextsgetCrypto()now correctly throws when crypto API is unavailablerandomUuidV4()now correctly falls back whencrypto.randomUUIDis missingvalid-typeofESLint rule to prevent this class of bugContext
These bugs were introduced in Sept-Oct 2024. Since
typeofalways returns a string, comparing against the valueundefined(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 lintpasses with 0 errorsyarn workspace @launchdarkly/js-client-sdk testpassesyarn workspace @launchdarkly/browser-telemetry testpassesDepends on #1299.
🤖 Generated with Claude Code
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 !== undefinedchecks totypeof x !== 'undefined'in the browser SDK and browser telemetry UUID helper, so non-browser contexts no longer incorrectly treatdocument,window, orcryptoas available.Re-enables ESLint
valid-typeofto prevent regressions, and adds Jest coverage to ensureisDocument/isWindowreturnfalsewhen globals are missing andrandomUuidV4properly falls back whencrypto.randomUUIDis unavailable.Reviewed by Cursor Bugbot for commit 074d3f3. Bugbot is set up for automated code reviews on this repo. Configure here.