Skip to content

Commit fa61b3b

Browse files
authored
fix: DH-20243: improved notebook settings merging (#2593)
It is expected that `getSettings` substitutes custom settings on top of default settings when the settings are pulled. The problem is this substitution was only happening properly if top level settings were being replaced. This meant that the nested linter settings were not getting properly replaced for `notebookSettings` because `notebookSettings` is never itself `undefined`, only the keys `config` and `isEnabled` are.
1 parent 59c8b25 commit fa61b3b

File tree

3 files changed

+41
-11
lines changed

3 files changed

+41
-11
lines changed

package-lock.json

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/redux/package.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"@deephaven/log": "file:../log",
2828
"@deephaven/plugin": "file:../plugin",
2929
"fast-deep-equal": "^3.1.3",
30+
"lodash.mergewith": "^4.6.2",
3031
"proxy-memoize": "^3.0.0",
3132
"redux-thunk": "2.4.1"
3233
},
@@ -39,5 +40,8 @@
3940
"sideEffects": false,
4041
"publishConfig": {
4142
"access": "public"
43+
},
44+
"devDependencies": {
45+
"@types/lodash.mergewith": "^4.6.9"
4246
}
4347
}

packages/redux/src/selectors.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { UndoPartial } from '@deephaven/utils';
22
import { memoize } from 'proxy-memoize';
3-
import type { RootState, WorkspaceSettings } from './store';
3+
import mergeWith from 'lodash.mergewith';
4+
import type { RootState } from './store';
45

56
const EMPTY_OBJECT = Object.freeze({});
67

@@ -61,17 +62,22 @@ export const getSettings = memoize(
6162
store: State
6263
): UndoPartial<State['workspace']['data']['settings']> => {
6364
const customizedSettings = getWorkspace(store)?.data.settings ?? {};
64-
65-
const settings = { ...getDefaultWorkspaceSettings(store) };
66-
const keys = Object.keys(customizedSettings) as (keyof WorkspaceSettings)[];
67-
for (let i = 0; i < keys.length; i += 1) {
68-
const key = keys[i];
69-
if (customizedSettings[key] !== undefined) {
70-
// @ts-expect-error assign non-undefined customized settings to settings
71-
settings[key] = customizedSettings[key];
65+
const defaultSettings = getDefaultWorkspaceSettings(store);
66+
67+
// Deep merge settings but replace arrays instead of merging them
68+
const newSettings = mergeWith(
69+
{},
70+
defaultSettings,
71+
customizedSettings,
72+
(objValue: unknown, srcValue: unknown) => {
73+
if (Array.isArray(objValue) && Array.isArray(srcValue)) {
74+
return srcValue;
75+
}
76+
return undefined;
7277
}
73-
}
74-
return settings as UndoPartial<State['workspace']['data']['settings']>;
78+
) as UndoPartial<State['workspace']['data']['settings']>;
79+
80+
return newSettings;
7581
}
7682
);
7783

0 commit comments

Comments
 (0)