Skip to content

fix: remove uuid dependency, replace with inline implementation#146

Open
kinyoklion wants to merge 3 commits into
mainfrom
devin/1778884546-update-uuid-package
Open

fix: remove uuid dependency, replace with inline implementation#146
kinyoklion wants to merge 3 commits into
mainfrom
devin/1778884546-update-uuid-package

Conversation

@kinyoklion
Copy link
Copy Markdown
Member

@kinyoklion kinyoklion commented May 15, 2026

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

N/A

Describe the solution you've provided

Removes the uuid package 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 to crypto.getRandomValues(), and finally to Math.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

  • Upgrading uuid to v11 (latest CJS-compatible version) — still keeps the external dependency.
  • Upgrading to uuid v14 (latest) — not feasible as it dropped CommonJS support entirely.

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 uuid v1 to an internal UUID v4 generator), which could affect ID format/uniqueness assumptions in downstream systems.

Overview
Removes the external uuid dependency and replaces all UUID generation with a new internal src/uuid.js helper that prefers crypto.randomUUID, then crypto.getRandomValues, then Math.random.

Updates AnonymousContextProcessor, EventSender, and diagnosticEvents to generate UUID v4 identifiers (anonymous context keys, X-LaunchDarkly-Payload-ID, and diagnostic IDs). Adds a unit test asserting DiagnosticId().diagnosticId matches UUID v4 format.

Reviewed by Cursor Bugbot for commit f28ce25. Bugbot is set up for automated code reviews on this repo. Configure here.

devin-ai-integration Bot and others added 3 commits May 15, 2026 22:36
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>
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@kinyoklion kinyoklion marked this pull request as ready for review May 15, 2026 22:44
@kinyoklion kinyoklion requested a review from a team as a code owner May 15, 2026 22:44
Comment thread src/uuid.js
// Adapted from:
// https://github.com/launchdarkly/js-core/blob/main/packages/sdk/browser/src/platform/randomUuidV4.ts

// It uses crypto.randomUUID when available.
Copy link
Copy Markdown
Member Author

@kinyoklion kinyoklion May 15, 2026

Choose a reason for hiding this comment

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

Generally speaking this will use crypto.randomUUID and then fallback to the polyfill when it isn't available.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant