From e20f424e74843e615fdc08671a343999236ce913 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Wed, 29 Mar 2023 11:36:23 -0300 Subject: [PATCH 1/3] fix: Removes Redux state mutations - iteration 1 --- .../src/components/Chart/chartReducer.ts | 4 ++-- superset-frontend/src/dashboard/util/crossFilters.ts | 5 +++-- .../src/dashboard/util/updateComponentParentsList.js | 12 ++++++------ 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts index 1d1ae60ea614e..d91d77c49ae1b 100644 --- a/superset-frontend/src/components/Chart/chartReducer.ts +++ b/superset-frontend/src/components/Chart/chartReducer.ts @@ -18,6 +18,7 @@ */ /* eslint camelcase: 0 */ import { t } from '@superset-ui/core'; +import { omit } from 'lodash'; import { HYDRATE_DASHBOARD } from 'src/dashboard/actions/hydrate'; import { DatasourcesAction } from 'src/dashboard/actions/datasources'; import { ChartState } from 'src/explore/types'; @@ -180,8 +181,7 @@ export default function chartReducer( /* eslint-disable no-param-reassign */ if (action.type === actions.REMOVE_CHART) { - delete charts[action.key]; - return charts; + return omit(charts, [action.key]); } if (action.type === actions.UPDATE_CHART_ID) { const { newId, key } = action; diff --git a/superset-frontend/src/dashboard/util/crossFilters.ts b/superset-frontend/src/dashboard/util/crossFilters.ts index 862db89798f81..c166c10fbf4c0 100644 --- a/superset-frontend/src/dashboard/util/crossFilters.ts +++ b/superset-frontend/src/dashboard/util/crossFilters.ts @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ - +import { cloneDeep } from 'lodash'; import { Behavior, FeatureFlag, @@ -60,7 +60,8 @@ export const getCrossFiltersConfiguration = ( if (behaviors.includes(Behavior.INTERACTIVE_CHART)) { if (initialConfig[chartId]) { - chartConfiguration[chartId] = initialConfig[chartId]; + // We need to clone to avoid mutating Redux state + chartConfiguration[chartId] = cloneDeep(initialConfig[chartId]); } if (!chartConfiguration[chartId]) { chartConfiguration[chartId] = { diff --git a/superset-frontend/src/dashboard/util/updateComponentParentsList.js b/superset-frontend/src/dashboard/util/updateComponentParentsList.js index 44e6c24a19adb..57602d303d491 100644 --- a/superset-frontend/src/dashboard/util/updateComponentParentsList.js +++ b/superset-frontend/src/dashboard/util/updateComponentParentsList.js @@ -1,5 +1,3 @@ -import { logging } from '@superset-ui/core'; - /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -18,6 +16,8 @@ import { logging } from '@superset-ui/core'; * specific language governing permissions and limitations * under the License. */ +import { logging } from '@superset-ui/core'; + export default function updateComponentParentsList({ currentComponent, layout = {}, @@ -32,11 +32,11 @@ export default function updateComponentParentsList({ if (Array.isArray(currentComponent.children)) { currentComponent.children.forEach(childId => { - const child = layout[childId]; - if (child) { - child.parents = parentsList; // eslint-disable-line no-param-reassign + if (layout[childId]) { + // eslint-disable-next-line no-param-reassign + layout[childId] = { ...layout[childId], parents: parentsList }; updateComponentParentsList({ - currentComponent: child, + currentComponent: layout[childId], layout, }); } else { From 271dbbe90086e95e623b71964a35cbea9d38bfdf Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Wed, 29 Mar 2023 12:04:08 -0300 Subject: [PATCH 2/3] Deep clones layout object --- .../src/dashboard/util/updateComponentParentsList.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/dashboard/util/updateComponentParentsList.js b/superset-frontend/src/dashboard/util/updateComponentParentsList.js index 57602d303d491..aac4a0579020d 100644 --- a/superset-frontend/src/dashboard/util/updateComponentParentsList.js +++ b/superset-frontend/src/dashboard/util/updateComponentParentsList.js @@ -17,6 +17,7 @@ * under the License. */ import { logging } from '@superset-ui/core'; +import { cloneDeep } from 'lodash'; export default function updateComponentParentsList({ currentComponent, @@ -33,10 +34,13 @@ export default function updateComponentParentsList({ if (Array.isArray(currentComponent.children)) { currentComponent.children.forEach(childId => { if (layout[childId]) { + // We need to clone to avoid mutating Redux state + const child = cloneDeep(layout[childId]); + child.parents = parentsList; // eslint-disable-next-line no-param-reassign - layout[childId] = { ...layout[childId], parents: parentsList }; + layout[childId] = child; updateComponentParentsList({ - currentComponent: layout[childId], + currentComponent: child, layout, }); } else { From df9d21ec3a8b5c9c77d5735d010149f2d95ae476 Mon Sep 17 00:00:00 2001 From: "Michael S. molina" Date: Fri, 31 Mar 2023 08:12:21 -0300 Subject: [PATCH 3/3] Uses spread operator --- .../src/dashboard/util/updateComponentParentsList.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/dashboard/util/updateComponentParentsList.js b/superset-frontend/src/dashboard/util/updateComponentParentsList.js index aac4a0579020d..5a3d7d78f655a 100644 --- a/superset-frontend/src/dashboard/util/updateComponentParentsList.js +++ b/superset-frontend/src/dashboard/util/updateComponentParentsList.js @@ -17,7 +17,6 @@ * under the License. */ import { logging } from '@superset-ui/core'; -import { cloneDeep } from 'lodash'; export default function updateComponentParentsList({ currentComponent, @@ -34,13 +33,13 @@ export default function updateComponentParentsList({ if (Array.isArray(currentComponent.children)) { currentComponent.children.forEach(childId => { if (layout[childId]) { - // We need to clone to avoid mutating Redux state - const child = cloneDeep(layout[childId]); - child.parents = parentsList; // eslint-disable-next-line no-param-reassign - layout[childId] = child; + layout[childId] = { + ...layout[childId], + parents: parentsList, + }; updateComponentParentsList({ - currentComponent: child, + currentComponent: layout[childId], layout, }); } else {