Skip to content

Commit 0f19e7f

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

File tree

2 files changed

+66
-42
lines changed

2 files changed

+66
-42
lines changed

‎src/hooks.spec.tsx

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { render, renderHook, screen, waitFor } from '@testing-library/react';
2222
import '@testing-library/jest-dom';
2323

2424
import { OptimizelyProvider } from './Provider';
25-
import { OnReadyResult, ReactSDKClient, VariableValuesObject } from './client';
25+
import { NotReadyReason, OnReadyResult, ReactSDKClient, VariableValuesObject } from './client';
2626
import { useExperiment, useFeature, useDecision, useTrackEvent, hooksLogger } from './hooks';
2727
import { OptimizelyDecision } from './utils';
2828
const defaultDecision: OptimizelyDecision = {
@@ -62,7 +62,7 @@ const mockFeatureVariables: VariableValuesObject = {
6262
foo: 'bar',
6363
};
6464

65-
describe.skip('hooks', () => {
65+
describe('hooks', () => {
6666
let activateMock: jest.Mock;
6767
let featureVariables: VariableValuesObject;
6868
let getOnReadyPromise: any;
@@ -176,52 +176,79 @@ describe.skip('hooks', () => {
176176
it('should return a variation when activate returns a variation', async () => {
177177
activateMock.mockReturnValue('12345');
178178

179-
const { container } = render(
179+
render(
180180
<OptimizelyProvider optimizely={optimizelyMock}>
181181
<MyExperimentComponent />
182182
</OptimizelyProvider>
183183
);
184-
await optimizelyMock.onReady();
185184
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('12345|true|false'));
186185
});
187186

188187
it('should return null when activate returns null', async () => {
189188
activateMock.mockReturnValue(null);
190189
featureVariables = {};
191-
const { container } = render(
190+
render(
192191
<OptimizelyProvider optimizely={optimizelyMock}>
193192
<MyExperimentComponent />
194193
</OptimizelyProvider>
195194
);
196-
await optimizelyMock.onReady();
197195
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('null|true|false'));
198196
});
199197

200198
it('should respect the timeout option passed', async () => {
201199
activateMock.mockReturnValue(null);
200+
mockDelay = 100;
202201
readySuccess = false;
202+
// 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+
};
203235

204236
render(
205237
<OptimizelyProvider optimizely={optimizelyMock}>
206-
<MyExperimentComponent options={{ timeout: mockDelay }} />
238+
<MyExperimentComponent options={{ timeout: mockDelay - 10 }} />
207239
</OptimizelyProvider>
208240
);
209-
210-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('null|false|false')); // initial render
211-
212-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('null|false|true')); // when didTimeout
213-
214-
// Simulate datafile fetch completing after timeout has already passed
215-
// Activate now returns a variation
241+
expect(screen.getByTestId('result')).toHaveTextContent('null|false|false'); // initial render
242+
readySuccess = true;
216243
activateMock.mockReturnValue('12345');
217-
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('12345|true|true')); // when clientReady
244+
// When timeout is reached, but dataReadyPromise is resolved later with the variation
245+
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('12345|true|true'));
218246
});
219247

220248
it('should gracefully handle the client promise rejecting after timeout', async () => {
221-
jest.useFakeTimers();
222-
223249
readySuccess = false;
224250
activateMock.mockReturnValue('12345');
251+
225252
getOnReadyPromise = (): Promise<void> =>
226253
new Promise((_, rej) => setTimeout(() => rej(REJECTION_REASON), mockDelay));
227254

@@ -231,11 +258,7 @@ describe.skip('hooks', () => {
231258
</OptimizelyProvider>
232259
);
233260

234-
jest.advanceTimersByTime(mockDelay + 1);
235-
236261
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('null|false|false'));
237-
238-
jest.useRealTimers();
239262
});
240263

241264
it('should re-render when the user attributes change using autoUpdate', async () => {
@@ -249,16 +272,14 @@ describe.skip('hooks', () => {
249272

250273
// TODO - Wrap this with async act() once we upgrade to React 16.9
251274
// See https://github.com/facebook/react/issues/15379
252-
await optimizelyMock.onReady();
253275
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('null|true|false'));
254276

255277
activateMock.mockReturnValue('12345');
256278
// Simulate the user object changing
257279
await act(async () => {
258280
userUpdateCallbacks.forEach((fn) => fn());
259281
});
260-
// component.update();
261-
// await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('12345|true|false');
282+
262283
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('12345|true|false'));
263284
});
264285

@@ -272,7 +293,6 @@ describe.skip('hooks', () => {
272293

273294
// // TODO - Wrap this with async act() once we upgrade to React 16.9
274295
// // See https://github.com/facebook/react/issues/15379
275-
await optimizelyMock.onReady();
276296
await waitFor(() => expect(screen.getByTestId('result')).toHaveTextContent('null|true|false'));
277297

278298
activateMock.mockReturnValue('12345');
@@ -425,7 +445,7 @@ describe.skip('hooks', () => {
425445
});
426446
});
427447

428-
describe('useFeature', () => {
448+
describe.skip('useFeature', () => {
429449
it('should render true when the feature is enabled', async () => {
430450
isFeatureEnabledMock.mockReturnValue(true);
431451

@@ -676,7 +696,7 @@ describe.skip('hooks', () => {
676696
});
677697
});
678698

679-
describe('useDecision', () => {
699+
describe.skip('useDecision', () => {
680700
it('should render true when the flag is enabled', async () => {
681701
decideMock.mockReturnValue({
682702
...defaultDecision,

‎src/hooks.ts

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable react-hooks/rules-of-hooks */
12
/**
23
* Copyright 2018-2019, 2022-2024, Optimizely
34
*
@@ -14,7 +15,6 @@
1415
* limitations under the License.
1516
*/
1617

17-
/* eslint-disable react-hooks/rules-of-hooks */
1818
import { useCallback, useContext, useEffect, useState, useRef } from 'react';
1919

2020
import { UserAttributes, OptimizelyDecideOption, getLogger } from '@optimizely/optimizely-sdk';
@@ -259,22 +259,18 @@ function useCompareAttrsMemoize(value: UserAttributes | undefined): UserAttribut
259259
export const useExperiment: UseExperiment = (experimentKey, options = {}, overrides = {}) => {
260260
const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
261261

262-
if (!optimizely) {
263-
hooksLogger.error(
264-
`Unable to use experiment ${experimentKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`
265-
);
266-
return [null, false, false];
267-
}
268-
269262
const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
263+
270264
const getCurrentDecision: () => ExperimentDecisionValues = useCallback(
271265
() => ({
272-
variation: optimizely.activate(experimentKey, overrides.overrideUserId, overrideAttrs),
266+
variation: optimizely?.activate(experimentKey, overrides.overrideUserId, overrideAttrs) || null,
273267
}),
274268
[optimizely, experimentKey, overrides.overrideUserId, overrideAttrs]
275269
);
276270

277-
const isClientReady = isServerSide || optimizely.isReady();
271+
const isClientReady = isServerSide || !!optimizely?.isReady();
272+
const isReadyPromiseFulfilled = !!optimizely?.getIsReadyPromiseFulfilled();
273+
278274
const [state, setState] = useState<ExperimentDecisionValues & InitializationState>(() => {
279275
const decisionState = isClientReady ? getCurrentDecision() : { variation: null };
280276
return {
@@ -293,6 +289,7 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri
293289
overrideUserId: overrides.overrideUserId,
294290
overrideAttributes: overrideAttrs,
295291
};
292+
296293
const [prevDecisionInputs, setPrevDecisionInputs] = useState<DecisionInputs>(currentDecisionInputs);
297294
if (!areDecisionInputsEqual(prevDecisionInputs, currentDecisionInputs)) {
298295
setPrevDecisionInputs(currentDecisionInputs);
@@ -309,19 +306,19 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri
309306
// and we need to wait for the promise and update decision.
310307
// 2. When client is using datafile only but client is not ready yet which means user
311308
// was provided as a promise and we need to subscribe and wait for user to become available.
312-
if ((optimizely.getIsUsingSdkKey() && !optimizely.getIsReadyPromiseFulfilled()) || !isClientReady) {
309+
if (optimizely && ((optimizely.getIsUsingSdkKey() && !isReadyPromiseFulfilled) || !isClientReady)) {
313310
subscribeToInitialization(optimizely, finalReadyTimeout, (initState) => {
314311
setState({
315312
...getCurrentDecision(),
316313
...initState,
317314
});
318315
});
319316
}
320-
}, []);
317+
}, [finalReadyTimeout, getCurrentDecision, isClientReady, isReadyPromiseFulfilled, optimizely]);
321318

322319
useEffect(() => {
323320
// Subscribe to update after first datafile is fetched and readyPromise is resolved to avoid redundant rendering.
324-
if (optimizely.getIsReadyPromiseFulfilled() && options.autoUpdate) {
321+
if (optimizely && isReadyPromiseFulfilled && options.autoUpdate) {
325322
return setupAutoUpdateListeners(optimizely, HookType.EXPERIMENT, experimentKey, hooksLogger, () => {
326323
setState((prevState) => ({
327324
...prevState,
@@ -330,18 +327,25 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri
330327
});
331328
}
332329
return (): void => {};
333-
}, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, experimentKey, getCurrentDecision]);
330+
}, [isReadyPromiseFulfilled, options.autoUpdate, optimizely, experimentKey, getCurrentDecision]);
334331

335332
useEffect(
336333
() =>
337-
optimizely.onForcedVariationsUpdate(() => {
334+
optimizely?.onForcedVariationsUpdate(() => {
338335
setState((prevState) => ({
339336
...prevState,
340337
...getCurrentDecision(),
341338
}));
342339
}),
343340
[getCurrentDecision, optimizely]
344341
);
342+
343+
if (!optimizely) {
344+
hooksLogger.error(
345+
`Unable to use experiment ${experimentKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`
346+
);
347+
}
348+
345349
return [state.variation, state.clientReady, state.didTimeout];
346350
};
347351

0 commit comments

Comments
 (0)