Skip to content
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.

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

Bugfix on splitReducer when handling actions with a null payload #122

Merged
merged 3 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
1.14.1 (October 15, 2024)
- Bugfixing - Fixed error in `splitReducer` when handling actions with a `null` payload, preventing crashes caused by accessing undefined payload properties (Related to https://github.com/splitio/redux-client/issues/121).

1.14.0 (September 13, 2024)
- Added `status` property to Split reducer's slice of state to track the SDK events of non-default clients (Related to https://github.com/splitio/redux-client/issues/113).
- Added `lastUpdate` and `isTimedout` properties to the object returned by the `getStatus` helper and `selectTreatmentAndStatus` and `selectTreatmentWithConfigAndStatus` selectors, to expose the last event timestamp and the timedout status of the SDK clients (Related to https://github.com/splitio/redux-client/issues/113).
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@splitsoftware/splitio-redux",
"version": "1.14.0",
"version": "1.14.1",
"description": "A library to easily use Split JS SDK with Redux and React Redux",
"main": "lib/index.js",
"module": "es/index.js",
Expand Down
15 changes: 10 additions & 5 deletions src/__tests__/asyncActions.browser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,8 @@ describe('getTreatments', () => {
payload: {
key: sdkBrowserConfig.core.key,
treatments: expect.any(Object),
timestamp: expect.any(Number)
timestamp: expect.any(Number),
nonDefaultKey: false
}
});

Expand Down Expand Up @@ -364,7 +365,8 @@ describe('getTreatments', () => {
payload: {
key: sdkBrowserConfig.core.key,
treatments: expect.any(Object),
timestamp: expect.any(Number)
timestamp: expect.any(Number),
nonDefaultKey: false
}
});

Expand Down Expand Up @@ -438,7 +440,8 @@ describe('getTreatments', () => {
payload: {
key: sdkBrowserConfig.core.key,
treatments: expect.any(Object),
timestamp: expect.any(Number)
timestamp: expect.any(Number),
nonDefaultKey: false
}
});

Expand All @@ -459,7 +462,8 @@ describe('getTreatments', () => {
payload: {
key: sdkBrowserConfig.core.key,
treatments: expect.any(Object),
timestamp: expect.any(Number)
timestamp: expect.any(Number),
nonDefaultKey: false
}
});

Expand All @@ -478,7 +482,8 @@ describe('getTreatments', () => {
payload: {
key: sdkBrowserConfig.core.key,
treatments: expect.any(Object),
timestamp: expect.any(Number)
timestamp: expect.any(Number),
nonDefaultKey: false
}
});

Expand Down
16 changes: 12 additions & 4 deletions src/__tests__/reducer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ describe('Split reducer', () => {
const initialTreatments = initialState.treatments;

// default key
const action = actionCreator(key, treatments, 1000);
const action = actionCreator(key, treatments, 1000, false);
expect(splitReducer(initialState, action)).toEqual({
...initialState,
isReady,
Expand Down Expand Up @@ -231,7 +231,7 @@ describe('Split reducer', () => {
const newTreatments: SplitIO.TreatmentsWithConfig = {
test_split: { ...previousTreatment },
};
const action = actionCreator(key, newTreatments, 1000);
const action = actionCreator(key, newTreatments, 1000, false);
const reduxState = splitReducer(stateWithTreatments, action);

// control assertion - treatment object was not replaced in the state
Expand Down Expand Up @@ -262,7 +262,7 @@ describe('Split reducer', () => {
config: previousTreatment.config,
},
};
const action = actionCreator(key, newTreatments, 1000);
const action = actionCreator(key, newTreatments, 1000, false);
const reduxState = splitReducer(stateWithTreatments, action);

// control assertion - treatment object was replaced in the state
Expand Down Expand Up @@ -293,7 +293,7 @@ describe('Split reducer', () => {
},
};
// const action = addTreatments(key, newTreatments);
const action = actionCreator(key, newTreatments, 1000);
const action = actionCreator(key, newTreatments, 1000, false);
const reduxState = splitReducer(stateWithTreatments, action);

// control assertion - treatment object was replaced in the state
Expand All @@ -314,4 +314,12 @@ describe('Split reducer', () => {
});
});

it('should ignore other actions', () => {
expect(splitReducer(initialState, { type: 'OTHER_ACTION' })).toBe(initialState);
expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: null })).toBe(initialState);
expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: undefined })).toBe(initialState);
expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: {} })).toBe(initialState);
expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: true })).toBe(initialState);
});

});
6 changes: 3 additions & 3 deletions src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function splitReady(timestamp: number, key?: SplitIO.SplitKey) {
};
}

export function splitReadyWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) {
export function splitReadyWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) {
return {
type: SPLIT_READY_WITH_EVALUATIONS,
payload: {
Expand All @@ -40,7 +40,7 @@ export function splitReadyFromCache(timestamp: number, key?: SplitIO.SplitKey) {
};
}

export function splitReadyFromCacheWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) {
export function splitReadyFromCacheWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) {
return {
type: SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS,
payload: {
Expand All @@ -62,7 +62,7 @@ export function splitUpdate(timestamp: number, key?: SplitIO.SplitKey) {
};
}

export function splitUpdateWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) {
export function splitUpdateWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) {
return {
type: SPLIT_UPDATE_WITH_EVALUATIONS,
payload: {
Expand Down
6 changes: 3 additions & 3 deletions src/asyncActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
if (client.evalOnReady.length) {
const treatments = __getTreatments(client, client.evalOnReady);

splitSdk.dispatch(splitReadyWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true));
splitSdk.dispatch(splitReadyWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false));
} else {
splitSdk.dispatch(splitReady(lastUpdate, key));
}
Expand All @@ -255,7 +255,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
if (client.evalOnReadyFromCache.length) {
const treatments = __getTreatments(client, client.evalOnReadyFromCache);

splitSdk.dispatch(splitReadyFromCacheWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true));
splitSdk.dispatch(splitReadyFromCacheWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false));
} else {
splitSdk.dispatch(splitReadyFromCache(lastUpdate, key));
}
Expand All @@ -270,7 +270,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
if (evalOnUpdate.length) {
const treatments = __getTreatments(client, evalOnUpdate);

splitSdk.dispatch(splitUpdateWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true));
splitSdk.dispatch(splitUpdateWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false));
} else {
splitSdk.dispatch(splitUpdate(lastUpdate, key));
}
Expand Down
66 changes: 33 additions & 33 deletions src/reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,65 +22,66 @@ const initialState: ISplitState = {
treatments: {},
};

