From c0e7077c1d15b38b7a626c12aeb17bacde65248d Mon Sep 17 00:00:00 2001 From: Tanner Linsley Date: Fri, 16 Oct 2026 02:30:32 -0600 Subject: [PATCH] fix(react): isolate custom tooltip target updates --- .changeset/react-tooltip-state.md | 5 + API-FRICTION.md | 9 ++ benchmarks/conformance/previews/manifest.json | 2 +- packages/react-charts/src/tooltip.test.tsx | 124 ++++++++++++++++++ packages/react-charts/src/tooltip.tsx | 80 ++++++----- 5 files changed, 185 insertions(+), 35 deletions(-) create mode 100644 .changeset/react-tooltip-state.md create mode 100644 packages/react-charts/src/tooltip.test.tsx diff --git a/.changeset/react-tooltip-state.md b/.changeset/react-tooltip-state.md new file mode 100644 index 00000000..4dfdb9eb --- /dev/null +++ b/.changeset/react-tooltip-state.md @@ -0,0 +1,5 @@ +--- +'@tanstack/react-charts': patch +--- + +Keep custom tooltip target updates from rerendering the React chart host. diff --git a/API-FRICTION.md b/API-FRICTION.md index 458dedd8..09c20aac 100644 --- a/API-FRICTION.md +++ b/API-FRICTION.md @@ -3821,6 +3821,15 @@ Each entry records: A direct Preact SSR-to-hydration regression additionally checks SVG and path identity, immutable label updates, a single surface, and empty-host teardown. +- Oct16 React tooltip follow-up (#187): custom body targets lived above the + chart host, causing three adapter updates for three keyboard target changes + even though the scene did not redraw. A tooltip-owned state boundary now + retains the chart child between target updates, while parent prop changes + still update it. Strict Mode and ordinary regression tests observe zero + adapter updates and correct body values; the current-main negative control + observes three updates. Existing 25 React lifecycle tests also pass. + Packed and full-workspace validation remain required before publication. + ### F-124 — Name-only inventories masked undocumented contracts - Status: resolved diff --git a/benchmarks/conformance/previews/manifest.json b/benchmarks/conformance/previews/manifest.json index 1a36ca56..184adbcd 100644 --- a/benchmarks/conformance/previews/manifest.json +++ b/benchmarks/conformance/previews/manifest.json @@ -2,7 +2,7 @@ "schemaVersion": 1, "width": 288, "height": 192, - "sourceHash": "408fe8d6fb17b71b48ce505eb93dfe8d34afaf2b99dcd855c8b789d78e7cd486", + "sourceHash": "e62990d1bc2ce6a38a60f900c2f8c32d8048c57ccd11c5be4f8d673ec9fcb9ad", "assets": [ { "id": "01-line-gaps", diff --git a/packages/react-charts/src/tooltip.test.tsx b/packages/react-charts/src/tooltip.test.tsx new file mode 100644 index 00000000..c96c0a7c --- /dev/null +++ b/packages/react-charts/src/tooltip.test.tsx @@ -0,0 +1,124 @@ +import * as React from 'react' +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { describe, expect, it, vi } from 'vitest' +import { defineChart, dot } from '@tanstack/charts' +import { tooltip } from '@tanstack/charts/tooltip' +import { scaleLinear } from 'd3-scale' +import { Chart } from './tooltip' + +const { updates } = vi.hoisted(() => ({ updates: vi.fn() })) +vi.mock('@tanstack/charts/adapter/renderer', async (importOriginal) => { + const actual = + await importOriginal() + return { + ...actual, + createChartRendererAdapter: ( + ...args: Parameters + ) => { + const adapter = actual.createChartRendererAdapter(...args) + const update = adapter.update + adapter.update = (options) => { + updates() + update(options) + } + return adapter + }, + } +}) + +describe('tooltip state ownership', () => { + it.each([false, true])( + 'updates only the body and still accepts parent props, strict=%s', + async (strict) => { + const definition = defineChart({ + marks: [ + dot( + [ + { x: 0, y: 1 }, + { x: 1, y: 2 }, + ], + { x: 'x', y: 'y' }, + ), + ], + scales: { + x: { scale: scaleLinear().domain([0, 1]) }, + y: { scale: scaleLinear().domain([0, 3]) }, + }, + tooltip: { use: tooltip }, + }) + const container = document.createElement('div') + document.body.append(container) + const root = createRoot(container) + const render = async (prefix: string, enabled = true, width = 480) => { + const chart = ( + ( + + {prefix}:{primaryPoint?.datum.y} + + ) + : undefined + } + /> + ) + await act(async () => + root.render( + strict ? {chart} : chart, + ), + ) + } + try { + await render('first') + const svg = container.querySelector('svg') + if (!svg) throw new Error('Missing SVG') + updates.mockClear() + for (const [key, value] of [ + ['focus', '1'], + ['ArrowRight', '2'], + ['ArrowLeft', '1'], + ]) { + await act(async () => + svg.dispatchEvent( + key === 'focus' + ? new FocusEvent('focusin', { bubbles: true }) + : new KeyboardEvent('keydown', { key, bubbles: true }), + ), + ) + expect(container.querySelector('[data-body]')?.textContent).toBe( + 'first:' + value, + ) + } + expect(updates).not.toHaveBeenCalled() + await render('second', true, 520) + expect(updates).toHaveBeenCalled() + expect(container.querySelector('svg')).toBe(svg) + expect(svg.getAttribute('aria-label')).toBe('second') + expect(container.querySelector('[data-body]')?.textContent).toBe( + 'second:1', + ) + await render('disabled', false) + expect(container.querySelector('[data-body]')).toBeNull() + await render('third') + await act(async () => + svg.dispatchEvent( + new KeyboardEvent('keydown', { key: 'ArrowRight', bubbles: true }), + ), + ) + expect(container.querySelector('[data-body]')?.textContent).toBe( + 'third:2', + ) + } finally { + await act(async () => root.unmount()) + expect(container.childNodes).toHaveLength(0) + container.remove() + } + }, + ) +}) diff --git a/packages/react-charts/src/tooltip.tsx b/packages/react-charts/src/tooltip.tsx index 54e50722..e8e82925 100644 --- a/packages/react-charts/src/tooltip.tsx +++ b/packages/react-charts/src/tooltip.tsx @@ -90,15 +90,12 @@ export function Chart< TYValue extends ChartValue = ChartValue, >(props: ChartProps) { const { renderTooltipBody, ...chartProps } = props - const tooltipBody = useTooltipBody(renderTooltipBody) return ( - <> - - {tooltipBody.portal} - + + {(onChange) => ( + + )} + ) } @@ -108,15 +105,15 @@ export function RendererChart< TYValue extends ChartValue = ChartValue, >(props: RendererChartProps) { const { renderTooltipBody, ...chartProps } = props - const tooltipBody = useTooltipBody(renderTooltipBody) return ( - <> - - {tooltipBody.portal} - + + {(onChange) => ( + + )} + ) } @@ -126,29 +123,39 @@ export function CanvasChart< TYValue extends ChartValue = ChartValue, >(props: CanvasChartProps) { const { renderTooltipBody, ...chartProps } = props - const tooltipBody = useTooltipBody(renderTooltipBody) return ( - <> - - {tooltipBody.portal} - + + {(onChange) => ( + + )} + ) } -function useTooltipBody< +function TooltipBody< TDatum, TXValue extends ChartValue, TYValue extends ChartValue, ->( +>({ + renderTooltipBody, + children, +}: { + children: ( + onChange: + | (( + target: ChartTooltipBodyTarget | null, + ) => void) + | undefined, + ) => React.ReactNode renderTooltipBody: | (( context: ChartTooltipBodyRenderContext, ) => React.ReactNode) - | undefined, -) { + | undefined +}) { const [target, setTarget] = React.useState children(notify), [children, notify]) + return ( + <> + {chart} + {renderTooltipBody && target ? createPortal( renderTooltipBody({ primaryPoint: target.primaryPoint, @@ -175,8 +186,9 @@ function useTooltipBody< }), target.element, ) - : null, - } + : null} + + ) } function DefaultTooltipBody({