Skip to content

Commit 6bb0463

Browse files
gastonfourniergastonfournier
authored
fix: backward compatibility issues in 5.1.0 and 5.2.0 (#540)
1 parent f985e51 commit 6bb0463

File tree

9 files changed

+145
-81
lines changed

9 files changed

+145
-81
lines changed

‎src/client.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,13 @@ import { EventEmitter } from 'events';
22
import { Strategy, StrategyTransportInterface } from './strategy';
33
import { FeatureInterface } from './feature';
44
import { RepositoryInterface } from './repository';
5-
import { Variant, VariantDefinition, defaultVariant, selectVariant } from './variant';
5+
import {
6+
Variant,
7+
VariantDefinition,
8+
VariantWithFeatureStatus,
9+
defaultVariant,
10+
selectVariant,
11+
} from './variant';
612
import { Context } from './context';
713
import { Constraint, Segment, StrategyResult } from './strategy/strategy';
814
import { createImpressionEvent, UnleashEvents } from './events';
@@ -197,7 +203,7 @@ export default class UnleashClient extends EventEmitter {
197203
}
198204
}
199205

200-
getVariant(name: string, context: Context, fallbackVariant?: Variant): Variant {
206+
getVariant(name: string, context: Context, fallbackVariant?: Variant): VariantWithFeatureStatus {
201207
const feature = this.repository.getToggle(name);
202208
const variant = this.resolveVariant(feature, context, true, fallbackVariant);
203209
if (feature?.impressionData) {
@@ -218,7 +224,11 @@ export default class UnleashClient extends EventEmitter {
218224
// This function is intended to close an issue in the proxy where feature enabled
219225
// state gets checked twice when resolving a variant with random stickiness and
220226
// gradual rollout. This is not intended for general use, prefer getVariant instead
221-
forceGetVariant(name: string, context: Context, fallbackVariant?: Variant): Variant {
227+
forceGetVariant(
228+
name: string,
229+
context: Context,
230+
fallbackVariant?: Variant,
231+
): VariantWithFeatureStatus {
222232
const feature = this.repository.getToggle(name);
223233
return this.resolveVariant(feature, context, true, fallbackVariant);
224234
}
@@ -228,11 +238,11 @@ export default class UnleashClient extends EventEmitter {
228238
context: Context,
229239
checkToggle: boolean,
230240
fallbackVariant?: Variant,
231-
): Variant {
241+
): VariantWithFeatureStatus {
232242
const fallback = fallbackVariant || defaultVariant;
233243

234244
if (typeof feature === 'undefined') {
235-
return { ...fallback, feature_enabled: false };
245+
return { ...fallback, feature_enabled: false, featureEnabled: false };
236246
}
237247

238248
let featureEnabled = !checkToggle;
@@ -241,28 +251,29 @@ export default class UnleashClient extends EventEmitter {
241251
featureEnabled = result.enabled;
242252

243253
if (result.enabled && result.variant) {
244-
return { ...result.variant, feature_enabled: featureEnabled };
254+
return { ...result.variant, feature_enabled: featureEnabled, featureEnabled };
245255
}
246256

247257
if (!result.enabled) {
248-
return { ...fallback, feature_enabled: featureEnabled };
258+
return { ...fallback, feature_enabled: featureEnabled, featureEnabled };
249259
}
250260
}
251261

252262
if (!feature.variants || !Array.isArray(feature.variants) || feature.variants.length === 0) {
253-
return { ...fallback, feature_enabled: featureEnabled };
263+
return { ...fallback, feature_enabled: featureEnabled, featureEnabled };
254264
}
255265

256266
const variant: VariantDefinition | null = selectVariant(feature, context);
257267
if (variant === null) {
258-
return { ...fallback, feature_enabled: featureEnabled };
268+
return { ...fallback, feature_enabled: featureEnabled, featureEnabled };
259269
}
260270

261271
return {
262272
name: variant.name,
263273
payload: variant.payload,
264274
enabled: true,
265275
feature_enabled: featureEnabled,
276+
featureEnabled,
266277
};
267278
}
268279
}

‎src/test/client.test.ts

Lines changed: 85 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,16 @@ test('invalid strategy should throw', (t) => {
3232
t.throws(() => new Client(repo, [{}]));
3333
t.throws(() => new Client(repo, [{ name: 'invalid' }]));
3434
t.throws(() => new Client(repo, [{ isEnabled: 'invalid' }]));
35-
t.throws(() => new Client(repo, [{
36-
name: 'valid', isEnabled: () => {
37-
},
38-
}, null]));
35+
t.throws(
36+
() =>
37+
new Client(repo, [
38+
{
39+
name: 'valid',
40+
isEnabled: () => {},
41+
},
42+
null,
43+
]),
44+
);
3945
});
4046

4147
test('should use provided repository', (t) => {
@@ -111,10 +117,7 @@ test('should use a set of custom strategies', (t) => {
111117
test('should return false a set of custom-false strategies', (t) => {
112118
const repo = {
113119
getToggle() {
114-
return buildToggle('feature', true, [
115-
{ name: 'custom-false' },
116-
{ name: 'custom-false' },
117-
]);
120+
return buildToggle('feature', true, [{ name: 'custom-false' }, { name: 'custom-false' }]);
118121
},
119122
};
120123

@@ -248,7 +251,8 @@ test('should always return defaultVariant if missing variant', (t) => {
248251
const defaultVariant = {
249252
enabled: false,
250253
name: 'disabled',
251-
feature_enabled: true
254+
feature_enabled: true,
255+
featureEnabled: true,
252256
};
253257
t.deepEqual(result, defaultVariant);
254258

@@ -259,7 +263,8 @@ test('should always return defaultVariant if missing variant', (t) => {
259263
type: 'string',
260264
value: '',
261265
},
262-
feature_enabled: true
266+
feature_enabled: true,
267+
featureEnabled: true,
263268
};
264269
const result2 = client.getVariant('feature-but-no-variant', {}, fallback);
265270

@@ -299,33 +304,34 @@ test('should trigger events on isEnabled if impressionData is true', (t) => {
299304
});
300305
client.isEnabled('feature-x', {}, () => false);
301306
t.true(called);
302-
303307
});
304308

305309
test('should trigger events on unsatisfied dependency', (t) => {
306310
let impressionCount = 0;
307311
let recordedWarnings = [];
308312
const repo = {
309313
getToggle(name: string) {
310-
if(name === 'feature-x') {
314+
if (name === 'feature-x') {
311315
return {
312316
name: 'feature-x',
313-
dependencies: [{feature: 'not-feature-x'}],
314-
strategies: [{ name: 'default' }],
317+
dependencies: [{ feature: 'not-feature-x' }],
318+
strategies: [{ name: 'default' }],
315319
variants: [],
316320
impressionData: true,
317-
}
321+
};
318322
} else {
319323
return undefined;
320324
}
321325
},
322326
};
323327
const client = new Client(repo, []);
324-
client.on(UnleashEvents.Impression, () => {
325-
impressionCount++;
326-
}).on(UnleashEvents.Warn, (warning) => {
327-
recordedWarnings.push(warning);
328-
});
328+
client
329+
.on(UnleashEvents.Impression, () => {
330+
impressionCount++;
331+
})
332+
.on(UnleashEvents.Warn, (warning) => {
333+
recordedWarnings.push(warning);
334+
});
329335
client.isEnabled('feature-x', {}, () => false);
330336
client.isEnabled('feature-x', {}, () => false);
331337
t.deepEqual(impressionCount, 2);
@@ -350,70 +356,83 @@ test('should trigger events on getVariant if impressionData is true', (t) => {
350356
test('should favor strategy variant over feature variant', (t) => {
351357
const repo = {
352358
getToggle() {
353-
return buildToggle('feature-x', true, [
354-
{
355-
name: 'default',
356-
constraints: [],
357-
variants: [{
358-
name: 'strategyVariantName',
359-
payload: { type: 'string', value: 'strategyVariantValue' },
360-
weight: 1000
361-
}],
362-
parameters: {},
363-
},
364-
], [
365-
{
366-
name: 'willBeIgnored',
367-
weight: 100,
368-
payload: {
369-
type: 'string',
370-
value: 'willBeIgnored',
359+
return buildToggle(
360+
'feature-x',
361+
true,
362+
[
363+
{
364+
name: 'default',
365+
constraints: [],
366+
variants: [
367+
{
368+
name: 'strategyVariantName',
369+
payload: { type: 'string', value: 'strategyVariantValue' },
370+
weight: 1000,
371+
},
372+
],
373+
parameters: {},
371374
},
372-
},
373-
], true);
375+
],
376+
[
377+
{
378+
name: 'willBeIgnored',
379+
weight: 100,
380+
payload: {
381+
type: 'string',
382+
value: 'willBeIgnored',
383+
},
384+
},
385+
],
386+
true,
387+
);
374388
},
375389
};
376390
const client = new Client(repo, defaultStrategies);
377391
const enabled = client.isEnabled('feature-x', {}, () => false);
378392
const variant = client.getVariant('feature-x', {});
379393
t.true(enabled);
380394
t.deepEqual(variant, {
381-
name: 'strategyVariantName',
382-
payload: { type: 'string', value: 'strategyVariantValue' },
383-
enabled: true,
384-
feature_enabled: true
385-
},
386-
);
395+
name: 'strategyVariantName',
396+
payload: { type: 'string', value: 'strategyVariantValue' },
397+
enabled: true,
398+
feature_enabled: true,
399+
featureEnabled: true,
400+
});
387401
});
388402

389-
390403
test('should return disabled variant for non-matching strategy variant', (t) => {
391404
const repo = {
392405
getToggle() {
393-
return buildToggle('feature-x', false, [
394-
{
395-
name: 'default',
396-
constraints: [],
397-
variants: [{
398-
name: 'strategyVariantName',
399-
payload: { type: 'string', value: 'strategyVariantValue' },
400-
weight: 1000
401-
}],
402-
parameters: {},
403-
},
404-
], [], true);
406+
return buildToggle(
407+
'feature-x',
408+
false,
409+
[
410+
{
411+
name: 'default',
412+
constraints: [],
413+
variants: [
414+
{
415+
name: 'strategyVariantName',
416+
payload: { type: 'string', value: 'strategyVariantValue' },
417+
weight: 1000,
418+
},
419+
],
420+
parameters: {},
421+
},
422+
],
423+
[],
424+
true,
425+
);
405426
},
406427
};
407428
const client = new Client(repo, defaultStrategies);
408429
const enabled = client.isEnabled('feature-x', {}, () => false);
409430
const variant = client.getVariant('feature-x', {});
410431
t.false(enabled);
411432
t.deepEqual(variant, {
412-
name: 'disabled',
413-
enabled: false,
414-
feature_enabled: false,
415-
},
416-
);
433+
name: 'disabled',
434+
enabled: false,
435+
feature_enabled: false,
436+
featureEnabled: false,
437+
});
417438
});
418-
419-

‎src/test/integration/client-specification.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ specs.forEach((testName) => {
7575
instance.on('error', reject);
7676
instance.on('synchronized', () => {
7777
const result = instance.getVariant(testCase.toggleName, testCase.context);
78-
t.deepEqual(result, testCase.expectedResult);
78+
t.deepEqual(result, {
79+
...testCase.expectedResult,
80+
featureEnabled: testCase.expectedResult.feature_enabled
81+
});
7982

8083
instance.destroy();
8184
resolve();

‎src/test/snapshots/client.test.ts.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Generated by [AVA](https://avajs.dev).
1010
1111
{
1212
enabled: true,
13+
featureEnabled: true,
1314
feature_enabled: true,
1415
name: 'variant3',
1516
payload: {
@@ -22,6 +23,7 @@ Generated by [AVA](https://avajs.dev).
2223
2324
{
2425
enabled: true,
26+
featureEnabled: true,
2527
feature_enabled: true,
2628
name: 'variant3',
2729
payload: {
@@ -34,6 +36,7 @@ Generated by [AVA](https://avajs.dev).
3436
3537
{
3638
enabled: true,
39+
featureEnabled: true,
3740
feature_enabled: true,
3841
name: 'variant3',
3942
payload: {
@@ -48,6 +51,7 @@ Generated by [AVA](https://avajs.dev).
4851
4952
{
5053
enabled: true,
54+
featureEnabled: true,
5155
feature_enabled: true,
5256
name: 'variant3',
5357
payload: {
@@ -60,6 +64,7 @@ Generated by [AVA](https://avajs.dev).
6064
6165
{
6266
enabled: true,
67+
featureEnabled: true,
6368
feature_enabled: true,
6469
name: 'variant1',
6570
payload: {
@@ -72,6 +77,7 @@ Generated by [AVA](https://avajs.dev).
7277
7378
{
7479
enabled: true,
80+
featureEnabled: true,
7581
feature_enabled: true,
7682
name: 'variant3',
7783
payload: {
@@ -86,6 +92,7 @@ Generated by [AVA](https://avajs.dev).
8692
8793
{
8894
enabled: true,
95+
featureEnabled: true,
8996
feature_enabled: true,
9097
name: 'variant3',
9198
payload: {
@@ -98,6 +105,7 @@ Generated by [AVA](https://avajs.dev).
98105
99106
{
100107
enabled: true,
108+
featureEnabled: true,
101109
feature_enabled: true,
102110
name: 'variant3',
103111
payload: {
@@ -110,6 +118,7 @@ Generated by [AVA](https://avajs.dev).
110118
111119
{
112120
enabled: true,
121+
featureEnabled: true,
113122
feature_enabled: true,
114123
name: 'variant2',
115124
payload: {

‎src/test/snapshots/client.test.ts.snap

9 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)