Skip to content

Commit b168c9e

Browse files
Merge pull request #2487 from opossum-tool/fix-avoid-state-mutations
fix: remove mutations of the redux state in reducers
2 parents 6a1ab2f + f7396c1 commit b168c9e

File tree

5 files changed

+38
-10
lines changed

5 files changed

+38
-10
lines changed

src/Frontend/state/actions/popup-actions/__tests__/popup-actions.test.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ describe('The actions called from the unsaved popup', () => {
517517
return testStore.getState();
518518
}
519519

520-
it('closesPopup', () => {
520+
it('closes popup', () => {
521521
const state: State = prepareTestState();
522522
expect(getOpenPopup(state)).toBeFalsy();
523523
});
@@ -711,7 +711,7 @@ describe('saveTemporaryDisplayPackageInfoAndNavigateToTargetViewIfSavingIsNotDis
711711
expect(getSelectedView(state)).toBe(View.Attribution);
712712
});
713713

714-
it('closesPopup', () => {
714+
it('closes popup', () => {
715715
const state: State = prepareTestState();
716716
expect(getOpenPopup(state)).toBeFalsy();
717717
});

src/Frontend/state/configure-store.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export function createAppStore() {
1919
// TECH DEBT: we should not be putting sets into the store
2020
// https://redux.js.org/style-guide/#do-not-put-non-serializable-values-in-state-or-actions
2121
serializableCheck: false,
22-
// TECH DEBT: we are mutating the redux state inside reducers
22+
// We set this to false because it significantly reduces performance in development mode.
2323
immutableCheck: false,
2424
}),
2525
});

src/Frontend/state/helpers/action-and-reducer-helpers.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ export function getAttributionIdOfFirstPackageCardInManualPackagePanel(
228228
): string {
229229
let displayedAttributionId = '';
230230
if (attributionIds && attributionIds.length > 0) {
231-
displayedAttributionId = attributionIds.sort(
231+
displayedAttributionId = [...attributionIds].sort(
232232
getAlphabeticalComparerForAttributions(
233233
state.allViews.manualData.attributions,
234234
false,
@@ -242,7 +242,7 @@ export function getAttributionIdOfFirstPackageCardInManualPackagePanel(
242242
getAttributionBreakpointCheckForResourceState(state),
243243
);
244244
if (closestParentAttributionIds.length > 0) {
245-
displayedAttributionId = closestParentAttributionIds.sort(
245+
displayedAttributionId = [...closestParentAttributionIds].sort(
246246
getAlphabeticalComparerForAttributions(
247247
state.allViews.manualData.attributions,
248248
false,

src/Frontend/state/helpers/save-action-helpers.ts

+14-2
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,13 @@ export function createManualAttribution(
5656
[newAttributionId]: [selectedResourceId],
5757
},
5858
resourcesWithAttributedChildren: {
59-
...manualData.resourcesWithAttributedChildren,
59+
paths: [...manualData.resourcesWithAttributedChildren.paths],
60+
pathsToIndices: {
61+
...manualData.resourcesWithAttributedChildren.pathsToIndices,
62+
},
63+
attributedChildren: {
64+
...manualData.resourcesWithAttributedChildren.attributedChildren,
65+
},
6066
},
6167
};
6268

@@ -595,7 +601,13 @@ function getAttributionDataShallowCopy(
595601
resourcesToAttributions: { ...attributionData.resourcesToAttributions },
596602
attributionsToResources: { ...attributionData.attributionsToResources },
597603
resourcesWithAttributedChildren: {
598-
...attributionData.resourcesWithAttributedChildren,
604+
paths: [...attributionData.resourcesWithAttributedChildren.paths],
605+
pathsToIndices: {
606+
...attributionData.resourcesWithAttributedChildren.pathsToIndices,
607+
},
608+
attributedChildren: {
609+
...attributionData.resourcesWithAttributedChildren.attributedChildren,
610+
},
599611
},
600612
};
601613
}

src/Frontend/state/reducers/resource-reducer.ts

+19-3
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,15 @@ export const resourceState = (
643643
new Set(state.auditView.resolvedExternalAttributions);
644644

645645
const resourcesWithAttributedChildren = {
646-
...state.allViews.externalData.resourcesWithAttributedChildren,
646+
paths:
647+
state.allViews.externalData.resourcesWithAttributedChildren.paths,
648+
pathsToIndices:
649+
state.allViews.externalData.resourcesWithAttributedChildren
650+
.pathsToIndices,
651+
attributedChildren: {
652+
...state.allViews.externalData.resourcesWithAttributedChildren
653+
.attributedChildren,
654+
},
647655
};
648656
resolvedExternalAttributionsWithAddedAttribution.add(
649657
resolvedAttributionId,
@@ -699,8 +707,16 @@ export const resourceState = (
699707
resourcesWithAttributedChildren:
700708
addUnresolvedAttributionsToResourcesWithAttributedChildren(
701709
{
702-
...state.allViews.externalData
703-
.resourcesWithAttributedChildren,
710+
paths:
711+
state.allViews.externalData.resourcesWithAttributedChildren
712+
.paths,
713+
pathsToIndices:
714+
state.allViews.externalData.resourcesWithAttributedChildren
715+
.pathsToIndices,
716+
attributedChildren: {
717+
...state.allViews.externalData
718+
.resourcesWithAttributedChildren.attributedChildren,
719+
},
704720
},
705721
state.allViews.externalData.attributionsToResources[
706722
action.payload

0 commit comments

Comments
 (0)