fix: issues from rebase

Author: joker23

packages/sdk/browser/src/BrowserClient.ts

@@ -16,7 +16,6 @@ import {
   LDHeaders,
   LDIdentifyResult,
   LDPluginEnvironmentMetadata,
-  LDTimeoutError,
   Platform,
   safeRegisterDebugOverridePlugins,
 } from '@launchdarkly/js-client-sdk-common';

packages/shared/sdk-client/__tests__/flag-manager/FlagManager.test.ts

@@ -118,15 +118,22 @@ describe('FlagManager override tests', () => {
     expect(flagManager.get('test-flag')?.flag.value).toBe('override-value');
   });
 
-  it('setOverride triggers flag change callback', () => {
+  it('setOverride triggers flag change callback', async () => {
+    const context = Context.fromLDContext({ kind: 'user', key: 'user-key' });
+    const flags = {
+      'test-flag': makeMockItemDescriptor(1, 'store-value'),
+    };
+
+    await flagManager.init(context, flags);
+
     const mockCallback: FlagsChangeCallback = jest.fn();
     flagManager.on(mockCallback);
 
     const debugOverride = flagManager.getDebugOverride();
     debugOverride?.setOverride('test-flag', 'override-value');
 
     expect(mockCallback).toHaveBeenCalledTimes(1);
-    expect(mockCallback).toHaveBeenCalledWith(null, ['test-flag'], 'override');
+    expect(mockCallback).toHaveBeenCalledWith(context, ['test-flag'], 'override');
   });
 
   it('removeOverride does nothing when override does not exist', () => {
@@ -151,7 +158,14 @@ describe('FlagManager override tests', () => {
     expect(flagManager.get('test-flag')?.flag.value).toBe('store-value');
   });
 
-  it('removeOverride triggers flag change callback', () => {
+  it('removeOverride triggers flag change callback', async () => {
+    const context = Context.fromLDContext({ kind: 'user', key: 'user-key' });
+    const flags = {
+      'test-flag': makeMockItemDescriptor(1, 'store-value'),
+    };
+
+    await flagManager.init(context, flags);
+
     const mockCallback: FlagsChangeCallback = jest.fn();
     flagManager.on(mockCallback);
 
@@ -160,8 +174,8 @@ describe('FlagManager override tests', () => {
     debugOverride?.removeOverride('test-flag');
 
     expect(mockCallback).toHaveBeenCalledTimes(2);
-    expect(mockCallback).toHaveBeenNthCalledWith(1, null, ['test-flag'], 'override');
-    expect(mockCallback).toHaveBeenNthCalledWith(2, null, ['test-flag'], 'override');
+    expect(mockCallback).toHaveBeenNthCalledWith(1, context, ['test-flag'], 'override');
+    expect(mockCallback).toHaveBeenNthCalledWith(2, context, ['test-flag'], 'override');
   });
 
   it('clearAllOverrides removes all overrides', () => {
@@ -176,18 +190,25 @@ describe('FlagManager override tests', () => {
     expect(Object.keys(flagManager.getAllOverrides())).toHaveLength(0);
   });
 
-  it('clearAllOverrides triggers flag change callback for all flags', () => {
+  it('clearAllOverrides triggers flag change callback for all flags', async () => {
     const mockCallback: FlagsChangeCallback = jest.fn();
     flagManager.on(mockCallback);
 
     const debugOverride = flagManager.getDebugOverride();
+    const context = Context.fromLDContext({ kind: 'user', key: 'user-key' });
+    const flags = {
+      'test-flag': makeMockItemDescriptor(1, 'store-value'),
+    };
+
+    await flagManager.init(context, flags);
+
     debugOverride?.setOverride('flag1', 'value1');
     debugOverride?.setOverride('flag2', 'value2');
     (mockCallback as jest.Mock).mockClear();
 
     debugOverride?.clearAllOverrides();
     expect(mockCallback).toHaveBeenCalledTimes(1);
-    expect(mockCallback).toHaveBeenCalledWith(null, ['flag1', 'flag2'], 'override');
+    expect(mockCallback).toHaveBeenCalledWith(context, ['flag1', 'flag2'], 'override');
   });
 
   it('getAllOverrides returns all overrides as ItemDescriptors', () => {

packages/shared/sdk-client/src/LDClientImpl.ts

@@ -132,7 +132,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
 
     this._flagManager.on((context, flagKeys, type) => {
       this._handleInspectionChanged(flagKeys, type);
-      const ldContext = context ? Context.toLDContext(context) : null;
+      const ldContext = Context.toLDContext(context);
       this.emitter.emit('change', ldContext, flagKeys);
       flagKeys.forEach((it) => {
         this.emitter.emit(`change:${it}`, ldContext);

packages/shared/sdk-client/src/flag-manager/FlagManager.ts

@@ -239,7 +239,7 @@ export default class DefaultFlagManager implements FlagManager {
       this._overrides = {};
     }
     this._overrides[key] = value;
-    this._flagUpdater.handleFlagChanges(null, [key], 'override');
+    this._flagUpdater.handleFlagChanges([key], 'override');
   }
 
   removeOverride(flagKey: string) {
@@ -254,14 +254,14 @@ export default class DefaultFlagManager implements FlagManager {
       this._overrides = undefined;
     }
 
-    this._flagUpdater.handleFlagChanges(null, [flagKey], 'override');
+    this._flagUpdater.handleFlagChanges([flagKey], 'override');
   }
 
   clearAllOverrides() {
     if (this._overrides) {
       const clearedOverrides = { ...this._overrides };
       this._overrides = undefined; // Reset to undefined
-      this._flagUpdater.handleFlagChanges(null, Object.keys(clearedOverrides), 'override');
+      this._flagUpdater.handleFlagChanges(Object.keys(clearedOverrides), 'override');
     }
   }
 

packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts

@@ -20,11 +20,6 @@ export type FlagChangeType = 'init' | 'patch' | 'override';
  * will call a variation method for flag values which you require.
  */
 export type FlagsChangeCallback = (
-  // REVIEWER: This is probably not desired, but I think there are some updates
-  // such as overrides that do not really have a context? Unless I am misunderstanding
-  // what context is exactly. Being able to support a null context may also help
-  // with distinguishing between being in the emphemeral state between the start of
-  // initialization and the end of identification and having an invalid context?
   context: Context,
   flagKeys: Array<string>,
   type: FlagChangeType,
@@ -46,14 +41,20 @@ export default class FlagUpdater {
     this._logger = logger;
   }
 
-  handleFlagChanges(context: Context, keys: string[], type: FlagChangeType): void {
-    this._changeCallbacks.forEach((callback) => {
-      try {
-        callback(context, keys, type);
-      } catch (err) {
-        /* intentionally empty */
-      }
-    });
+  handleFlagChanges(keys: string[], type: FlagChangeType): void {
+    if (this._activeContext) {
+      this._changeCallbacks.forEach((callback) => {
+        try {
+          callback(this._activeContext!, keys, type);
+        } catch (err) {
+          /* intentionally empty */
+        }
+      });
+    } else {
+      this._logger.warn(
+        'Received a change event wihtout an active context. Changes will not be propagated.',
+      );
+    }
   }
 
   init(context: Context, newFlags: { [key: string]: ItemDescriptor }) {
@@ -62,7 +63,7 @@ export default class FlagUpdater {
     this._flagStore.init(newFlags);
     const changed = calculateChangedKeys(oldFlags, newFlags);
     if (changed.length > 0) {
-      this.handleFlagChanges(context, changed, 'init');
+      this.handleFlagChanges(changed, 'init');
     }
   }
 
@@ -87,7 +88,7 @@ export default class FlagUpdater {
     }
 
     this._flagStore.insertOrUpdate(key, item);
-    this.handleFlagChanges(this._activeContext!, [key], 'patch');
+    this.handleFlagChanges([key], 'patch');
     return true;
   }