Skip to content

Commit 064bb1c

Browse files
junaed-optimizelyjunaed-optimizely
committed
[FSSDK-10544] useDecision + useTrackEvent update
1 parent 8fc45b0 commit 064bb1c

File tree

2 files changed

+81
-112
lines changed

2 files changed

+81
-112
lines changed

‎src/hooks.spec.tsx

Lines changed: 44 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -82,54 +82,40 @@ 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-
});
10685

107-
const clientAndUserReadyPromise = new Promise<OnReadyResult>((resolve) => {
108-
setTimeout(() => {
109-
resolve({
110-
success: readySuccess,
111-
});
112-
}, mockDelay);
113-
});
114-
115-
return Promise.race([clientAndUserReadyPromise, timeoutPromise]);
116-
};
11786
beforeEach(() => {
118-
getOnReadyPromise = ({ timeout = 0 }: any): Promise<OnReadyResult> =>
119-
new Promise((resolve) => {
120-
setTimeout(function () {
121-
resolve(
122-
Object.assign(
123-
{
124-
success: readySuccess,
125-
},
126-
!readySuccess && {
127-
dataReadyPromise: new Promise((r) => setTimeout(r, mockDelay)),
128-
}
129-
)
130-
);
131-
}, timeout || mockDelay);
87+
getOnReadyPromise = ({ timeout = 0 }: any): Promise<OnReadyResult> => {
88+
const timeoutPromise = new Promise<OnReadyResult>((resolve) => {
89+
setTimeout(
90+
() => {
91+
resolve({
92+
success: false,
93+
reason: NotReadyReason.TIMEOUT,
94+
dataReadyPromise: new Promise((r) =>
95+
setTimeout(
96+
() =>
97+
r({
98+
success: readySuccess,
99+
}),
100+
mockDelay
101+
)
102+
),
103+
});
104+
},
105+
timeout || mockDelay + 1
106+
);
132107
});
108+
109+
const clientAndUserReadyPromise = new Promise<OnReadyResult>((resolve) => {
110+
setTimeout(() => {
111+
resolve({
112+
success: readySuccess,
113+
});
114+
}, mockDelay);
115+
});
116+
117+
return Promise.race([clientAndUserReadyPromise, timeoutPromise]);
118+
};
133119
activateMock = jest.fn();
134120
isFeatureEnabledMock = jest.fn();
135121
featureVariables = mockFeatureVariables;
@@ -230,13 +216,13 @@ describe('hooks', () => {
230216
activateMock.mockReturnValue(null);
231217
mockDelay = 100;
232218
readySuccess = false;
233-
// Todo: getOnReadyPromise might be moved in the top later
234-
getOnReadyPromise = getOnReadyTimeoutPromise;
219+
235220
render(
236221
<OptimizelyProvider optimizely={optimizelyMock}>
237222
<MyExperimentComponent options={{ timeout: mockDelay - 10 }} />
238223
</OptimizelyProvider>
239224
);
225+
240226
expect(screen.getByTestId('result')).toHaveTextContent('null|false|false'); // initial render
241227
readySuccess = true;
242228
activateMock.mockReturnValue('12345');
@@ -473,7 +459,7 @@ describe('hooks', () => {
473459
isFeatureEnabledMock.mockReturnValue(false);
474460
featureVariables = {};
475461
readySuccess = false;
476-
getOnReadyPromise = getOnReadyTimeoutPromise;
462+
477463
render(
478464
<OptimizelyProvider optimizely={optimizelyMock}>
479465
<MyFeatureComponent options={{ timeout: mockDelay - 10 }} />
@@ -677,7 +663,7 @@ describe('hooks', () => {
677663
});
678664
});
679665

680-
describe.skip('useDecision', () => {
666+
describe('useDecision', () => {
681667
it('should render true when the flag is enabled', async () => {
682668
decideMock.mockReturnValue({
683669
...defaultDecision,
@@ -689,8 +675,6 @@ describe('hooks', () => {
689675
<MyDecideComponent />
690676
</OptimizelyProvider>
691677
);
692-
await optimizelyMock.onReady();
693-
// component.update();
694678
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|false'));
695679
});
696680

@@ -700,50 +684,41 @@ describe('hooks', () => {
700684
enabled: false,
701685
variables: { foo: 'bar' },
702686
});
687+
703688
render(
704689
<OptimizelyProvider optimizely={optimizelyMock}>
705690
<MyDecideComponent />
706691
</OptimizelyProvider>
707692
);
708-
await optimizelyMock.onReady();
693+
709694
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{"foo":"bar"}|true|false'));
710695
});
711696

712697
it('should respect the timeout option passed', async () => {
713698
decideMock.mockReturnValue({ ...defaultDecision });
714699
readySuccess = false;
700+
mockDelay = 100;
715701

716702
render(
717703
<OptimizelyProvider optimizely={optimizelyMock}>
718-
<MyDecideComponent options={{ timeout: mockDelay }} />
704+
<MyDecideComponent options={{ timeout: mockDelay - 10 }} />
719705
</OptimizelyProvider>
720706
);
721-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'));
722707

723-
await optimizelyMock.onReady();
724-
// component.update();
725-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|true'));
708+
// Initial render
709+
expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false');
726710

727-
// Simulate datafile fetch completing after timeout has already passed
728-
// flag is now true and decision contains variables
729711
decideMock.mockReturnValue({
730712
...defaultDecision,
731713
enabled: true,
732714
variables: { foo: 'bar' },
733715
});
734-
735-
await optimizelyMock.onReady().then((res) => res.dataReadyPromise);
736-
737-
// Simulate datafile fetch completing after timeout has already passed
738-
// Wait for completion of dataReadyPromise
739-
await optimizelyMock.onReady().then((res) => res.dataReadyPromise);
740-
741-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|true')); // when clientReady
716+
readySuccess = true;
717+
// When timeout is reached, but dataReadyPromise is resolved later with the decision value
718+
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('true|{"foo":"bar"}|true|true'));
742719
});
743720

744721
it('should gracefully handle the client promise rejecting after timeout', async () => {
745-
jest.useFakeTimers();
746-
747722
readySuccess = false;
748723
decideMock.mockReturnValue({ ...defaultDecision });
749724
getOnReadyPromise = (): Promise<void> =>
@@ -755,11 +730,7 @@ describe('hooks', () => {
755730
</OptimizelyProvider>
756731
);
757732

758-
jest.advanceTimersByTime(mockDelay + 1);
759-
760733
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|false|false'));
761-
762-
jest.useRealTimers();
763734
});
764735

765736
it('should re-render when the user attributes change using autoUpdate', async () => {
@@ -772,7 +743,6 @@ describe('hooks', () => {
772743

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

778748
decideMock.mockReturnValue({
@@ -797,7 +767,6 @@ describe('hooks', () => {
797767

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

803772
decideMock.mockReturnValue({
@@ -991,7 +960,6 @@ describe('hooks', () => {
991960
);
992961

993962
decideMock.mockReturnValue({ ...defaultDecision, variables: { foo: 'bar' } });
994-
await optimizelyMock.onReady();
995963
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{"foo":"bar"}|true|false'));
996964
});
997965

@@ -1017,7 +985,6 @@ describe('hooks', () => {
1017985
{ variationKey: 'var2' }
1018986
);
1019987

1020-
await optimizelyMock.onReady();
1021988
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'));
1022989
});
1023990

@@ -1045,7 +1012,6 @@ describe('hooks', () => {
10451012
{ variationKey: 'var2' }
10461013
);
10471014

1048-
await optimizelyMock.onReady();
10491015
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('false|{}|true|false'));
10501016
});
10511017
});

‎src/hooks.ts

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
/* eslint-disable react-hooks/rules-of-hooks */
21
/**
32
* Copyright 2018-2019, 2022-2024, Optimizely
43
*
@@ -14,7 +13,8 @@
1413
* See the License for the specific language governing permissions and
1514
* limitations under the License.
1615
*/
17-
import { useCallback, useContext, useEffect, useState, useRef } from 'react';
16+
17+
import { useCallback, useContext, useEffect, useState, useRef, useMemo } from 'react';
1818

1919
import { UserAttributes, OptimizelyDecideOption, getLogger } from '@optimizely/optimizely-sdk';
2020

@@ -449,35 +449,33 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {})
449449
export const useDecision: UseDecision = (flagKey, options = {}, overrides = {}) => {
450450
const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
451451

452-
if (!optimizely) {
453-
hooksLogger.error(
454-
`Unable to use decision ${flagKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`
455-
);
456-
return [
452+
const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
453+
454+
const defaultDecision = useMemo(
455+
() =>
457456
createFailedDecision(flagKey, 'Optimizely SDK not configured properly yet.', {
458-
id: null,
459-
attributes: {},
457+
id: overrides.overrideUserId || null,
458+
attributes: overrideAttrs || {},
460459
}),
461-
false,
462-
false,
463-
];
464-
}
460+
[flagKey, overrideAttrs, overrides.overrideUserId]
461+
);
465462

466-
const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
463+
const getCurrentDecision: () => { decision: OptimizelyDecision } = useCallback(
464+
() => ({
465+
decision:
466+
optimizely?.decide(flagKey, options.decideOptions, overrides.overrideUserId, overrideAttrs) || defaultDecision,
467+
}),
468+
[flagKey, defaultDecision, optimizely, options.decideOptions, overrideAttrs, overrides.overrideUserId]
469+
);
467470

468-
const getCurrentDecision: () => { decision: OptimizelyDecision } = () => ({
469-
decision: optimizely.decide(flagKey, options.decideOptions, overrides.overrideUserId, overrideAttrs),
470-
});
471+
const isClientReady = isServerSide || !!optimizely?.isReady();
472+
const isReadyPromiseFulfilled = !!optimizely?.getIsReadyPromiseFulfilled();
471473

472-
const isClientReady = isServerSide || optimizely.isReady();
473474
const [state, setState] = useState<{ decision: OptimizelyDecision } & InitializationState>(() => {
474475
const decisionState = isClientReady
475476
? getCurrentDecision()
476477
: {
477-
decision: createFailedDecision(flagKey, 'Optimizely SDK not configured properly yet.', {
478-
id: overrides.overrideUserId || null,
479-
attributes: overrideAttrs,
480-
}),
478+
decision: defaultDecision,
481479
};
482480
return {
483481
...decisionState,
@@ -504,21 +502,23 @@ export const useDecision: UseDecision = (flagKey, options = {}, overrides = {})
504502
}
505503

506504
const finalReadyTimeout = options.timeout !== undefined ? options.timeout : timeout;
505+
507506
useEffect(() => {
508507
// Subscribe to initialzation promise only
509508
// 1. When client is using Sdk Key, which means the initialization will be asynchronous
510509
// and we need to wait for the promise and update decision.
511510
// 2. When client is using datafile only but client is not ready yet which means user
512511
// was provided as a promise and we need to subscribe and wait for user to become available.
513-
if (optimizely.getIsUsingSdkKey() || !isClientReady) {
512+
if (optimizely && (optimizely.getIsUsingSdkKey() || !isClientReady)) {
514513
subscribeToInitialization(optimizely, finalReadyTimeout, (initState) => {
515514
setState({
516515
...getCurrentDecision(),
517516
...initState,
518517
});
519518
});
520519
}
521-
}, []);
520+
// eslint-disable-next-line react-hooks/exhaustive-deps
521+
}, [finalReadyTimeout, getCurrentDecision, optimizely]);
522522

523523
useEffect(() => {
524524
if (overrides.overrideUserId || overrides.overrideAttributes || !options.autoUpdate) {
@@ -532,11 +532,11 @@ export const useDecision: UseDecision = (flagKey, options = {}, overrides = {})
532532
...getCurrentDecision(),
533533
}));
534534
});
535-
}, [overrides.overrideUserId, overrides.overrideAttributes, options.autoUpdate]);
535+
}, [overrides.overrideUserId, overrides.overrideAttributes, options.autoUpdate, flagKey, getCurrentDecision]);
536536

537537
useEffect(() => {
538538
// Subscribe to update after first datafile is fetched and readyPromise is resolved to avoid redundant rendering.
539-
if (optimizely.getIsReadyPromiseFulfilled() && options.autoUpdate) {
539+
if (optimizely && isReadyPromiseFulfilled && options.autoUpdate) {
540540
return setupAutoUpdateListeners(optimizely, HookType.FEATURE, flagKey, hooksLogger, () => {
541541
setState((prevState) => ({
542542
...prevState,
@@ -545,14 +545,20 @@ export const useDecision: UseDecision = (flagKey, options = {}, overrides = {})
545545
});
546546
}
547547
return (): void => {};
548-
}, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, flagKey, getCurrentDecision]);
548+
}, [isReadyPromiseFulfilled, options.autoUpdate, optimizely, flagKey, getCurrentDecision]);
549+
550+
if (!optimizely) {
551+
hooksLogger.error(
552+
`Unable to use decision ${flagKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`
553+
);
554+
}
549555

550556
return [state.decision, state.clientReady, state.didTimeout];
551557
};
552558

553559
export const useTrackEvent: UseTrackEvent = () => {
554560
const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
555-
const isClientReady = !!(isServerSide || optimizely?.isReady());
561+
const isClientReady = isServerSide || !!optimizely?.isReady();
556562

557563
const track = useCallback(
558564
(...rest: Parameters<ReactSDKClient['track']>): void => {
@@ -569,10 +575,6 @@ export const useTrackEvent: UseTrackEvent = () => {
569575
[optimizely, isClientReady]
570576
);
571577

572-
if (!optimizely) {
573-
return [track, false, false];
574-
}
575-
576578
const [state, setState] = useState<{
577579
clientReady: boolean;
578580
didTimeout: DidTimeout;
@@ -589,12 +591,13 @@ export const useTrackEvent: UseTrackEvent = () => {
589591
// and we need to wait for the promise and update decision.
590592
// 2. When client is using datafile only but client is not ready yet which means user
591593
// was provided as a promise and we need to subscribe and wait for user to become available.
592-
if (optimizely.getIsUsingSdkKey() || !isClientReady) {
594+
if (optimizely && (optimizely.getIsUsingSdkKey() || !isClientReady)) {
593595
subscribeToInitialization(optimizely, timeout, (initState) => {
594596
setState(initState);
595597
});
596598
}
597-
}, []);
599+
// eslint-disable-next-line react-hooks/exhaustive-deps
600+
}, [optimizely, timeout]);
598601

599602
return [track, state.clientReady, state.didTimeout];
600603
};

0 commit comments

Comments
 (0)