Skip to content

Commit

Permalink
fix(dashboard): prevent initialState from being shared across dashboa…
Browse files Browse the repository at this point in the history
…rd instances
  • Loading branch information
jmbuss committed Apr 18, 2023
1 parent 0ef2563 commit 5403928
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 3 deletions.
8 changes: 7 additions & 1 deletion packages/dashboard/src/store/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { configureStore } from '@reduxjs/toolkit';
import merge from 'lodash/merge';
import cloneDeep from 'lodash/cloneDeep';
import { initialState } from './state';
import { dashboardReducer } from './reducer';
import type { Store } from 'redux';
Expand All @@ -10,7 +11,12 @@ import type { RecursivePartial, DashboardConfiguration } from '~/types';
export type DashboardStore = Store<DashboardState, DashboardAction>;

export const configureDashboardStore = (preloadedState?: RecursivePartial<DashboardState>) => {
const mergedState = merge(initialState, preloadedState);
/**
* Merge modifies the source object so it must be cloned or initialState
* will be shared between different instances of the dashboard store.
*/
const mergedState = cloneDeep(initialState);
merge(mergedState, preloadedState);

const store = configureStore({
reducer: dashboardReducer,
Expand Down
25 changes: 25 additions & 0 deletions packages/dashboard/src/store/state.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import merge from 'lodash/merge';
import { initialState } from './state';

it('does not modify initialState', () => {
const illegalModification = () => {
merge(initialState, {
dashboardConfiguration: {
widgets: [
{
id: 'test-widget',
type: 'test',
x: 0,
y: 0,
z: 1,
height: 10,
width: 10,
properties: {},
},
],
},
});
};

expect(illegalModification).toThrowError();
});
14 changes: 12 additions & 2 deletions packages/dashboard/src/store/state.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { DashboardWidgetsConfiguration, DashboardWidget } from '~/types';
import { deepFreeze } from '~/util/deepFreeze';

export type DashboardState = {
grid: {
Expand All @@ -15,7 +16,16 @@ export type DashboardState = {
dashboardConfiguration: DashboardWidgetsConfiguration;
};

export const initialState: DashboardState = {
/**
* default state for the dashboard to use.
*
* We want to prevent modification of this object
* since it is exported as a singleton and will be
* used to setup the initial dashboard state between
* different instances of dashboard.
*
*/
export const initialState: DashboardState = deepFreeze({
grid: {
enabled: true,
width: 100,
Expand All @@ -31,4 +41,4 @@ export const initialState: DashboardState = {
viewport: { duration: '5m' },
widgets: [],
},
};
});
29 changes: 29 additions & 0 deletions packages/dashboard/src/util/deepFreeze.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { deepFreeze } from './deepFreeze';

it('prevents extensions of an object', () => {
const obj = deepFreeze({});

const illegalExtension = () => {
/**
* Should prevent extension even if you try to
* circumvent the type restrictions
*/
(obj as any)['test'] = 1; // eslint-disable-line
};

expect(illegalExtension).toThrowError();
});

it('prevents modifications of an object', () => {
const obj = deepFreeze({
nestedObject: {
value: 1,
},
});

const illegalModification = () => {
obj.nestedObject.value = 2;
};

expect(illegalModification).toThrowError();
});
9 changes: 9 additions & 0 deletions packages/dashboard/src/util/deepFreeze.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export const deepFreeze = <T extends object>(obj: T) => {
for (const key in obj) {
const value = obj[key];
if (value && typeof value === 'object' && !Object.isFrozen(value)) {
deepFreeze(value);
}
}
return Object.freeze(obj);
};

0 comments on commit 5403928

Please sign in to comment.