Skip to content

Commit 0d4eb59

Browse files
[FSSDK-9839] fix: rendering default OptimizelyVariation when not last (#226)
* fix: rendering `default` with matching variation OptimizelyExperiment renders `default` when before matching variations * test: add coverage for `default` selection * chore: update copyright & remove `any` * test: revert use of `any` 😩
1 parent bc69201 commit 0d4eb59

File tree

2 files changed

+67
-10
lines changed

2 files changed

+67
-10
lines changed

src/Experiment.spec.tsx

+53-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/**
2-
* Copyright 2018-2019, Optimizely
2+
* Copyright 2018-2019, 2023, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* https://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -170,7 +170,7 @@ describe('<OptimizelyExperiment>', () => {
170170
await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('correct variation'));
171171
});
172172

173-
it('should render using <OptimizelyVariation default>', async () => {
173+
it('should render using <OptimizelyVariation default> in last position', async () => {
174174
const { container } = render(
175175
<OptimizelyProvider optimizely={optimizelyMock}>
176176
<OptimizelyExperiment experiment="experiment1">
@@ -196,6 +196,56 @@ describe('<OptimizelyExperiment>', () => {
196196
await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('default variation'));
197197
});
198198

199+
it('should NOT render using <OptimizelyVariation default> in first position when matching variation', async () => {
200+
const { container } = render(
201+
<OptimizelyProvider optimizely={optimizelyMock}>
202+
<OptimizelyExperiment experiment="experiment1">
203+
<OptimizelyVariation default>
204+
<span data-testid="variation-key">default variation</span>
205+
</OptimizelyVariation>
206+
<OptimizelyVariation variation="variationResult">
207+
<span data-testid="variation-key">matching variation</span>
208+
</OptimizelyVariation>
209+
</OptimizelyExperiment>
210+
</OptimizelyProvider>
211+
);
212+
213+
// while it's waiting for onReady()
214+
expect(container.innerHTML).toBe('');
215+
216+
// Simulate client becoming ready
217+
resolver.resolve({ success: true });
218+
219+
await optimizelyMock.onReady();
220+
221+
await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('matching variation'));
222+
});
223+
224+
it('should render using <OptimizelyVariation default> in first position when NO matching variation', async () => {
225+
const { container } = render(
226+
<OptimizelyProvider optimizely={optimizelyMock}>
227+
<OptimizelyExperiment experiment="experiment1">
228+
<OptimizelyVariation default>
229+
<span data-testid="variation-key">default variation</span>
230+
</OptimizelyVariation>
231+
<OptimizelyVariation variation="otherVariation">
232+
<span data-testid="variation-key">other non-matching variation</span>
233+
</OptimizelyVariation>
234+
</OptimizelyExperiment>
235+
</OptimizelyProvider>
236+
);
237+
238+
// while it's waiting for onReady()
239+
expect(container.innerHTML).toBe('');
240+
241+
// Simulate client becoming ready
242+
resolver.resolve({ success: true });
243+
244+
await optimizelyMock.onReady();
245+
246+
await waitFor(() => expect(screen.getByTestId('variation-key')).toHaveTextContent('default variation'));
247+
});
248+
199249
it('should render an empty string when no default or matching variation is provided', async () => {
200250
const { container } = render(
201251
<OptimizelyProvider optimizely={optimizelyMock}>

src/Experiment.tsx

+14-7
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/**
2-
* Copyright 2018-2019, Optimizely
2+
* Copyright 2018-2019, 2023, Optimizely
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* https://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -55,26 +55,33 @@ const Experiment: React.FunctionComponent<ExperimentProps> = props => {
5555
return <>{(children as ChildrenRenderFunction)(variation, clientReady, didTimeout)}</>;
5656
}
5757

58-
let match: React.ReactElement<VariationProps> | null = null;
58+
let defaultMatch: React.ReactElement<VariationProps> | null = null;
59+
let variationMatch: React.ReactElement<VariationProps> | null = null;
5960

6061
// We use React.Children.forEach instead of React.Children.toArray().find()
6162
// here because toArray adds keys to all child elements and we do not want
6263
// to trigger an unmount/remount
6364
React.Children.forEach(children, (child: React.ReactElement<VariationProps>) => {
64-
if (match || !React.isValidElement(child)) {
65+
if (!React.isValidElement(child)) {
6566
return;
6667
}
6768

6869
if (child.props.variation) {
6970
if (variation === child.props.variation) {
70-
match = child;
71+
variationMatch = child;
7172
}
7273
} else if (child.props.default) {
73-
match = child;
74+
defaultMatch = child;
7475
}
7576
});
7677

77-
return match ? React.cloneElement(match, { variation }) : null;
78+
let match: React.ReactElement<VariationProps> | null = null;
79+
if (variationMatch) {
80+
match = variationMatch;
81+
} else if (defaultMatch) {
82+
match = defaultMatch;
83+
}
84+
return match;
7885
};
7986

8087
export const OptimizelyExperiment = Experiment;

0 commit comments

Comments
 (0)