From b8d8e8a618570e3bc53db4412398df104f87fad8 Mon Sep 17 00:00:00 2001 From: Marcus Andersson Date: Wed, 22 Oct 2025 09:26:31 +0200 Subject: [PATCH 1/2] Checking for mutability of panel prior to trying to cleaning it up. --- src/migrations.test.ts | 93 ++++++++++++++++++++++++++++++++++++++++++ src/migrations.ts | 24 +++++++++-- 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/src/migrations.test.ts b/src/migrations.test.ts index c466451e..f5c68280 100644 --- a/src/migrations.test.ts +++ b/src/migrations.test.ts @@ -249,4 +249,97 @@ describe('Clock migrations', () => { expect(options).toMatchSnapshot(); expect(panel).toMatchSnapshot(); }); + + // When we introduced schemas in G12 we also made the panel object immutable + // We can only mutate the panel options object. + describe('support immutability on panel in G12', () => { + it('should not mutate panel with input-only config', () => { + const panel = createImmutablePanel({ + options: { + countdownSettings: { source: 'input' }, + }, + datasource: { type: 'test', uid: '123' }, + targets: [], + } as unknown as PanelModel); + + expect(() => clockMigrationHandler(panel)).not.toThrow(); + }); + + it('should not mutate panel with query-based config', () => { + const panel = createImmutablePanel({ + options: { + countdownSettings: { source: 'query' }, + }, + datasource: { type: 'test', uid: '123' }, + targets: [], + } as unknown as PanelModel); + + expect(() => clockMigrationHandler(panel)).not.toThrow(); + }); + + it('should not mutate panel with legacy config (pre 2.1.4)', () => { + const panel = createImmutablePanel({ + options: {}, + datasource: { type: 'test', uid: '123' }, + targets: [], + clockType: '12 hour', + } as unknown as PanelModel); + + expect(() => clockMigrationHandler(panel)).not.toThrow(); + }); + + it('should not mutate panel with refreshSettings', () => { + const panel = createImmutablePanel({ + options: { + countdownSettings: { source: 'query' }, + }, + refreshSettings: { syncWithDashboard: true }, + datasource: { type: 'test', uid: '123' }, + targets: [], + } as unknown as PanelModel); + + expect(() => clockMigrationHandler(panel)).not.toThrow(); + }); + + it('should not mutate panel with options.refreshSettings', () => { + const panel = createImmutablePanel({ + options: { + countdownSettings: { source: 'query' }, + refreshSettings: { syncWithDashboard: true }, + }, + datasource: { type: 'test', uid: '123' }, + targets: [], + } as unknown as PanelModel); + + expect(() => clockMigrationHandler(panel)).not.toThrow(); + }); + }); }); + +function createImmutablePanel(panel: Partial): PanelModel { + // Create a shallow copy to avoid mutating the original + const immutablePanel = { ...panel }; + + // Deep freeze all properties except options + Object.keys(immutablePanel).forEach((key) => { + if (key !== 'options') { + const value = immutablePanel[key as keyof PanelModel]; + if (value && typeof value === 'object') { + deepFreeze(value); + } + } + }); + + // Freeze the panel itself (options content remains mutable) + return Object.freeze(immutablePanel) as PanelModel; +} + +function deepFreeze(obj: any): any { + Object.freeze(obj); + Object.getOwnPropertyNames(obj).forEach((prop) => { + if (obj[prop] && typeof obj[prop] === 'object' && !Object.isFrozen(obj[prop])) { + deepFreeze(obj[prop]); + } + }); + return obj; +} diff --git a/src/migrations.ts b/src/migrations.ts index 16b010f9..4d895090 100644 --- a/src/migrations.ts +++ b/src/migrations.ts @@ -10,11 +10,13 @@ export const clockMigrationHandler = (panel: PanelModel): Partial< options.refresh = ClockRefresh.dashboard; } - if (detectInputOnlyPluginConfig(panel)) { - migrateInputOnlyPluginConfig(panel); + if (isMutablePanel(panel)) { + if (detectInputOnlyPluginConfig(panel)) { + migrateInputOnlyPluginConfig(panel); + } + // configuration options moved as the panel migrated, clean up if needed + cleanupConfig(panel); } - // configuration options moved as the panel migrated, clean up if needed - cleanupConfig(panel); return options; }; @@ -159,3 +161,17 @@ const cleanupConfig = (panel: PanelModel) => { delete panel.timezoneSettings; } }; + +function isMutablePanel(panel: PanelModel) { + try { + // Try to modify panel to check if it's mutable + + // @ts-ignore + panel.asdf = 'test'; + // @ts-ignore + delete panel.asdf; + return true; + } catch (error) { + return false; + } +} From 8cc306ed1d33d9df06c7ca7e3be31d3558261943 Mon Sep 17 00:00:00 2001 From: Marcus Andersson Date: Thu, 23 Oct 2025 08:52:34 +0200 Subject: [PATCH 2/2] Cleaned up code. --- src/migrations.test.ts | 94 +++++++----------------------------------- src/migrations.ts | 21 +++------- 2 files changed, 21 insertions(+), 94 deletions(-) diff --git a/src/migrations.test.ts b/src/migrations.test.ts index f5c68280..4108752b 100644 --- a/src/migrations.test.ts +++ b/src/migrations.test.ts @@ -1,5 +1,5 @@ import { PanelModel } from '@grafana/data'; - +import { cloneDeep } from 'lodash'; import { clockMigrationHandler } from './migrations'; describe('Clock migrations', () => { @@ -250,11 +250,9 @@ describe('Clock migrations', () => { expect(panel).toMatchSnapshot(); }); - // When we introduced schemas in G12 we also made the panel object immutable - // We can only mutate the panel options object. - describe('support immutability on panel in G12', () => { - it('should not mutate panel with input-only config', () => { - const panel = createImmutablePanel({ + describe('support readonly targets in G12', () => { + it('should not try to mutate targets when migrating panel', () => { + const panel = createPanelWithReadonlyTargets({ options: { countdownSettings: { source: 'input' }, }, @@ -264,82 +262,20 @@ describe('Clock migrations', () => { expect(() => clockMigrationHandler(panel)).not.toThrow(); }); - - it('should not mutate panel with query-based config', () => { - const panel = createImmutablePanel({ - options: { - countdownSettings: { source: 'query' }, - }, - datasource: { type: 'test', uid: '123' }, - targets: [], - } as unknown as PanelModel); - - expect(() => clockMigrationHandler(panel)).not.toThrow(); - }); - - it('should not mutate panel with legacy config (pre 2.1.4)', () => { - const panel = createImmutablePanel({ - options: {}, - datasource: { type: 'test', uid: '123' }, - targets: [], - clockType: '12 hour', - } as unknown as PanelModel); - - expect(() => clockMigrationHandler(panel)).not.toThrow(); - }); - - it('should not mutate panel with refreshSettings', () => { - const panel = createImmutablePanel({ - options: { - countdownSettings: { source: 'query' }, - }, - refreshSettings: { syncWithDashboard: true }, - datasource: { type: 'test', uid: '123' }, - targets: [], - } as unknown as PanelModel); - - expect(() => clockMigrationHandler(panel)).not.toThrow(); - }); - - it('should not mutate panel with options.refreshSettings', () => { - const panel = createImmutablePanel({ - options: { - countdownSettings: { source: 'query' }, - refreshSettings: { syncWithDashboard: true }, - }, - datasource: { type: 'test', uid: '123' }, - targets: [], - } as unknown as PanelModel); - - expect(() => clockMigrationHandler(panel)).not.toThrow(); - }); }); }); -function createImmutablePanel(panel: Partial): PanelModel { - // Create a shallow copy to avoid mutating the original - const immutablePanel = { ...panel }; - - // Deep freeze all properties except options - Object.keys(immutablePanel).forEach((key) => { - if (key !== 'options') { - const value = immutablePanel[key as keyof PanelModel]; - if (value && typeof value === 'object') { - deepFreeze(value); - } - } +function createPanelWithReadonlyTargets(panel: Partial): PanelModel { + // https://github.com/grafana/grafana/blob/2bbba880cd2a8269e262e4ea7138fcd43f4d5c66/public/app/features/dashboard-scene/serialization/angularMigration.ts#L18 + const targetClone = cloneDeep(panel.targets); + Object.defineProperty(panel, 'targets', { + get: function () { + console.warn( + 'Accessing the targets property when migrating a panel plugin is deprecated. Changes to this property will be ignored.' + ); + return targetClone; + }, }); - // Freeze the panel itself (options content remains mutable) - return Object.freeze(immutablePanel) as PanelModel; -} - -function deepFreeze(obj: any): any { - Object.freeze(obj); - Object.getOwnPropertyNames(obj).forEach((prop) => { - if (obj[prop] && typeof obj[prop] === 'object' && !Object.isFrozen(obj[prop])) { - deepFreeze(obj[prop]); - } - }); - return obj; + return panel as unknown as PanelModel; } diff --git a/src/migrations.ts b/src/migrations.ts index 4d895090..8277627e 100644 --- a/src/migrations.ts +++ b/src/migrations.ts @@ -10,13 +10,13 @@ export const clockMigrationHandler = (panel: PanelModel): Partial< options.refresh = ClockRefresh.dashboard; } - if (isMutablePanel(panel)) { + if (!isReadonlyTarget(panel)) { if (detectInputOnlyPluginConfig(panel)) { migrateInputOnlyPluginConfig(panel); } - // configuration options moved as the panel migrated, clean up if needed - cleanupConfig(panel); } + // configuration options moved as the panel migrated, clean up if needed + cleanupConfig(panel); return options; }; @@ -162,16 +162,7 @@ const cleanupConfig = (panel: PanelModel) => { } }; -function isMutablePanel(panel: PanelModel) { - try { - // Try to modify panel to check if it's mutable - - // @ts-ignore - panel.asdf = 'test'; - // @ts-ignore - delete panel.asdf; - return true; - } catch (error) { - return false; - } +function isReadonlyTarget(panel: PanelModel) { + const description = Object.getOwnPropertyDescriptor(panel, 'targets'); + return typeof description?.set === 'undefined' && typeof description?.get === 'function'; }