Skip to content

Commit 8fc45b0

Browse files
junaed-optimizelyjunaed-optimizely
committed
[FSSDK-10544] useFeature code + test refactor
1 parent 0f19e7f commit 8fc45b0

File tree

2 files changed

+61
-76
lines changed

2 files changed

+61
-76
lines changed

‎src/hooks.spec.tsx

Lines changed: 41 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,38 @@ describe('hooks', () => {
8282
let setForcedDecisionMock: jest.Mock<void>;
8383
let hooksLoggerErrorSpy: jest.SpyInstance;
8484
const REJECTION_REASON = 'A rejection reason you should never see in the test runner';
85+
const getOnReadyTimeoutPromise = ({ timeout = 0 }: any): Promise<OnReadyResult> => {
86+
const timeoutPromise = new Promise<OnReadyResult>((resolve) => {
87+
setTimeout(
88+
() => {
89+
resolve({
90+
success: false,
91+
reason: NotReadyReason.TIMEOUT,
92+
dataReadyPromise: new Promise((r) =>
93+
setTimeout(
94+
() =>
95+
r({
96+
success: readySuccess,
97+
}),
98+
mockDelay
99+
)
100+
),
101+
});
102+
},
103+
timeout || mockDelay + 1
104+
);
105+
});
106+
107+
const clientAndUserReadyPromise = new Promise<OnReadyResult>((resolve) => {
108+
setTimeout(() => {
109+
resolve({
110+
success: readySuccess,
111+
});
112+
}, mockDelay);
113+
});
85114

115+
return Promise.race([clientAndUserReadyPromise, timeoutPromise]);
116+
};
86117
beforeEach(() => {
87118
getOnReadyPromise = ({ timeout = 0 }: any): Promise<OnReadyResult> =>
88119
new Promise((resolve) => {
@@ -200,39 +231,7 @@ describe('hooks', () => {
200231
mockDelay = 100;
201232
readySuccess = false;
202233
// Todo: getOnReadyPromise might be moved in the top later
203-
getOnReadyPromise = ({ timeout = 0 }: any): Promise<OnReadyResult> => {
204-
const timeoutPromise = new Promise<OnReadyResult>((resolve) => {
205-
setTimeout(
206-
() => {
207-
resolve({
208-
success: false,
209-
reason: NotReadyReason.TIMEOUT,
210-
dataReadyPromise: new Promise((r) =>
211-
setTimeout(
212-
() =>
213-
r({
214-
success: readySuccess,
215-
}),
216-
mockDelay
217-
)
218-
),
219-
});
220-
},
221-
timeout || mockDelay + 1
222-
);
223-
});
224-
225-
const clientAndUserReadyPromise = new Promise<OnReadyResult>((resolve) => {
226-
setTimeout(() => {
227-
resolve({
228-
success: readySuccess,
229-
});
230-
}, mockDelay);
231-
});
232-
233-
return Promise.race([clientAndUserReadyPromise, timeoutPromise]);
234-
};
235-
234+
getOnReadyPromise = getOnReadyTimeoutPromise;
236235
render(
237236
<OptimizelyProvider optimizely={optimizelyMock}>
238237
<MyExperimentComponent options={{ timeout: mockDelay - 10 }} />
@@ -445,7 +444,7 @@ describe('hooks', () => {
445444
});
446445
});
447446

448-
describe.skip('useFeature', () => {
447+
describe('useFeature', () => {
449448
it('should render true when the feature is enabled', async () => {
450449
isFeatureEnabledMock.mockReturnValue(true);
451450

@@ -454,7 +453,6 @@ describe('hooks', () => {
454453
<MyFeatureComponent />
455454
</OptimizelyProvider>
456455
);
457-
await optimizelyMock.onReady();
458456
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|false'));
459457
});
460458

@@ -467,44 +465,33 @@ describe('hooks', () => {
467465
<MyFeatureComponent />
468466
</OptimizelyProvider>
469467
);
470-
await optimizelyMock.onReady();
471468
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'));
472469
});
473470

474471
it('should respect the timeout option passed', async () => {
472+
mockDelay = 100;
475473
isFeatureEnabledMock.mockReturnValue(false);
476474
featureVariables = {};
477475
readySuccess = false;
478-
476+
getOnReadyPromise = getOnReadyTimeoutPromise;
479477
render(
480478
<OptimizelyProvider optimizely={optimizelyMock}>
481-
<MyFeatureComponent options={{ timeout: mockDelay }} />
479+
<MyFeatureComponent options={{ timeout: mockDelay - 10 }} />
482480
</OptimizelyProvider>
483481
);
484482

485-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false')); // initial render
486-
487-
await optimizelyMock.onReady();
488-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|true')); // when didTimeout
483+
// Initial render
484+
expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false');
489485

490-
// Simulate datafile fetch completing after timeout has already passed
491-
// isFeatureEnabled now returns true, getFeatureVariables returns variable values
486+
readySuccess = true;
492487
isFeatureEnabledMock.mockReturnValue(true);
493488
featureVariables = mockFeatureVariables;
494-
// Wait for completion of dataReadyPromise
495-
await optimizelyMock.onReady().then((res) => res.dataReadyPromise);
496489

497-
// Simulate datafile fetch completing after timeout has already passed
498-
// Activate now returns a variation
499-
activateMock.mockReturnValue('12345');
500-
// Wait for completion of dataReadyPromise
501-
await optimizelyMock.onReady().then((res) => res.dataReadyPromise);
502-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|true')); // when clientReady
490+
// When timeout is reached, but dataReadyPromise is resolved later with the feature enabled
491+
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|true'));
503492
});
504493

505494
it('should gracefully handle the client promise rejecting after timeout', async () => {
506-
jest.useFakeTimers();
507-
508495
readySuccess = false;
509496
isFeatureEnabledMock.mockReturnValue(true);
510497
getOnReadyPromise = (): Promise<void> =>
@@ -516,11 +503,7 @@ describe('hooks', () => {
516503
</OptimizelyProvider>
517504
);
518505

519-
jest.advanceTimersByTime(mockDelay + 1);
520-
521506
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'));
522-
523-
jest.useRealTimers();
524507
});
525508

526509
it('should re-render when the user attributes change using autoUpdate', async () => {
@@ -535,7 +518,6 @@ describe('hooks', () => {
535518

536519
// TODO - Wrap this with async act() once we upgrade to React 16.9
537520
// See https://github.com/facebook/react/issues/15379
538-
await optimizelyMock.onReady();
539521
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'));
540522

541523
isFeatureEnabledMock.mockReturnValue(true);
@@ -559,7 +541,6 @@ describe('hooks', () => {
559541

560542
// TODO - Wrap this with async act() once we upgrade to React 16.9
561543
// See https://github.com/facebook/react/issues/15379
562-
await optimizelyMock.onReady();
563544
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'));
564545

565546
isFeatureEnabledMock.mockReturnValue(true);

‎src/hooks.ts

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
1817
import { useCallback, useContext, useEffect, useState, useRef } from 'react';
1918

2019
import { UserAttributes, OptimizelyDecideOption, getLogger } from '@optimizely/optimizely-sdk';
@@ -358,24 +357,19 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri
358357
*/
359358
export const useFeature: UseFeature = (featureKey, options = {}, overrides = {}) => {
360359
const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
361-
362-
if (!optimizely) {
363-
hooksLogger.error(
364-
`Unable to properly use feature ${featureKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`
365-
);
366-
return [false, {}, false, false];
367-
}
368-
369360
const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
361+
370362
const getCurrentDecision: () => FeatureDecisionValues = useCallback(
371363
() => ({
372-
isEnabled: optimizely.isFeatureEnabled(featureKey, overrides.overrideUserId, overrideAttrs),
373-
variables: optimizely.getFeatureVariables(featureKey, overrides.overrideUserId, overrideAttrs),
364+
isEnabled: !!optimizely?.isFeatureEnabled(featureKey, overrides.overrideUserId, overrideAttrs),
365+
variables: optimizely?.getFeatureVariables(featureKey, overrides.overrideUserId, overrideAttrs) || {},
374366
}),
375367
[optimizely, featureKey, overrides.overrideUserId, overrideAttrs]
376368
);
377369

378-
const isClientReady = isServerSide || optimizely.isReady();
370+
const isClientReady = isServerSide || !!optimizely?.isReady();
371+
const isReadyPromiseFulfilled = !!optimizely?.getIsReadyPromiseFulfilled();
372+
379373
const [state, setState] = useState<FeatureDecisionValues & InitializationState>(() => {
380374
const decisionState = isClientReady ? getCurrentDecision() : { isEnabled: false, variables: {} };
381375
return {
@@ -393,7 +387,9 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {})
393387
overrideUserId: overrides.overrideUserId,
394388
overrideAttributes: overrides.overrideAttributes,
395389
};
390+
396391
const [prevDecisionInputs, setPrevDecisionInputs] = useState<DecisionInputs>(currentDecisionInputs);
392+
397393
if (!areDecisionInputsEqual(prevDecisionInputs, currentDecisionInputs)) {
398394
setPrevDecisionInputs(currentDecisionInputs);
399395
setState((prevState) => ({
@@ -403,25 +399,27 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {})
403399
}
404400

405401
const finalReadyTimeout = options.timeout !== undefined ? options.timeout : timeout;
402+
406403
useEffect(() => {
407404
// Subscribe to initialzation promise only
408405
// 1. When client is using Sdk Key, which means the initialization will be asynchronous
409406
// and we need to wait for the promise and update decision.
410407
// 2. When client is using datafile only but client is not ready yet which means user
411408
// was provided as a promise and we need to subscribe and wait for user to become available.
412-
if (optimizely.getIsUsingSdkKey() || !isClientReady) {
409+
if (optimizely && (optimizely.getIsUsingSdkKey() || !isClientReady)) {
413410
subscribeToInitialization(optimizely, finalReadyTimeout, (initState) => {
414411
setState({
415412
...getCurrentDecision(),
416413
...initState,
417414
});
418415
});
419416
}
420-
}, []);
417+
// eslint-disable-next-line react-hooks/exhaustive-deps
418+
}, [finalReadyTimeout, getCurrentDecision, optimizely]);
421419

422420
useEffect(() => {
423421
// Subscribe to update after first datafile is fetched and readyPromise is resolved to avoid redundant rendering.
424-
if (optimizely.getIsReadyPromiseFulfilled() && options.autoUpdate) {
422+
if (optimizely && isReadyPromiseFulfilled && options.autoUpdate) {
425423
return setupAutoUpdateListeners(optimizely, HookType.FEATURE, featureKey, hooksLogger, () => {
426424
setState((prevState) => ({
427425
...prevState,
@@ -430,7 +428,13 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {})
430428
});
431429
}
432430
return (): void => {};
433-
}, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, featureKey, getCurrentDecision]);
431+
}, [isReadyPromiseFulfilled, options.autoUpdate, optimizely, featureKey, getCurrentDecision]);
432+
433+
if (!optimizely) {
434+
hooksLogger.error(
435+
`Unable to properly use feature ${featureKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`
436+
);
437+
}
434438

435439
return [state.isEnabled, state.variables, state.clientReady, state.didTimeout];
436440
};

0 commit comments

Comments
 (0)