Skip to content

enhancement (<OptimizelyFeature>): Convert to functional component that uses useFeature hook #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 15, 2020
Merged

enhancement (<OptimizelyFeature>): Convert to functional component that uses useFeature hook #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
14f657a
convert <OptimizelyFeature> component to functional component that us…
Mar 11, 2020
c57e44e
update tests
Mar 13, 2020
783ba81
update changelog
Mar 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

## [Unreleased]

- 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.

### New Features

- Added `useFeature` hook
- 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.

### Enhancements

- Exposed the entire context object used by
- Exposed the entire context object used by `<OptimizelyProvider>`.
- 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.


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

### New Features
Expand Down
82 changes: 64 additions & 18 deletions src/Feature.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ describe('<OptimizelyFeature>', () => {

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar');
});

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

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

await optimizelyMock.onReady();

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar');
});

it('should pass the values for clientReady and didTimeout', async () => {
const component = mount(
<OptimizelyProvider optimizely={optimizelyMock} timeout={200}>
<OptimizelyFeature feature="feature1" timeout={100}>
{(isEnabled, variables, clientReady, didTimeout) =>
`${isEnabled ? 'true' : 'false'}|${variables.foo}|${clientReady}|${didTimeout}`
}
</OptimizelyFeature>
</OptimizelyProvider>
);

// while it's waiting for onReady()
expect(component.text()).toBe('');
resolver.resolve({ success: true });

await optimizelyMock.onReady();

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar|true|false');
});

it('should respect a locally passed timeout prop', async () => {
const component = mount(
<OptimizelyProvider optimizely={optimizelyMock} timeout={200}>
Expand All @@ -123,14 +147,36 @@ describe('<OptimizelyFeature>', () => {

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

await optimizelyMock.onReady();

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar');
});

it('should pass the override props through', async () => {
const component = mount(
<OptimizelyProvider optimizely={optimizelyMock} timeout={200}>
<OptimizelyFeature feature="feature1" overrideUserId="james123" overrideAttributes={{ betaUser: true }}>
{(isEnabled, variables) => `${isEnabled ? 'true' : 'false'}|${variables.foo}`}
</OptimizelyFeature>
</OptimizelyProvider>
);

// while it's waiting for onReady()
expect(component.text()).toBe('');
resolver.resolve({ success: true });

await optimizelyMock.onReady();

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', 'james123', { betaUser: true });
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', 'james123', { betaUser: true });
expect(component.text()).toBe('true|bar');
});

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

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

await optimizelyMock.onReady();

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar');

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

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

await optimizelyMock.onReady();

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar');

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

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

await optimizelyMock.onReady();
await optimizelyMock.onReady().then(res => res.dataReadyPromise);

component.update();

expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1');
expect(optimizelyMock.isFeatureEnabled).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(optimizelyMock.getFeatureVariables).toHaveBeenCalledWith('feature1', undefined, undefined);
expect(component.text()).toBe('true|bar');
});
});
Expand Down
160 changes: 27 additions & 133 deletions src/Feature.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,148 +14,42 @@
* limitations under the License.
*/
import * as React from 'react';
import { withOptimizely, WithOptimizelyProps } from './withOptimizely';
import { VariableValuesObject, OnReadyResult, DEFAULT_ON_READY_TIMEOUT } from './client';
import { getLogger } from '@optimizely/js-sdk-logging';
import { UserAttributes } from '@optimizely/optimizely-sdk';

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

export interface FeatureProps extends WithOptimizelyProps {
// TODO add support for overrideUserId
feature: string;
timeout?: number;
autoUpdate?: boolean;
children: (isEnabled: boolean, variables: VariableValuesObject) => React.ReactNode;
}

export interface FeatureState {
canRender: boolean;
isEnabled: boolean;
variables: VariableValuesObject;
overrideUserId?: string;
overrideAttributes?: UserAttributes;
children: (
isEnabled: boolean,
variables: VariableValuesObject,
clientReady: boolean,
didTimeout: boolean
) => React.ReactNode;
}

