fix: remove uuid dependency, replace with inline implementation#146
Open
kinyoklion wants to merge 3 commits into
Open
fix: remove uuid dependency, replace with inline implementation#146kinyoklion wants to merge 3 commits into
kinyoklion wants to merge 3 commits into
Conversation
Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
uuid v12+ dropped CommonJS support, so v11 is the latest compatible version for this CommonJS project. No API changes were required as the v1 export remains stable. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
Remove the uuid package dependency and replace it with an inline UUID v4 implementation adapted from js-core's browser SDK polyfill. This avoids the CommonJS compatibility issue with uuid v12+ while also eliminating the external dependency entirely. Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
kinyoklion
commented
May 15, 2026
| // Adapted from: | ||
| // https://github.com/launchdarkly/js-core/blob/main/packages/sdk/browser/src/platform/randomUuidV4.ts | ||
|
|
||
| // It uses crypto.randomUUID when available. |
Member
Author
There was a problem hiding this comment.
Generally speaking this will use crypto.randomUUID and then fallback to the polyfill when it isn't available.
Contributor
There was a problem hiding this comment.
Correct — in Node 18+ crypto is available as a global, so it will use crypto.getRandomValues() (or crypto.randomUUID() where supported). The Math.random() fallback only kicks in for environments where crypto isn't globally available.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements
Related issues
N/A
Describe the solution you've provided
Removes the
uuidpackage dependency entirely and replaces it with an inline UUID v4 implementation adapted from js-core's browser SDK polyfill.The implementation uses
crypto.randomUUID()when available, falls back tocrypto.getRandomValues(), and finally toMath.random().All three usages (
diagnosticEvents.js,AnonymousContextProcessor.js,EventSender.js) now use UUID v4 instead of v1. The diagnostic events spec originally called for v4 anyway.Describe alternatives you've considered
Additional context
All 635 tests pass after the change.
Link to Devin session: https://app.devin.ai/sessions/63cc8da489324f20b17db43d0d296dea
Requested by: @kinyoklion
Note
Medium Risk
Changes how anonymous context keys, event payload IDs, and diagnostic IDs are generated (switching from
uuidv1 to an internal UUID v4 generator), which could affect ID format/uniqueness assumptions in downstream systems.Overview
Removes the external
uuiddependency and replaces all UUID generation with a new internalsrc/uuid.jshelper that preferscrypto.randomUUID, thencrypto.getRandomValues, thenMath.random.Updates
AnonymousContextProcessor,EventSender, anddiagnosticEventsto generate UUID v4 identifiers (anonymous context keys,X-LaunchDarkly-Payload-ID, and diagnostic IDs). Adds a unit test assertingDiagnosticId().diagnosticIdmatches UUID v4 format.Reviewed by Cursor Bugbot for commit f28ce25. Bugbot is set up for automated code reviews on this repo. Configure here.