Skip to content

feat: Always emit change events for flags that are part of an experiment. #115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions src/__tests__/LDClient-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -618,6 +618,73 @@ describe('LDClient', () => {
});
});

it('emits change event, even for flag that has not changed, if that flag is part of an experiment', async () => {
const user = { key: 'user' };
const state0 = {
environment: 'env',
context: user,
flags: { flagkey: { value: 'value0' } },
};
const sp = stubPlatform.mockStateProvider(state0);

await withClient(null, { stateProvider: sp, sendEvents: false }, async client => {
await client.waitForInitialization(5);

expect(client.variation('flagkey')).toEqual('value0');

const state1 = {
flags: { flagkey: { value: 'value1' } },
};

const state2 = {
flags: { flagkey: { value: 'value1', reason: { inExperiment: true } } },
};

const gotChange = eventSink(client, 'change:flagkey');

sp.emit('update', state1);

const args = await gotChange.take();
expect(args).toEqual(['value1', 'value0']);

sp.emit('update', state2);

const args2 = await gotChange.take();
expect(args2).toEqual(['value1', 'value1']);
});
});

it('does not emit a change event if the flag value did not change and the value is not part of an experiment', async () => {
const user = { key: 'user' };
const state0 = {
environment: 'env',
context: user,
flags: { flagkey: { value: 'value0' } },
};
const sp = stubPlatform.mockStateProvider(state0);

await withClient(null, { stateProvider: sp, sendEvents: false }, async client => {
await client.waitForInitialization(5);

expect(client.variation('flagkey')).toEqual('value0');

const state1 = {
flags: { flagkey: { value: 'value1' } },
};

const gotChange = eventSink(client, 'change:flagkey');

sp.emit('update', state1);

const args = await gotChange.take();
expect(args).toEqual(['value1', 'value0']);

sp.emit('update', state1);

expect(gotChange.isEmpty()).toBeTruthy();
});
});

it('disables identify()', async () => {
const user = { key: 'user' };
const user1 = { key: 'user1' };
Expand Down
37 changes: 35 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,37 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
}
}

/**
* Check if flags are part of an experiment.
*
* @param flags Flags to check if they are in an experiment.
* @returns True if any of the flags are in an experiment.
*/
function hasExperimentation(...flags) {
return flags.some(flag => !!flag?.reason?.inExperiment);
}

/**
* Determine if a change notification should be emitted for the flag.
*
* A notification will be sent if the flag value changes, or if either of the evaluation reasons
* indicates the fla was part of an experiment.
*
* When transitioning between contexts, or during a flag update, it is possible that the flag
* value remains the same, but you may be entering or exiting an experiment. Additionally the
* value and the reason may be identical, but we still want to emit an event. This is because
* we want the developer to call variation on that flag, which they may only be doing in
* response to a change. Calling variation will result in an analytic event, which is how
* experiment exposures are tracked.
*
* @param oldFlag The old flag value.
* @param newFlag The new flag value.
* @returns True if a change notification should be emitted.
*/
function shouldNotify(oldFlag, newFlag) {
return !utils.deepEquals(newFlag.value, oldFlag.value) || hasExperimentation(oldFlag, newFlag);
}

// Returns a Promise which will be resolved when we have completely updated the internal flags state,
// dispatched all change events, and updated local storage if appropriate. This Promise is guaranteed
// never to have an unhandled rejection.
Expand All @@ -521,16 +552,18 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) {
return Promise.resolve();
}

for (const key in flags) {
for (const key of Object.keys(flags)) {
if (utils.objectHasOwnProperty(flags, key) && flags[key]) {
if (newFlags[key] && !utils.deepEquals(newFlags[key].value, flags[key].value)) {
if (newFlags[key] && shouldNotify(flags[key], newFlags[key])) {
changes[key] = { previous: flags[key].value, current: getFlagDetail(newFlags[key]) };
} else if (!newFlags[key] || newFlags[key].deleted) {
changes[key] = { previous: flags[key].value };
}
}
}
for (const key in newFlags) {
// Above we handle flags that already existed. Here we handle flags that are in the new flags, but
// were not in the previous flags.
if (utils.objectHasOwnProperty(newFlags, key) && newFlags[key] && (!flags[key] || flags[key].deleted)) {
changes[key] = { current: getFlagDetail(newFlags[key]) };
}
Expand Down
Loading