Skip to content

Commit 14a80f2

Browse files
author
James Fox
authored
enhancement (<OptimizelyFeature>): Convert to functional component that uses useFeature hook (#32)
Summary Updates <OptimizelyFeature> to be a functional component that Also adds support for userId and userAttribute overrides. Addresses #30 Release Is this a breaking change? In theory the logic should behave the same, as #28 was designed to implement the same logic as <OptimizelyFeature>. Timing wise, when isServerSide is false, the state setting of the correct values of the feature/variables in useFeature happens in a second chained thennable, rather than the first. I dont think this should be considered a breaking change (and thus warrant a major release), but I wanted to point it out.
1 parent b7ae79f commit 14a80f2

File tree

4 files changed

+96
-155
lines changed

4 files changed

+96
-155
lines changed

‎CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
77

88
## [Unreleased]
99

10+
- Refactored `<OptimizelyFeature>` to a functional component that uses the `useFeature` hook under the hood. See [#32](https://github.com/optimizely/react-sdk/pull/32) for more details.
11+
1012
### New Features
1113

1214
- Added `useFeature` hook
1315
- Can be used to retrieve the status of a feature flag and its variables. See [#28](https://github.com/optimizely/react-sdk/pull/28) for more details.
1416

1517
### Enhancements
1618

17-
- Exposed the entire context object used by
19+
- Exposed the entire context object used by `<OptimizelyProvider>`.
1820
- Enables support for using APIs which require passing reference to a context object, like `useContext`. [#27](https://github.com/optimizely/react-sdk/pull/27) for more details.
1921

20-
2122
## [1.2.0-alpha.1] - March 5th, 2020
2223

2324
### New Features

‎src/Feature.spec.tsx

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ describe('<OptimizelyFeature>', () => {
8181

8282
component.update();
8383

84-
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
85-
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
84+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
85+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
8686
expect(component.text()).toBe('true|bar');
8787
});
8888

@@ -99,17 +99,41 @@ describe('<OptimizelyFeature>', () => {
9999

100100
// while it's waiting for onReady()
101101
expect(component.text()).toBe('');
102-
resolver.resolve({ sucess: true });
102+
resolver.resolve({ success: true });
103103

104104
await optimizelyMock.onReady();
105105

106106
component.update();
107107

108-
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
109-
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
108+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
109+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
110110
expect(component.text()).toBe('true|bar');
111111
});
112112

113+
it('should pass the values for clientReady and didTimeout', async () => {
114+
const component = mount(
115+
<OptimizelyProvider optimizely={optimizelyMock} timeout={200}>
116+
<OptimizelyFeature feature="feature1" timeout={100}>
117+
{(isEnabled, variables, clientReady, didTimeout) =>
118+
`${isEnabled ? 'true' : 'false'}|${variables.foo}|${clientReady}|${didTimeout}`
119+
}
120+
</OptimizelyFeature>
121+
</OptimizelyProvider>
122+
);
123+
124+
// while it's waiting for onReady()
125+
expect(component.text()).toBe('');
126+
resolver.resolve({ success: true });
127+
128+
await optimizelyMock.onReady();
129+
130+
component.update();
131+
132+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
133+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
134+
expect(component.text()).toBe('true|bar|true|false');
135+
});
136+
113137
it('should respect a locally passed timeout prop', async () => {
114138
const component = mount(
115139
<OptimizelyProvider optimizely={optimizelyMock} timeout={200}>
@@ -123,14 +147,36 @@ describe('<OptimizelyFeature>', () => {
123147

124148
// while it's waiting for onReady()
125149
expect(component.text()).toBe('');
126-
resolver.resolve({ sucess: true });
150+
resolver.resolve({ success: true });
151+
152+
await optimizelyMock.onReady();
153+
154+
component.update();
155+
156+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
157+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
158+
expect(component.text()).toBe('true|bar');
159+
});
160+
161+
it('should pass the override props through', async () => {
162+
const component = mount(
163+
<OptimizelyProvider optimizely={optimizelyMock} timeout={200}>
164+
<OptimizelyFeature feature="feature1" overrideUserId="james123" overrideAttributes={{ betaUser: true }}>
165+
{(isEnabled, variables) => `${isEnabled ? 'true' : 'false'}|${variables.foo}`}
166+
</OptimizelyFeature>
167+
</OptimizelyProvider>
168+
);
169+
170+
// while it's waiting for onReady()
171+
expect(component.text()).toBe('');
172+
resolver.resolve({ success: true });
127173

128174
await optimizelyMock.onReady();
129175

130176
component.update();
131177

132-
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
133-
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
178+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', 'james123', { betaUser: true });
179+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', 'james123', { betaUser: true });
134180
expect(component.text()).toBe('true|bar');
135181
});
136182

@@ -148,14 +194,14 @@ describe('<OptimizelyFeature>', () => {
148194

149195
// while it's waiting for onReady()
150196
expect(component.text()).toBe('');
151-
resolver.resolve({ sucess: true });
197+
resolver.resolve({ success: true });
152198

153199
await optimizelyMock.onReady();
154200

155201
component.update();
156202

157-
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
158-
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
203+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
204+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
159205
expect(component.text()).toBe('true|bar');
160206

161207
const updateFn = (optimizelyMock.notificationCenter.addNotificationListener as jest.Mock).mock.calls[0][1];
@@ -190,14 +236,14 @@ describe('<OptimizelyFeature>', () => {
190236

191237
// while it's waiting for onReady()
192238
expect(component.text()).toBe('');
193-
resolver.resolve({ sucess: true });
239+
resolver.resolve({ success: true });
194240

195241
await optimizelyMock.onReady();
196242

197243
component.update();
198244

199-
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
200-
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
245+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
246+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
201247
expect(component.text()).toBe('true|bar');
202248

203249
const updateFn = (optimizelyMock.onUserUpdate as jest.Mock).mock.calls[0][0];
@@ -233,14 +279,14 @@ describe('<OptimizelyFeature>', () => {
233279

234280
// while it's waiting for onReady()
235281
expect(component.text()).toBe('');
236-
resolver.resolve({ sucess: false, reason: 'fail' });
282+
resolver.resolve({ success: false, reason: 'fail', dataReadyPromise: Promise.resolve() });
237283

238-
await optimizelyMock.onReady();
284+
await optimizelyMock.onReady().then(res => res.dataReadyPromise);
239285

240286
component.update();
241287

242-
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
243-
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
288+
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
289+
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
244290
expect(component.text()).toBe('true|bar');
245291
});
246292
});

‎src/Feature.tsx

Lines changed: 27 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -14,148 +14,42 @@
1414
* limitations under the License.
1515
*/
1616
import * as React from 'react';
17-
import { withOptimizely, WithOptimizelyProps } from './withOptimizely';
18-
import { VariableValuesObject, OnReadyResult, DEFAULT_ON_READY_TIMEOUT } from './client';
19-
import { getLogger } from '@optimizely/js-sdk-logging';
17+
import { UserAttributes } from '@optimizely/optimizely-sdk';
2018

21-
const logger = getLogger('<OptimizelyFeature>');
19+
import { VariableValuesObject } from './client';
20+
import { useFeature } from './hooks';
21+
import { withOptimizely, WithOptimizelyProps } from './withOptimizely';
2222

2323
export interface FeatureProps extends WithOptimizelyProps {
24-
// TODO add support for overrideUserId
2524
feature: string;
2625
timeout?: number;
2726
autoUpdate?: boolean;
28-
children: (isEnabled: boolean, variables: VariableValuesObject) => React.ReactNode;
29-
}
30-
31-
export interface FeatureState {
32-
canRender: boolean;
33-
isEnabled: boolean;
34-
variables: VariableValuesObject;
27+
overrideUserId?: string;
28+
overrideAttributes?: UserAttributes;
29+
children: (
30+
isEnabled: boolean,
31+
variables: VariableValuesObject,
32+
clientReady: boolean,
33+
didTimeout: boolean
34+
) => React.ReactNode;
3535
}
3636

37-
class FeatureComponent extends React.Component<FeatureProps, FeatureState> {
38-
private optimizelyNotificationId?: number;
39-
private unregisterUserListener: () => void;
40-
private autoUpdate = false;
41-
42-
constructor(props: FeatureProps) {
43-
super(props);
44-
45-
this.unregisterUserListener = () => {};
46-
47-
const { autoUpdate, isServerSide, optimizely, feature } = props;
48-
this.autoUpdate = !!autoUpdate;
49-
if (isServerSide) {
50-
if (optimizely === null) {
51-
throw new Error('optimizely prop must be supplied');
52-
}
53-
const isEnabled = optimizely.isFeatureEnabled(feature);
54-
const variables = optimizely.getFeatureVariables(feature);
55-
this.state = {
56-
canRender: true,
57-
isEnabled,
58-
variables,
59-
};
60-
} else {
61-
this.state = {
62-
canRender: false,
63-
isEnabled: false,
64-
variables: {},
65-
};
66-
}
67-
}
68-
69-
componentDidMount() {
70-
const { feature, optimizely, optimizelyReadyTimeout, isServerSide, timeout } = this.props;
71-
if (!optimizely) {
72-
throw new Error('optimizely prop must be supplied');
73-
}
74-
75-
if (isServerSide) {
76-
return;
77-
}
78-
79-
// allow overriding of the ready timeout via the `timeout` prop passed to <Experiment />
80-
const finalReadyTimeout: number | undefined = timeout !== undefined ? timeout : optimizelyReadyTimeout;
81-
82-
optimizely.onReady({ timeout: finalReadyTimeout }).then((res: OnReadyResult) => {
83-
if (res.success) {
84-
logger.info('feature="%s" successfully rendered for user="%s"', feature, optimizely.user.id);
85-
} else {
86-
logger.info(
87-
'feature="%s" could not be checked before timeout of %sms, reason="%s" ',
88-
feature,
89-
timeout === undefined ? DEFAULT_ON_READY_TIMEOUT : timeout,
90-
res.reason || ''
91-
);
92-
}
93-
94-
const isEnabled = optimizely.isFeatureEnabled(feature);
95-
const variables = optimizely.getFeatureVariables(feature);
96-
this.setState({
97-
canRender: true,
98-
isEnabled,
99-
variables,
100-
});
101-
102-
if (this.autoUpdate) {
103-
this.setupAutoUpdateListeners();
104-
}
105-
});
106-
}
107-
108-
setupAutoUpdateListeners() {
109-
const { optimizely, feature } = this.props;
110-
if (optimizely === null) {
111-
return;
112-
}
113-
114-
this.optimizelyNotificationId = optimizely.notificationCenter.addNotificationListener(
115-
'OPTIMIZELY_CONFIG_UPDATE',
116-
() => {
117-
logger.info('OPTIMIZELY_CONFIG_UPDATE, re-evaluating feature="%s" for user="%s"', feature, optimizely.user.id);
118-
const isEnabled = optimizely.isFeatureEnabled(feature);
119-
const variables = optimizely.getFeatureVariables(feature);
120-
this.setState({
121-
isEnabled,
122-
variables,
123-
});
124-
}
125-
);
126-
127-
this.unregisterUserListener = optimizely.onUserUpdate(() => {
128-
logger.info('User update, re-evaluating feature="%s" for user="%s"', feature, optimizely.user.id);
129-
const isEnabled = optimizely.isFeatureEnabled(feature);
130-
const variables = optimizely.getFeatureVariables(feature);
131-
this.setState({
132-
isEnabled,
133-
variables,
134-
});
135-
});
37+
const FeatureComponent: React.FunctionComponent<FeatureProps> = props => {
38+
const { feature, timeout, autoUpdate, children, overrideUserId, overrideAttributes } = props;
39+
const [isEnabled, variables, clientReady, didTimeout] = useFeature(
40+
feature,
41+
{ timeout, autoUpdate },
42+
{ overrideUserId, overrideAttributes }
43+
);
44+
45+
if (!clientReady && !didTimeout) {
46+
// Only block rendering while were waiting for the client within the allowed timeout.
47+
return null;
13648
}
13749

138-
componentWillUnmount() {
139-
const { optimizely, isServerSide } = this.props;
140-
if (isServerSide || !this.autoUpdate) {
141-
return;
142-
}
143-
if (optimizely && this.optimizelyNotificationId) {
144-
optimizely.notificationCenter.removeNotificationListener(this.optimizelyNotificationId);
145-
}
146-
this.unregisterUserListener();
147-
}
148-
149-
render() {
150-
const { children } = this.props;
151-
const { isEnabled, variables, canRender } = this.state;
152-
153-
if (!canRender) {
154-
return null;
155-
}
156-
157-
return children(isEnabled, variables);
158-
}
159-
}
50+
// Wrap the return value here in a Fragment to please the HOC's expected React.ComponentType
51+
// See https://github.com/DefinitelyTyped/DefinitelyTyped/issues/18051
52+
return <>{children(isEnabled, variables, clientReady, didTimeout)}</>;
53+
};
16054

16155
export const OptimizelyFeature = withOptimizely(FeatureComponent);

‎src/hooks.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616
import { useCallback, useContext, useEffect, useState } from 'react';
17-
import * as optimizely from '@optimizely/optimizely-sdk';
17+
import { UserAttributes } from '@optimizely/optimizely-sdk';
1818
import { getLogger } from '@optimizely/js-sdk-logging';
1919

2020
import { setupAutoUpdateListeners } from './autoUpdate';
@@ -38,7 +38,7 @@ type UseFeatureOptions = {
3838

3939
type UseFeatureOverrides = {
4040
overrideUserId?: string;
41-
overrideAttributes?: optimizely.UserAttributes;
41+
overrideAttributes?: UserAttributes;
4242
};
4343

4444
interface UseFeature {

0 commit comments

Comments
 (0)