class FeatureComponent extends React.Component<FeatureProps, FeatureState> {
private optimizelyNotificationId?: number;
private unregisterUserListener: () => void;
private autoUpdate = false;

constructor(props: FeatureProps) {
super(props);

this.unregisterUserListener = () => {};

const { autoUpdate, isServerSide, optimizely, feature } = props;
this.autoUpdate = !!autoUpdate;
if (isServerSide) {
if (optimizely === null) {
throw new Error('optimizely prop must be supplied');
}
const isEnabled = optimizely.isFeatureEnabled(feature);
const variables = optimizely.getFeatureVariables(feature);
this.state = {
canRender: true,
isEnabled,
variables,
};
} else {
this.state = {
canRender: false,
isEnabled: false,
variables: {},
};
}
}

componentDidMount() {
const { feature, optimizely, optimizelyReadyTimeout, isServerSide, timeout } = this.props;
if (!optimizely) {
throw new Error('optimizely prop must be supplied');
}

if (isServerSide) {
return;
}

// allow overriding of the ready timeout via the `timeout` prop passed to <Experiment />
const finalReadyTimeout: number | undefined = timeout !== undefined ? timeout : optimizelyReadyTimeout;

optimizely.onReady({ timeout: finalReadyTimeout }).then((res: OnReadyResult) => {
if (res.success) {
logger.info('feature="%s" successfully rendered for user="%s"', feature, optimizely.user.id);
} else {
logger.info(
'feature="%s" could not be checked before timeout of %sms, reason="%s" ',
feature,
timeout === undefined ? DEFAULT_ON_READY_TIMEOUT : timeout,
res.reason || ''
);
}

const isEnabled = optimizely.isFeatureEnabled(feature);
const variables = optimizely.getFeatureVariables(feature);
this.setState({
canRender: true,
isEnabled,
variables,
});

if (this.autoUpdate) {
this.setupAutoUpdateListeners();
}
});
}

setupAutoUpdateListeners() {
const { optimizely, feature } = this.props;
if (optimizely === null) {
return;
}

this.optimizelyNotificationId = optimizely.notificationCenter.addNotificationListener(
'OPTIMIZELY_CONFIG_UPDATE',
() => {
logger.info('OPTIMIZELY_CONFIG_UPDATE, re-evaluating feature="%s" for user="%s"', feature, optimizely.user.id);
const isEnabled = optimizely.isFeatureEnabled(feature);
const variables = optimizely.getFeatureVariables(feature);
this.setState({
isEnabled,
variables,
});
}
);

this.unregisterUserListener = optimizely.onUserUpdate(() => {
logger.info('User update, re-evaluating feature="%s" for user="%s"', feature, optimizely.user.id);
const isEnabled = optimizely.isFeatureEnabled(feature);
const variables = optimizely.getFeatureVariables(feature);
this.setState({
isEnabled,
variables,
});
});
const FeatureComponent: React.FunctionComponent<FeatureProps> = props => {
const { feature, timeout, autoUpdate, children, overrideUserId, overrideAttributes } = props;
const [isEnabled, variables, clientReady, didTimeout] = useFeature(
feature,
{ timeout, autoUpdate },
{ overrideUserId, overrideAttributes }
);

if (!clientReady && !didTimeout) {
// Only block rendering while were waiting for the client within the allowed timeout.
return null;
}

componentWillUnmount() {
const { optimizely, isServerSide } = this.props;
if (isServerSide || !this.autoUpdate) {
return;
}
if (optimizely && this.optimizelyNotificationId) {
optimizely.notificationCenter.removeNotificationListener(this.optimizelyNotificationId);
}
this.unregisterUserListener();
}

render() {
const { children } = this.props;
const { isEnabled, variables, canRender } = this.state;

if (!canRender) {
return null;
}

return children(isEnabled, variables);
}
}
// Wrap the return value here in a Fragment to please the HOC's expected React.ComponentType
// See https://github.com/DefinitelyTyped/DefinitelyTyped/issues/18051
return <>{children(isEnabled, variables, clientReady, didTimeout)}</>;
};

export const OptimizelyFeature = withOptimizely(FeatureComponent);
4 changes: 2 additions & 2 deletions src/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/
import { useCallback, useContext, useEffect, useState } from 'react';
import * as optimizely from '@optimizely/optimizely-sdk';
Copy link
Contributor Author

@jamesopti jamesopti Mar 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should have caused a lint error below (using the variable name optimizely when its already declared in outer scope)

@mjc1283 - Is the tsc compiler smart enough to figure out which usage is a type?

The scopes make it work as expected but I thought the tsc linter would complain.

Copy link
Contributor

@mjc1283 mjc1283 Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to linting against this. In our example code we try to use optimizelySdk as the name of the import.

import { UserAttributes } from '@optimizely/optimizely-sdk';
import { getLogger } from '@optimizely/js-sdk-logging';

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

type UseFeatureOverrides = {
overrideUserId?: string;
overrideAttributes?: optimizely.UserAttributes;
overrideAttributes?: UserAttributes;
};

interface UseFeature {
Expand Down