Merge pull request #2 from CarsonF/functional-provider

SplitProvider - Functional and aware of config changes with optimization improvements.
samuelcastro · Aug 22, 2019 · 6580ad7 · 6580ad7
2 parents 43fe38e + a45a85c
commit 6580ad7
Show file tree

Hide file tree

Showing 4 changed files with 154 additions and 82 deletions.
diff --git a/README.md b/README.md
@@ -5,6 +5,7 @@ A [Split.io](https://www.split.io/) library to easily manage splits in React.
 ## Get Started
 
 - [Installation](#installation)
+- [Configuration](#configuration)
 - [Usage](#usage)
 - [Contributions](#install-dependencies)
 - [All Available Scripts](#all-available-scripts)
@@ -24,7 +25,7 @@ samuelcastro@mac:~$ yarn add react-splitio
 samuelcastro@mac:~$ npm install react-splitio
 ```
 
-## Usage
+## Configuration
 
 On your root component define the Split provider:
 
@@ -34,6 +35,49 @@ On your root component define the Split provider:
 </SplitProvider>
 ```
 
+### Performance
+Note that if your `SDK_CONFIG_OBJECT` is defined inside of a component it will create unnecessary work for `SplitProvider`,
+because the object will have a different identity each render (`previousConfig !== newConfig`).
+
+Instead define config outside of your component:
+```tsx
+const config = { ... };
+
+const Root = () => (
+  <SplitProvider config={config}>
+    <App />
+  </SplitProvider>
+)
+```
+Or if you need to configure dynamically, memoize the object:
+```tsx
+const MySplitProvider = ({ trafficType, children }) => {
+  const config = useMemo(() => ({
+    core: {
+      authorizationKey: '',
+      trafficType,
+    }
+  }), [trafficType]);
+  return (
+    <SplitProvider config={config}>
+      {children}
+    </SplitProvider>
+  );
+};
+```
+
+### Impression Listener
+
+Split allows you to [implement a custom impression listener](https://help.split.io/hc/en-us/articles/360020564931-Node-js-SDK#listener).
+`SplitProvider` has an optional convenience `onImpression` callback you can use instead.
+```tsx
+<SplitProvider config={} onImpression={impressionData => {
+  // do something with the impression data.
+}}>
+```
+
+## Usage
+
 Now assuming you have a split named: `feature1` you can do something like:
 
 ```jsx

diff --git a/src/Split.tsx b/src/Split.tsx
@@ -16,7 +16,7 @@ const Split: React.SFC<ISplitProps> = ({ name, children }) => (
   <SplitContext.Consumer>
     {({ client, isReady, lastUpdate }: ISplitContextValues) =>
       children(
-        isReady
+        client && isReady
           ? name instanceof Array
             ? client.getTreatmentsWithConfig(name as string[])
             : client.getTreatmentWithConfig(name as string)

diff --git a/src/SplitProvider.tsx b/src/SplitProvider.tsx
@@ -1,30 +1,28 @@
 import { SplitFactory } from '@splitsoftware/splitio';
-import React, { createContext } from 'react';
+import React, {
+  createContext,
+  useCallback,
+  useEffect,
+  useMemo,
+  useState,
+} from 'react';
 
-import {
-  ISplitContextValues,
-  ISplitProviderProps,
-  SplitReactContext,
-} from './types';
+import { ISplitContextValues, ISplitProviderProps } from './types';
 
 /**
  * Creating a React.Context with default values.
- * @returns {SplitReactContext}
  */
-export const SplitContext: SplitReactContext = createContext<
-  ISplitContextValues
->({
-  client: {} as SplitIO.IClient,
+export const SplitContext = createContext<ISplitContextValues>({
+  client: null,
   isReady: false,
-  lastUpdate: null,
+  lastUpdate: 0,
 });
 
 /**
  * SplitProvider will initialize client and listen for events that will set things up.
  * Make sure SplitProvider is wrapper your entire app.
  * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK}
  * @param {ISplitProviderProps} props
- * @param {ISplitContextValues} state
  * @returns {React.Component}
  *
  * @example
@@ -33,74 +31,97 @@ export const SplitContext: SplitReactContext = createContext<
  *    <App />
  *  </SplitProvider>
  */
-const SplitProvider = class extends React.Component<
-  ISplitProviderProps,
-  ISplitContextValues
-> {
-  constructor(props: ISplitProviderProps) {
-    super(props);
+const SplitProvider = ({
+  config,
+  children,
+  onImpression,
+}: ISplitProviderProps) => {
+  const [client, setClient] = useState<SplitIO.IClient | null>(null);
+  const [{ isReady, lastUpdate }, setUpdated] = useState({
+    isReady: false,
+    lastUpdate: 0,
+  });
 
-    /**
-     * Instatiating a factory in order to create a client.
-     * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#2-instantiate-the-sdk-and-create-a-new-split-client}
-     */
-    const factory: SplitIO.ISDK = SplitFactory(props.config);
+  // Handle impression listener separately from config.
+  // - Allows us to use the onImpression property
+  // - And because the listener function is ignored from JSON.stringify,
+  //   so changing that in the config wouldn't hook up the new function.
+  // - Because of this ^ the function can change without us having to recreate the client.
+  const handleImpression = useCallback((data: SplitIO.ImpressionData) => {
+    if (onImpression) {
+      onImpression(data);
+    }
+    if (config.impressionListener && config.impressionListener.logImpression) {
+      config.impressionListener.logImpression(data);
+    }
+  }, []);
 
-    this.state = {
-      client: factory.client(),
-      isReady: false,
-      lastUpdate: null,
-    };
-  }
-
-  /**
-   * Listening for split events
-   */
-  componentDidMount() {
-    const { client } = this.state;
+  // Determine whether config has changed which would require client to be recreated.
+  // Convert config object to string so it works with the identity check ran with useEffect's dependency list.
+  // We memoize this so if the user has memoized their config object we don't call JSON.stringify needlessly.
+  // We also freeze the config object here so users know modifying it is not what they want to do.
+  const configHash = useMemo(() => JSON.stringify(deepFreeze(config)), [
+    config,
+  ]);
 
-    /**
-     * When SDK is ready this isReady to true
-     */
-    client.on(client.Event.SDK_READY, () => this.setState({ isReady: true }));
+  useEffect(() => {
+    // Reset state when re-creating the client after config modification
+    if (isReady || lastUpdate > 0) {
+      setUpdated({ isReady: false, lastUpdate: 0 });
+    }
 
-    /**
-     * When an update occurs update lastUpdate, this will force a re-render
-     */
-    client.on(client.Event.SDK_UPDATE, () =>
-      this.setState({
-        lastUpdate: Date.now(),
-      }),
-    );
+    /** @link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#2-instantiate-the-sdk-and-create-a-new-split-client */
+    const nextClient = SplitFactory({
+      ...config,
+      impressionListener: {
+        logImpression: handleImpression,
+      },
+    }).client();
+    setClient(nextClient);
 
-    // TODO: Are there any other events that we need to listen?
-  }
+    // Only make state changes if component is mounted.
+    // https://github.com/facebook/react/issues/14369#issuecomment-468267798
+    let isMounted = true;
+    const updateListener = () => {
+      if (isMounted) {
+        setUpdated({ isReady: true, lastUpdate: Date.now() });
+      }
+    };
+    nextClient.on(nextClient.Event.SDK_READY, updateListener);
+    nextClient.on(nextClient.Event.SDK_UPDATE, updateListener);
 
-  /**
-   * Destroying client instance when component unmonts
-   */
-  componentWillUnmount() {
-    const { client } = this.state;
+    return () => {
+      isMounted = false;
+      if (client) {
+        client.destroy();
+      }
+    };
+  }, [configHash]);
 
-    client.destroy();
-  }
+  return (
+    <SplitContext.Provider
+      value={{
+        client,
+        isReady,
+        lastUpdate,
+      }}
+    >
+      {children}
+    </SplitContext.Provider>
+  );
+};
 
-  render() {
-    const { isReady, client, lastUpdate } = this.state;
-    const { children } = this.props;
+export default SplitProvider;
 
-    return (
-      <SplitContext.Provider
-        value={{
-          client,
-          isReady,
-          lastUpdate,
-        }}
-      >
-        {children}
-      </SplitContext.Provider>
-    );
+const deepFreeze = <T extends {}>(object: T): T => {
+  // Freeze properties before freezing self
+  const propNames = Object.getOwnPropertyNames(object);
+  for (const name of propNames) {
+    const value = object[name];
+    if (value && typeof value === 'object') {
+      object[name] = deepFreeze(value);
+    }
   }
-};
 
-export default SplitProvider;
+  return Object.freeze(object);
+};
diff --git a/src/types.ts b/src/types.ts
@@ -17,7 +17,7 @@ export interface ISplitContextValues {
    * @property {SplitIO.IClient} client
    * @see {@link https://help.split.io/hc/en-us/articles/360020448791-JavaScript-SDK#2-instantiate-the-sdk-and-create-a-new-split-client}
    */
-  client: SplitIO.IClient;
+  client: SplitIO.IClient | null;
 
   /**
    * isReady is a property that will show up when the client SDK is ready to be consumed.
@@ -28,9 +28,9 @@ export interface ISplitContextValues {
 
   /**
    * Shows up when was the last SDK update
-   * @property {number | null} lastUpdate
+   * @property {number} lastUpdate
    */
-  lastUpdate: number | null;
+  lastUpdate: number;
 }
 
 /**
@@ -41,10 +41,9 @@ export type SplitReactContext = Context<ISplitContextValues>;
 
 /**
  * Split Provider interface. Interface that will be implemented in order to create a split provider
- * with the SDK browse settings information. The provider will create client out of factory listening
+ * with the SDK browser settings. The provider will create client out of factory and listen
  * for SDK events.
  * @interface ISplitProviderProps
- * @see {@link https://docs.split.io/docs/nodejs-sdk-overview#section-listener}
  */
 export interface ISplitProviderProps {
   /**
@@ -54,6 +53,14 @@ export interface ISplitProviderProps {
    */
   config: IBrowserSettings;
 
+  /**
+   * Called when an impression is evaluated.
+   * This is a convince property that's idiomatic with React. The config option works as well.
+   * @see {@link https://help.split.io/hc/en-us/articles/360020564931-Node-js-SDK#listener}
+   * @property {Function} onImpression
+   */
+  onImpression?: SplitIO.IImpressionListener['logImpression'];
+
   /**
    * Children of our React Split Provider.
    * @property {React.ReactNode} children
@@ -83,7 +90,7 @@ export interface ISplitProps {
    */
   children: (
     treatments: TreatmentWithConfig | TreatmentsWithConfig | null,
-    client: SplitIO.IClient,
-    lastUpdate: number | null,
+    client: SplitIO.IClient | null,
+    lastUpdate: number,
   ) => React.ReactNode;
 }