Skip to content

Commit

Permalink
Refactor: don't use an object for params
Browse files Browse the repository at this point in the history
Because the types are different, there's no chance of mixing them up
  • Loading branch information
thomasheartman committed Jan 19, 2024
1 parent 752383b commit b937543
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -63,30 +63,30 @@ describe('Strategy change conflict detection', () => {
};

test('It ignores property order in strategy comparison', () => {
const result = getChangesThatWouldBeOverwritten({
currentStrategyConfig: existingStrategy,
change: change,
});
const result = getChangesThatWouldBeOverwritten(
existingStrategy,
change,
);

expect(result).toBeNull();
});

test('It treats `undefined` or missing segments in old config as equal to `[]` in change', () => {
const resultUndefined = getChangesThatWouldBeOverwritten({
currentStrategyConfig: {
const resultUndefined = getChangesThatWouldBeOverwritten(
{
...existingStrategy,
segments: undefined,
},
change: change,
});
change,
);

expect(resultUndefined).toBeNull();

const { segments, ...withoutSegments } = existingStrategy;
const resultMissing = getChangesThatWouldBeOverwritten({
currentStrategyConfig: withoutSegments,
change: change,
});
const resultMissing = getChangesThatWouldBeOverwritten(
withoutSegments,
change,
);

expect(resultMissing).toBeNull();
});
Expand Down Expand Up @@ -128,15 +128,10 @@ describe('Strategy change conflict detection', () => {
].flatMap((existing) =>
[undefinedVariantsInSnapshot, missingVariantsInSnapshot].map(
(changeValue) =>
getChangesThatWouldBeOverwritten({
currentStrategyConfig: existing,
change: changeValue,
}),
getChangesThatWouldBeOverwritten(existing, changeValue),
),
);

console.log('cases', cases);

expect(cases.every((result) => result === null)).toBeTruthy();
});

Expand Down Expand Up @@ -176,10 +171,7 @@ describe('Strategy change conflict detection', () => {
segments: [3],
};

const result = getChangesThatWouldBeOverwritten({
currentStrategyConfig: withChanges,
change: change,
});
const result = getChangesThatWouldBeOverwritten(withChanges, change);

const { id, name, ...changedProperties } = withChanges;

Expand Down Expand Up @@ -234,10 +226,10 @@ describe('Strategy change conflict detection', () => {
};

expect(
getChangesThatWouldBeOverwritten({
currentStrategyConfig: existingStrategyMod,
change: constraintChange,
}),
getChangesThatWouldBeOverwritten(
existingStrategyMod,
constraintChange,
),
).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@ type DataToOverwrite<Prop extends keyof IFeatureStrategy> = {
};
type ChangesThatWouldBeOverwritten = DataToOverwrite<keyof IFeatureStrategy>[];

export const getChangesThatWouldBeOverwritten = ({
currentStrategyConfig,
change,
}: {
currentStrategyConfig?: IFeatureStrategy;
change: IChangeRequestUpdateStrategy;
}): ChangesThatWouldBeOverwritten | null => {
export const getChangesThatWouldBeOverwritten = (
currentStrategyConfig: IFeatureStrategy | undefined,
change: IChangeRequestUpdateStrategy,
): ChangesThatWouldBeOverwritten | null => {
if (change.payload.snapshot && currentStrategyConfig) {
const hasChanged = (a: unknown, b: unknown) => {
if (typeof a === 'object') {
Expand Down

0 comments on commit b937543

Please sign in to comment.