From 2ad152a63880be78c7f023ba4138c0a3ad747dc6 Mon Sep 17 00:00:00 2001 From: Evgenii Alaev Date: Sat, 13 Jun 2026 01:12:57 +0200 Subject: [PATCH 1/2] fix(highcharts): keep tooltip formatter override stable across re-renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit prepareConfig no longer mutates the caller's libraryConfig: the raw tooltip.formatter is stripped from a clone (preparedHighchartsOptions) instead of from the input object. A custom libraryConfig.tooltip.formatter now survives repeated config builds — e.g. React 18+ StrictMode double-mount, which surfaced this regression after a React 17 -> 19 upgrade. Adds regression tests at the prepareConfig level and at the component level (rendering under ); both assert the formatter survives and libraryConfig is not mutated. --- .../tooltip-formatter.visual.test.tsx | 85 +++++++++++++++++++ .../renderer/helpers/config/config.js | 9 +- 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 src/plugins/highcharts/__tests__/tooltip-formatter.visual.test.tsx diff --git a/src/plugins/highcharts/__tests__/tooltip-formatter.visual.test.tsx b/src/plugins/highcharts/__tests__/tooltip-formatter.visual.test.tsx new file mode 100644 index 00000000..185532c4 --- /dev/null +++ b/src/plugins/highcharts/__tests__/tooltip-formatter.visual.test.tsx @@ -0,0 +1,85 @@ +// Regression tests for the custom tooltip formatter override (libraryConfig.tooltip.formatter). +// +// Background: prepareConfig used to `delete options.highcharts.tooltip.formatter`, mutating the +// caller's libraryConfig. The override then worked only on the FIRST config build; any later +// build reusing the same libraryConfig reference fell back to the default ChartKit tooltip. +// React 18+ StrictMode double-mounts components in dev (mount -> unmount -> remount), which is +// exactly such a "second build with the same config" — so the override silently broke after a +// React 17 -> 19 upgrade. +// +// These run in the browser (visual) project because prepareConfig / HighchartsComponent need `window`. + +import React, {StrictMode} from 'react'; + +import {render} from '../../../../test-utils/utils'; +import {ChartKit} from '../../../components/ChartKit'; +import {settings} from '../../../libs'; +import {HighchartsPlugin} from '../index'; +import {data as lineMock} from '../mocks/line'; +import {prepareConfig} from '../renderer/helpers/config/config'; + +settings.set({lang: 'en'}); + +const CUSTOM_TOOLTIP = 'CUSTOM-TOOLTIP-OVERRIDE'; + +function callTooltipFormatter(config: {tooltip: {formatter: (tooltip: unknown) => string}}) { + const fakeThis = {series: {type: 'line'}}; + const fakeTooltip = { + splitTooltip: false, + chart: {options: {chart: {type: 'line'}}, userOptions: {_getComments: () => []}}, + defaultFormatter: () => ['DEFAULT'], + }; + return config.tooltip.formatter.call(fakeThis, fakeTooltip); +} + +describe('highcharts custom tooltip formatter override', () => { + test('prepareConfig: survives repeated builds and does not mutate libraryConfig', () => { + const libraryConfig = { + chart: {type: 'line'}, + tooltip: {formatter: () => CUSTOM_TOOLTIP}, + }; + + const first = prepareConfig(lineMock.data, {highcharts: libraryConfig}) as ReturnType< + typeof prepareConfig + > & {tooltip: {formatter: (tooltip: unknown) => string}}; + const second = prepareConfig(lineMock.data, {highcharts: libraryConfig}) as typeof first; + + expect(callTooltipFormatter(first)).toContain(CUSTOM_TOOLTIP); + // Regression: the second build with the SAME object used to fall back to the default tooltip. + expect(callTooltipFormatter(second)).toContain(CUSTOM_TOOLTIP); + // The caller's libraryConfig must not be mutated. + expect(typeof libraryConfig.tooltip.formatter).toBe('function'); + }); + + test('component: rendering under StrictMode does not consume tooltip.formatter', async () => { + settings.set({plugins: [HighchartsPlugin]}); + + // The same reference is reused across StrictMode's mount -> unmount -> remount cycle, + // mirroring a module-level libraryConfig shared between renders. + const sharedLibraryConfig = { + chart: {type: 'line'}, + tooltip: {formatter: () => CUSTOM_TOOLTIP}, + }; + const chartData = {data: lineMock.data, libraryConfig: sharedLibraryConfig}; + + await render( + +
+ +
+ , + ); + + // Wait until HighchartsComponent has mounted (getDerivedStateFromProps -> prepareConfig ran). + await vi.waitFor(() => { + expect(document.querySelector('.chartkit-graph')).toBeTruthy(); + }); + + // Root cause: the shared libraryConfig must still carry the formatter after rendering. + expect(typeof sharedLibraryConfig.tooltip.formatter).toBe('function'); + }); +}); diff --git a/src/plugins/highcharts/renderer/helpers/config/config.js b/src/plugins/highcharts/renderer/helpers/config/config.js index da848689..6799c136 100644 --- a/src/plugins/highcharts/renderer/helpers/config/config.js +++ b/src/plugins/highcharts/renderer/helpers/config/config.js @@ -1828,7 +1828,8 @@ export function prepareConfig(data, options, isMobile, holidays) { params.tooltip.formatter = function (tooltip) { return `
${formatter.call(this, tooltip)}
`; }; - delete options.highcharts.tooltip.formatter; + // The raw formatter is stripped from the merge source (preparedHighchartsOptions) below, + // not from options.highcharts, so the caller's libraryConfig is not mutated. } else { params.tooltip.formatter = function (tooltip) { const serieType = @@ -1926,6 +1927,12 @@ export function prepareConfig(data, options, isMobile, holidays) { options.highcharts, ); + // params.tooltip.formatter already holds the wrapped user formatter (see above). + // Drop the raw formatter from this clone so the merge below doesn't override the wrapper. + if (preparedHighchartsOptions.tooltip) { + delete preparedHighchartsOptions.tooltip.formatter; + } + mergeWith(params, getTypeParams(data, options), preparedHighchartsOptions, (a, b) => { if (typeof a === 'function' && typeof b === 'function' && a !== b) { return function (event, ...args) { From 5453e3f12fd40126001362056b97b5c5e58ea572 Mon Sep 17 00:00:00 2001 From: Evgenii Alaev Date: Mon, 15 Jun 2026 11:15:57 +0200 Subject: [PATCH 2/2] test(gravity-charts): skip flaky visual tests until #896 The Base/SplitTooltip visual tests snapshot before @gravity-ui/charts finishes its async render, so they flake in CI (blank-canvas capture). Skip them until the timing race and the withSplitPane reflow issue are fixed. Tracked in #896. --- src/plugins/gravity-charts/__tests__/Base.visual.test.tsx | 4 +++- .../gravity-charts/__tests__/SplitTooltip.visual.test.tsx | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/plugins/gravity-charts/__tests__/Base.visual.test.tsx b/src/plugins/gravity-charts/__tests__/Base.visual.test.tsx index f91637c8..d9b2d4cd 100644 --- a/src/plugins/gravity-charts/__tests__/Base.visual.test.tsx +++ b/src/plugins/gravity-charts/__tests__/Base.visual.test.tsx @@ -13,7 +13,9 @@ describe('GravityCharts base tests', () => { settings.set({plugins: [GravityChartsPlugin]}); }); - test('should render chart with valid data', async () => { + // TODO: flaky — screenshot captured before async chart render. Skipped until fixed. + // https://github.com/gravity-ui/chartkit/issues/896 + test.skip('should render chart with valid data', async () => { const data: ChartData = { series: { data: [ diff --git a/src/plugins/gravity-charts/__tests__/SplitTooltip.visual.test.tsx b/src/plugins/gravity-charts/__tests__/SplitTooltip.visual.test.tsx index c1ed2ea9..3cddb255 100644 --- a/src/plugins/gravity-charts/__tests__/SplitTooltip.visual.test.tsx +++ b/src/plugins/gravity-charts/__tests__/SplitTooltip.visual.test.tsx @@ -25,7 +25,9 @@ const SPLIT_TOOLTIP_DATA: ChartData = { xAxis: {type: 'category', categories: ['A', 'B']}, }; -describe('Split tooltip visual tests', () => { +// TODO: flaky — screenshots captured before async chart render. Skipped until fixed. +// https://github.com/gravity-ui/chartkit/issues/896 +describe.skip('Split tooltip visual tests', () => { beforeAll(() => { settings.set({plugins: [GravityChartsPlugin]}); });