Skip to content

refactor: simplify getting initializationState#1202

Open
joker23 wants to merge 5 commits intomainfrom
skz/sdk-1945/initialize-state-tracking
Open

refactor: simplify getting initializationState#1202
joker23 wants to merge 5 commits intomainfrom
skz/sdk-1945/initialize-state-tracking

Conversation

@joker23
Copy link
Contributor

@joker23 joker23 commented Mar 18, 2026

This PR will align our initialization status handling closer to the base browser sdk by:

  • combining initializing and unknown state which should get rid of one extra state dispatch on initialization
  • adding additional idempotent guards to the start function. Note that these are redundant protections.

Open with Devin

Note

Medium Risk
Medium risk because it changes the public initialization-state contract (dropping unknown) and alters server-side/noop client behavior to return a failure + error, which may affect consumers that previously treated these as non-error states.

Overview
Initialization state handling is simplified by removing the unknown state from InitializedState/InitializationStatus and defaulting new clients and test harnesses to initializing.

createClient() behavior changes: start() now uses explicit idempotency guards (startCalled/startNotified) and triggers a one-time onContextChange notification after the first successful start() completion; the server-side/noop client now reports failed with a concrete getInitializationError() message instead of returning unknown/undefined.

Tests and the migration docs are updated to reflect the new state model and server-side error semantics.

Written by Cursor Bugbot for commit 86d0919. This will update automatically on new commits. Configure here.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 18, 2026

@launchdarkly/js-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 25661 bytes
Compressed size limit: 29000
Uncompressed size: 126143 bytes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 18, 2026

@launchdarkly/js-client-sdk size report
This is the brotli compressed size of the ESM build.
Compressed size: 24981 bytes
Compressed size limit: 25000
Uncompressed size: 86916 bytes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 18, 2026

@launchdarkly/js-client-sdk-common size report
This is the brotli compressed size of the ESM build.
Compressed size: 29156 bytes
Compressed size limit: 38000
Uncompressed size: 156220 bytes

@github-actions
Copy link
Contributor

github-actions bot commented Mar 18, 2026

@launchdarkly/browser size report
This is the brotli compressed size of the ESM build.
Compressed size: 172978 bytes
Compressed size limit: 200000
Uncompressed size: 804043 bytes

@joker23 joker23 force-pushed the skz/sdk-1945/initialize-state-tracking branch 2 times, most recently from a5d4d5f to 85fce1b Compare March 18, 2026 23:35
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@joker23
Copy link
Contributor Author

joker23 commented Mar 19, 2026

@cursor review

cursor[bot]

This comment was marked as resolved.

@joker23 joker23 force-pushed the skz/sdk-1945/initialize-state-tracking branch from 85fce1b to d710974 Compare March 19, 2026 19:08
@joker23 joker23 force-pushed the skz/sdk-1945/initialize-state-tracking branch from d710974 to 2da69a5 Compare March 19, 2026 19:38
@joker23
Copy link
Contributor Author

joker23 commented Mar 19, 2026

@cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

@joker23 joker23 marked this pull request as ready for review March 19, 2026 20:11
@joker23 joker23 requested a review from a team as a code owner March 19, 2026 20:11
function notifyContextSubscribers() {
const newContext = baseClient.getContext();
if (newContext) {
subscribers.forEach((cb) => cb(newContext));
Copy link
Member

Choose a reason for hiding this comment

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

Should these be ran in another turn of the event loop? Or do we want them inline.

Basically do you want the result of the identify before the context notification, or after?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can only notify after the identify promise is resolved... I don't think we can really inline this as we are using the browser sdk identify method.

devin-ai-integration[bot]

This comment was marked as resolved.

cursor[bot]

This comment was marked as resolved.

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
cursor[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@joker23 joker23 requested a review from kinyoklion March 20, 2026 20:49
We should notify the context change before notifying the initialization result
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants