Merge branch 'hooks_baseline' into hooks_update_options

Author: EmilianoSanchez

.eslintrc.js

@@ -29,7 +29,7 @@ module.exports = {
     'eol-last': ['error', 'always'],
     'keyword-spacing': 'error',
     'no-trailing-spaces': 'error',
-    'space-before-function-paren': ['error', {'named': 'never'}],
+    'space-before-function-paren': ['error', { 'named': 'never' }],
     'react/display-name': 'off',
     '@typescript-eslint/no-empty-function': 'off',
     '@typescript-eslint/no-inferrable-types': 'off',
@@ -40,6 +40,11 @@ module.exports = {
       'destructuring': 'all'
     }]
   },
+  'settings': {
+    'react': {
+      'version': '16.3.0' // minimum supported version of React
+    }
+  },
   'overrides': [{
     'files': ['src/**/*.ts', 'src/**/*.tsx'],
     'excludedFiles': ['src/**/__tests__/**'],
@@ -53,7 +58,7 @@ module.exports = {
     },
     'rules': {
       'no-restricted-syntax': ['error', 'ForOfStatement', 'ForInStatement'],
-      'compat/compat': ['error', 'defaults, not ie < 11'],
+      'compat/compat': ['error', 'defaults, ie 11'],
       'no-throw-literal': 'error',
       'import/no-self-import': 'error',
       'import/no-default-export': 'error',

CHANGES.txt

@@ -1,5 +1,8 @@
-1.10.0 (XXX XX, 2023)
- - Bugfixing - Removed check of hooks availability to follow rules of hooks. If attempting to use hooks with React below version 16.8.0, an error will be thrown rather than logging an error message.
+1.10.0 (September XX, 2023)
+ - Added TypeScript types and interfaces to the library index exports, allowing them to be imported from the library index. For example, `import type { ISplitFactoryProps } from '@splitsoftware/splitio-react';` (Related to issue https://github.com/splitio/react-client/issues/162).
+ - Updated linter and other dependencies for vulnerability fixes.
+ - Bugfixing - To adhere to the rules of hooks and prevent React warnings, conditional code within hooks was removed. Previously, this code checked for the availability of the hooks API (available in React version 16.8.0 or above) and logged an error message. Now, using hooks with React versions below 16.8.0 will throw an error.
+ - Bugfixing - Updated `useClient` and `useTreatments` hooks to re-render and re-evaluate feature flags when they consume a different SDK client than the context and its status updates (i.e., when it emits SDK_READY or other event).
 
 1.9.0 (July 18, 2023)
  - Updated some transitive dependencies for vulnerability fixes.

package-lock.json

@@ -75,10 +75,10 @@
     "@types/react-dom": "^18.0.0",
     "@types/react-test-renderer": "^18.0.0",
     "@types/shallowequal": "^1.1.1",
-    "@typescript-eslint/eslint-plugin": "^5.55.0",
-    "@typescript-eslint/parser": "^5.55.0",
-    "eslint": "^8.36.0",
-    "eslint-plugin-compat": "^4.1.2",
+    "@typescript-eslint/eslint-plugin": "^6.6.0",
+    "@typescript-eslint/parser": "^6.6.0",
+    "eslint": "^8.48.0",
+    "eslint-plugin-compat": "^4.2.0",
     "eslint-plugin-import": "^2.27.5",
     "eslint-plugin-react": "^7.32.2",
     "eslint-plugin-react-hooks": "^4.6.0",

package.json

@@ -3,6 +3,7 @@ import { SplitContext } from './SplitContext';
 import { ISplitClientProps, ISplitContextValues, IUpdateProps } from './types';
 import { ERROR_SC_NO_FACTORY } from './constants';
 import { getStatus, getSplitClient, initAttributes, IClientWithContext } from './utils';
+import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient';
 
 /**
  * Common component used to handle the status and events of a Split client passed as prop.
@@ -11,18 +12,15 @@ import { getStatus, getSplitClient, initAttributes, IClientWithContext } from '.
 export class SplitComponent extends React.Component<IUpdateProps & { factory: SplitIO.IBrowserSDK | null, client: SplitIO.IBrowserClient | null, attributes?: SplitIO.Attributes, children: any }, ISplitContextValues> {
 
   static defaultProps = {
-    updateOnSdkUpdate: false,
-    updateOnSdkTimedout: false,
-    updateOnSdkReady: true,
-    updateOnSdkReadyFromCache: true,
     children: null,
     factory: null,
     client: null,
+    ...DEFAULT_UPDATE_OPTIONS,
   }
 
   // Using `getDerivedStateFromProps` since the state depends on the status of the client in props, which might change over time.
   // It could be avoided by removing the client and its status from the component state.
-  // But it implies to have another instance property to use instead of the state, because we need a unique reference value for SplitContext.Producer
+  // But it implies to have another instance property to use instead of the state, because we need a unique reference value for SplitContext.Provider
   static getDerivedStateFromProps(props: ISplitClientProps & { factory: SplitIO.IBrowserSDK | null, client: SplitIO.IBrowserClient | null }, state: ISplitContextValues) {
     const { client, factory, attributes } = props;
     // initAttributes can be called in the `render` method too, but it is better here for separation of concerns

src/SplitClient.tsx

@@ -4,6 +4,7 @@ import { SplitComponent } from './SplitClient';
 import { ISplitFactoryProps } from './types';
 import { WARN_SF_CONFIG_AND_FACTORY, ERROR_SF_NO_CONFIG_AND_FACTORY } from './constants';
 import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClient } from './utils';
+import { DEFAULT_UPDATE_OPTIONS } from './useSplitClient';
 
 /**
  * SplitFactory will initialize the Split SDK and its main client, listen for its events in order to update the Split Context,
@@ -18,11 +19,8 @@ import { getSplitFactory, destroySplitFactory, IFactoryWithClients, getSplitClie
 export class SplitFactory extends React.Component<ISplitFactoryProps, { factory: SplitIO.IBrowserSDK | null, client: SplitIO.IBrowserClient | null }> {
 
   static defaultProps: ISplitFactoryProps = {
-    updateOnSdkUpdate: false,
-    updateOnSdkTimedout: false,
-    updateOnSdkReady: true,
-    updateOnSdkReadyFromCache: true,
     children: null,
+    ...DEFAULT_UPDATE_OPTIONS,
   };
 
   readonly state: Readonly<{ factory: SplitIO.IBrowserSDK | null, client: SplitIO.IBrowserClient | null }>;

src/SplitFactory.tsx

@@ -253,36 +253,18 @@ describe('SplitClient', () => {
         this.state = { splitKey: 'user1' };
       }
 
-      componentDidMount() {
-        setTimeout(() => {
-          act(() => this.setState({ splitKey: 'user2' }));
-          setTimeout(() => {
-            act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT));
-            setTimeout(() => {
-              act(() => (outerFactory as any).client('user1').__emitter__.emit(Event.SDK_READY_TIMED_OUT));
-              setTimeout(() => {
-                act(() => this.setState({ splitKey: 'user3' }));
-                setTimeout(() => {
-                  act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY));
-                  setTimeout(() => {
-                    act(() => (outerFactory as any).client('user3').__emitter__.emit(Event.SDK_READY));
-                    setTimeout(() => {
-                      act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_UPDATE));
-                      setTimeout(() => {
-                        act(() => (outerFactory as any).client('user3').__emitter__.emit(Event.SDK_UPDATE));
-                        setTimeout(() => {
-                          expect(renderTimes).toBe(6);
-
-                          done();
-                        });
-                      });
-                    });
-                  });
-                });
-              });
-            });
-          });
-        });
+      async componentDidMount() {
+        await act(() => this.setState({ splitKey: 'user2' }));
+        await act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT));
+        await act(() => (outerFactory as any).client('user1').__emitter__.emit(Event.SDK_READY_TIMED_OUT));
+        await act(() => this.setState({ splitKey: 'user3' }));
+        await act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY));
+        await act(() => (outerFactory as any).client('user3').__emitter__.emit(Event.SDK_READY));
+        await act(() => (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_UPDATE));
+        await act(() => (outerFactory as any).client('user3').__emitter__.emit(Event.SDK_UPDATE));
+        expect(renderTimes).toBe(6);
+
+        done();
       }
 
       render() {

src/__tests__/SplitClient.test.tsx

@@ -224,7 +224,7 @@ describe('SplitTreatments optimization', () => {
     expect(outerFactory.client().getTreatmentsWithConfig).toBeCalledTimes(2);
   });
 
-  it('rerenders and re-evaluates feature flags if lastUpdate timestamp changes (e.g., SDK_UPDATE event).', (done) => {
+  it('rerenders and re-evaluates feature flags if lastUpdate timestamp changes (e.g., SDK_UPDATE event).', () => {
     expect(renderTimes).toBe(1);
 
     // State update and split evaluation
@@ -234,16 +234,13 @@ describe('SplitTreatments optimization', () => {
     (outerFactory as any).client().destroy();
     wrapper.rerender(<Component names={names} attributes={attributes} splitKey={splitKey} />);
 
-    setTimeout(() => {
-      // Updates were batched as a single render, due to automatic batching https://reactjs.org/blog/2022/03/29/react-v18.html#new-feature-automatic-batching
-      expect(renderTimes).toBe(3);
-      expect(outerFactory.client().getTreatmentsWithConfig).toBeCalledTimes(2);
+    // Updates were batched as a single render, due to automatic batching https://reactjs.org/blog/2022/03/29/react-v18.html#new-feature-automatic-batching
+    expect(renderTimes).toBe(3);
+    expect(outerFactory.client().getTreatmentsWithConfig).toBeCalledTimes(2);
 
-      // Restore the client to be READY
-      (outerFactory as any).client().__restore();
-      (outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
-      done();
-    })
+    // Restore the client to be READY
+    (outerFactory as any).client().__restore();
+    (outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
   });
 
   it('rerenders and re-evaluates feature flags if client changes.', () => {
@@ -301,8 +298,6 @@ describe('SplitTreatments optimization', () => {
     expect(renderTimesComp1).toBe(2);
     expect(renderTimesComp2).toBe(2); // updateOnSdkReadyFromCache === false, in second component
 
-    // delay SDK events to guarantee a different lastUpdate timestamp for SplitTreatments to re-evaluate
-    await new Promise(resolve => setTimeout(resolve, 10));
     act(() => {
       (outerFactory as any).client().__emitter__.emit(Event.SDK_READY_TIMED_OUT);
       (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY_TIMED_OUT);
@@ -311,7 +306,6 @@ describe('SplitTreatments optimization', () => {
     expect(renderTimesComp1).toBe(3);
     expect(renderTimesComp2).toBe(3);
 
-    await new Promise(resolve => setTimeout(resolve, 10));
     act(() => {
       (outerFactory as any).client().__emitter__.emit(Event.SDK_READY);
       (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_READY);
@@ -320,7 +314,6 @@ describe('SplitTreatments optimization', () => {
     expect(renderTimesComp1).toBe(3); // updateOnSdkReady === false, in first component
     expect(renderTimesComp2).toBe(4);
 
-    await new Promise(resolve => setTimeout(resolve, 10));
     act(() => {
       (outerFactory as any).client().__emitter__.emit(Event.SDK_UPDATE);
       (outerFactory as any).client('user2').__emitter__.emit(Event.SDK_UPDATE);

src/__tests__/SplitTreatments.test.tsx

@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-unused-vars */
 import {
   SplitContext as ExportedSplitContext,
   SplitSdk as ExportedSplitSdk,
@@ -11,6 +12,16 @@ import {
   useManager as exportedUseManager,
   useTrack as exportedUseTrack,
   useTreatments as exportedUseTreatments,
+  // Checks that types are exported. Otherwise, the test would fail with a TS error.
+  ISplitClientChildProps,
+  ISplitClientProps,
+  ISplitContextValues,
+  ISplitFactoryChildProps,
+  ISplitFactoryProps,
+  ISplitStatus,
+  ISplitTreatmentsChildProps,
+  ISplitTreatmentsProps,
+  IUpdateProps
 } from '../index';
 import { SplitContext } from '../SplitContext';
 import { SplitFactory as SplitioEntrypoint } from '@splitsoftware/splitio/client';

src/__tests__/index.test.ts

@@ -80,26 +80,42 @@ describe('useTreatments', () => {
     expect(client.getTreatmentsWithConfig).toHaveReturnedWith(treatments);
   });
 
-  test('returns the Treatments from a new client given a splitKey, or control if the client is not operational.', async () => {
+  test('returns the Treatments from a new client given a splitKey, and re-evaluates on SDK events.', () => {
     const outerFactory = SplitSdk(sdkBrowser);
     const client: any = outerFactory.client('user2');
-    let treatments;
-
-    client.__emitter__.emit(Event.SDK_READY);
-    await client.destroy();
+    let renderTimes = 0;
 
     render(
       <SplitFactory factory={outerFactory} >
         {React.createElement(() => {
-          treatments = useTreatments(featureFlagNames, attributes, 'user2');
+          const treatments = useTreatments(featureFlagNames, attributes, 'user2');
+
+          renderTimes++;
+          switch (renderTimes) {
+            case 1:
+              // returns control if not operational (SDK not ready), without calling `getTreatmentsWithConfig` method
+              expect(client.getTreatmentsWithConfig).not.toBeCalled();
+              expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG });
+              break;
+            case 2:
+            case 3:
+              // once operational (SDK_READY or SDK_READY_FROM_CACHE), it evaluates feature flags
+              expect(client.getTreatmentsWithConfig).toHaveBeenLastCalledWith(featureFlagNames, attributes);
+              expect(client.getTreatmentsWithConfig).toHaveLastReturnedWith(treatments);
+              break;
+            default:
+              throw new Error('Unexpected render');
+          }
+
           return null;
         })}
       </SplitFactory>
     );
 
-    // returns control treatment if not operational (SDK not ready or destroyed), without calling `getTreatmentsWithConfig` method
-    expect(client.getTreatmentsWithConfig).not.toBeCalled();
-    expect(treatments).toEqual({ split1: CONTROL_WITH_CONFIG });
+    act(() => client.__emitter__.emit(Event.SDK_READY_FROM_CACHE));
+    act(() => client.__emitter__.emit(Event.SDK_READY));
+    act(() => client.__emitter__.emit(Event.SDK_UPDATE)); // should not trigger a re-render by default
+    expect(client.getTreatmentsWithConfig).toBeCalledTimes(2);
   });
 
   // THE FOLLOWING TEST WILL PROBABLE BE CHANGED BY 'return a null value or throw an error if it is not inside an SplitProvider'