Skip to content

Commit

Permalink
fix(settings): fix merge logic for plugins update in settings (#465)
Browse files Browse the repository at this point in the history
  • Loading branch information
ananas7 committed Apr 12, 2024
1 parent b50c8a8 commit 0d99e79
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 2 deletions.
89 changes: 89 additions & 0 deletions src/libs/settings/__tests__/settings-update.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import {ChartKitPlugin} from 'src/types';

import {settings} from '../settings';

const resetSettings = () => settings.set({lang: 'en'});

const getMockedPlugin = (type: string, renderer: string) =>
({
type,
renderer,
}) as unknown as ChartKitPlugin;

// Order test is important because settings module is singleton and we can't delete plugins
describe('libs/settings update plugins', () => {
it('Update plugins when it is empty', () => {
settings.set({
plugins: [getMockedPlugin('highcharts', 'initial'), getMockedPlugin('d3', 'initial')],
});

expect(settings.get('plugins')).toEqual([
{
type: 'highcharts',
renderer: 'initial',
},
{
type: 'd3',
renderer: 'initial',
},
]);
});

it('Update existing plugin d3', () => {
const initial = settings.get('plugins');

expect(initial).toEqual([
{
type: 'highcharts',
renderer: 'initial',
},
{
type: 'd3',
renderer: 'initial',
},
]);

settings.set({
plugins: [getMockedPlugin('d3', 'update')],
});

const result = settings.get('plugins');

expect(result).toEqual([
{
type: 'highcharts',
renderer: 'initial',
},
{
type: 'd3',
renderer: 'update',
},
]);
});

it('Add new plugin', () => {
settings.set({
plugins: [getMockedPlugin('yagr', 'update')],
});

const result = settings.get('plugins');

expect(result).toEqual([
{
type: 'highcharts',
renderer: 'initial',
},
{
type: 'd3',
renderer: 'update',
},
{
type: 'yagr',
renderer: 'update',
},
]);
});

beforeAll(resetSettings);
afterEach(resetSettings);
});
41 changes: 41 additions & 0 deletions src/libs/settings/mergeSettingStrategy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import isObject from 'lodash/isObject';
import mergeWith from 'lodash/mergeWith';

import {ChartKitPlugin} from 'src/types';

// @ts-ignore
export function mergeSettingStrategy(objValue: any, srcValue: any, key: string): any {

Check warning on line 7 in src/libs/settings/mergeSettingStrategy.ts

View workflow job for this annotation

GitHub Actions / Verify Files

Unexpected any. Specify a different type

Check warning on line 7 in src/libs/settings/mergeSettingStrategy.ts

View workflow job for this annotation

GitHub Actions / Verify Files

Unexpected any. Specify a different type

Check warning on line 7 in src/libs/settings/mergeSettingStrategy.ts

View workflow job for this annotation

GitHub Actions / Verify Files

Unexpected any. Specify a different type
if (key === 'plugins') {
const currentPlugins: ChartKitPlugin[] = [...objValue];
const newPlugins: ChartKitPlugin[] = [...srcValue];
// modify existing plugins
let newSettingsPlugins = currentPlugins.map((currentPlugin) => {
const newPluginIndex = newPlugins.findIndex(({type}) => type === currentPlugin.type);

if (newPluginIndex !== -1) {
const newPlugin = newPlugins[newPluginIndex];
newPlugins.splice(newPluginIndex, 1);

return {
type: currentPlugin.type,
renderer: newPlugin.renderer,
};
}

return currentPlugin;
});

// add new plugins if it exist after modified
if (newPlugins.length > 0) {
newSettingsPlugins = [...newSettingsPlugins, ...newPlugins];
}

return newSettingsPlugins;
}

if (isObject(objValue)) {
return mergeWith(objValue, srcValue, mergeSettingStrategy);
}

return srcValue;
}
5 changes: 3 additions & 2 deletions src/libs/settings/settings.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import {configure} from '@gravity-ui/uikit';
import get from 'lodash/get';
import merge from 'lodash/merge';
import mergeWith from 'lodash/mergeWith';

import {i18nFactory} from '../../i18n';
import type {ChartKitHolidays, ChartKitLang, ChartKitPlugin} from '../../types';

import {EventEmitter} from './eventEmitter';
import {mergeSettingStrategy} from './mergeSettingStrategy';

interface Settings {
plugins: ChartKitPlugin[];
Expand Down Expand Up @@ -54,7 +55,7 @@ class ChartKitSettings {
set(updates: Partial<Settings>) {
const filteredUpdates = removeUndefinedValues(updates);

this.settings = merge(this.settings, filteredUpdates);
this.settings = mergeWith(this.settings, filteredUpdates, mergeSettingStrategy);

if (filteredUpdates.lang) {
const lang = filteredUpdates.lang || this.get('lang');
Expand Down

0 comments on commit 0d99e79

Please sign in to comment.