function setStatus(state: ISplitState, patch: Partial<IStatus>, key?: string) {
return key ? {
function setStatus(state: ISplitState, patch: Partial<IStatus>, action: ISplitAction) {
const { timestamp, key, nonDefaultKey } = action.payload;

return nonDefaultKey || (nonDefaultKey === undefined && key) ? {
...state,
status: {
...state.status,
[key]: state.status && state.status[key] ? {
...state.status[key],
...patch,
lastUpdate: timestamp,
} : {
...initialStatus,
...patch,
lastUpdate: timestamp,
}
},
} : {
...state,
...patch,
lastUpdate: timestamp,
};
}

function setReady(state: ISplitState, timestamp: number, key?: string) {
function setReady(state: ISplitState, action: ISplitAction) {
return setStatus(state, {
isReady: true,
isTimedout: false,
lastUpdate: timestamp,
}, key);
}, action);
}

function setReadyFromCache(state: ISplitState, timestamp: number, key?: string) {
function setReadyFromCache(state: ISplitState, action: ISplitAction) {
return setStatus(state, {
isReadyFromCache: true,
lastUpdate: timestamp,
}, key);
}, action);
}

function setTimedout(state: ISplitState, timestamp: number, key?: string) {
function setTimedout(state: ISplitState, action: ISplitAction) {
return setStatus(state, {
isTimedout: true,
hasTimedout: true,
lastUpdate: timestamp,
}, key);
}, action);
}

function setUpdated(state: ISplitState, timestamp: number, key?: string) {
return setStatus(state, {
lastUpdate: timestamp,
}, key);
function setUpdated(state: ISplitState, action: ISplitAction) {
return setStatus(state, {}, action);
}

function setDestroyed(state: ISplitState, timestamp: number, key?: string) {
function setDestroyed(state: ISplitState, action: ISplitAction) {
return setStatus(state, {
isDestroyed: true,
lastUpdate: timestamp,
}, key);
}, action);
}

/**
* Copy the given `treatments` for the given `key` to a `result` Split's slice of state. Returns the `result` object.
*/
function assignTreatments(result: ISplitState, key: string, treatments: SplitIO.TreatmentsWithConfig): ISplitState {
function assignTreatments(result: ISplitState, action: ISplitAction): ISplitState {
const { key, treatments } = action.payload;

result.treatments = { ...result.treatments };
Object.entries<SplitIO.TreatmentWithConfig>(treatments).forEach(([featureFlagName, treatment]) => {
if (result.treatments[featureFlagName]) {
Expand All @@ -106,42 +107,41 @@ export const splitReducer: Reducer<ISplitState> = function (
state = initialState,
action,
) {
const { type, payload: { timestamp, key, treatments, nonDefaultKey } = {} } = action as ISplitAction;

switch (type) {
switch (action.type) {
case SPLIT_READY:
return setReady(state, timestamp, key);
return setReady(state, action as ISplitAction);

case SPLIT_READY_FROM_CACHE:
return setReadyFromCache(state, timestamp, key);
return setReadyFromCache(state, action as ISplitAction);

case SPLIT_TIMEDOUT:
return setTimedout(state, timestamp, key);
return setTimedout(state, action as ISplitAction);

case SPLIT_UPDATE:
return setUpdated(state, timestamp, key);
return setUpdated(state, action as ISplitAction);

case SPLIT_DESTROY:
return setDestroyed(state, timestamp, key);
return setDestroyed(state, action as ISplitAction);

case ADD_TREATMENTS: {
const result = { ...state };
return assignTreatments(result, key, treatments);
return assignTreatments(result, action as ISplitAction);
}

case SPLIT_READY_WITH_EVALUATIONS: {
const result = setReady(state, timestamp, nonDefaultKey && key);
return assignTreatments(result, key, treatments);
const result = setReady(state, action as ISplitAction);
return assignTreatments(result, action as ISplitAction);
}

case SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS: {
const result = setReadyFromCache(state, timestamp, nonDefaultKey && key);
return assignTreatments(result, key, treatments);
const result = setReadyFromCache(state, action as ISplitAction);
return assignTreatments(result, action as ISplitAction);
}

case SPLIT_UPDATE_WITH_EVALUATIONS: {
const result = setUpdated(state, timestamp, nonDefaultKey && key);
return assignTreatments(result, key, treatments);
const result = setUpdated(state, action as ISplitAction);
return assignTreatments(result, action as ISplitAction);
}

default:
Expand Down
2 changes: 1 addition & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { IGetTreatmentsParams } from './types';
* Validates if a given value is a plain object
*/
export function isObject(obj: unknown) {
return obj && typeof obj === 'object' && obj.constructor === Object;
return obj !== null && typeof obj === 'object' && (obj.constructor === Object || (obj.constructor != null && obj.constructor.name === 'Object'));
}

/**
Expand Down
Loading