From 028774cb0e334da041974422a3943bc6d9dbd7fd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 20 Sep 2026 01:25:54 +0000 Subject: [PATCH 1/2] fix: honor data prop when spec is supplied in openinula-vchart Merge a valid data prop into the supplied spec on create, and call updateFullDataSync when only data changes. Matches react-vchart. Related #4657 Co-authored-by: David --- .../fix-data-prop-with-spec-4657.json | 10 ++++++++++ packages/openinula-vchart/src/VChart.tsx | 2 +- packages/openinula-vchart/src/charts/BaseChart.tsx | 13 ++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json diff --git a/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json b/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json new file mode 100644 index 0000000000..9cb61a6d02 --- /dev/null +++ b/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@visactor/openinula-vchart", + "comment": "fix: honor the data prop when spec is supplied, including data-only updates", + "type": "patch" + } + ], + "packageName": "@visactor/openinula-vchart" +} diff --git a/packages/openinula-vchart/src/VChart.tsx b/packages/openinula-vchart/src/VChart.tsx index 98f24b3493..cee4a88992 100644 --- a/packages/openinula-vchart/src/VChart.tsx +++ b/packages/openinula-vchart/src/VChart.tsx @@ -2,7 +2,7 @@ import { BaseChartProps, createChart } from './charts/BaseChart'; import VChartCore from '@visactor/vchart'; export { VChartCore }; -export type VChartProps = Omit; +export type VChartProps = Omit; export const VChart = createChart('VChart', { vchartConstructor: VChartCore diff --git a/packages/openinula-vchart/src/charts/BaseChart.tsx b/packages/openinula-vchart/src/charts/BaseChart.tsx index 36c57d2fc6..61db08625e 100644 --- a/packages/openinula-vchart/src/charts/BaseChart.tsx +++ b/packages/openinula-vchart/src/charts/BaseChart.tsx @@ -2,7 +2,7 @@ import type { IVChart, IData, IInitOption, ISpec, IVChartConstructor } from '@vi import React, { useState, useEffect, useRef, useImperativeHandle, ReactNode } from 'openinula'; import withContainer, { ContainerProps } from '../containers/withContainer'; import RootChartContext, { ChartContextType } from '../context/chart'; -import { isEqual, isNil, pickWithout } from '@visactor/vutils'; +import { isEqual, isNil, isValid, pickWithout } from '@visactor/vutils'; import { toArray } from '../util'; import { REACT_PRIVATE_PROPS } from '../constants'; import { @@ -137,6 +137,13 @@ const BaseChart: React.FC = React.forwardRef((props, ref) => { if (hasSpec && props.spec) { spec = props.spec; + + if (isValid(props.data)) { + spec = { + ...props.spec, + data: props.data + } as ISpec; + } } else { spec = { ...prevSpec.current, @@ -209,6 +216,10 @@ const BaseChart: React.FC = React.forwardRef((props, ref) => { enableExitAnimation: false }); handleChartRender(); + } else if (eventsBinded.current.data !== props.data) { + chartContext.current.chart.updateFullDataSync(props.data as any); + handleChartRender(); + eventsBinded.current = props; } return; } From 9bfef888fbb32af0a5b9b3398158a4727b280712 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 29 Sep 2026 20:26:18 +0000 Subject: [PATCH 2/2] fix(openinula-vchart): update spec when data ids cannot be matched updateFullDataSync only replaces datasets that already have an id. Data without a matchable id, and removing the data prop, now re-parse the effective spec so the update matches the first render. Co-authored-by: David --- .../fix-data-prop-with-spec-4657.json | 2 +- packages/openinula-vchart/.eslintrc.cjs | 9 ++ .../__tests__/base-chart-data-prop.test.tsx | 135 ++++++++++++++++ packages/openinula-vchart/jest.config.js | 147 ++++++++++++++++++ packages/openinula-vchart/jest.setup.js | 11 ++ packages/openinula-vchart/package.json | 3 +- .../openinula-vchart/src/charts/BaseChart.tsx | 47 +++++- .../openinula-vchart/tsconfig.eslint.json | 2 +- 8 files changed, 348 insertions(+), 8 deletions(-) create mode 100644 packages/openinula-vchart/__tests__/base-chart-data-prop.test.tsx create mode 100644 packages/openinula-vchart/jest.config.js create mode 100644 packages/openinula-vchart/jest.setup.js diff --git a/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json b/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json index 9cb61a6d02..b2a6a78668 100644 --- a/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json +++ b/common/changes/@visactor/openinula-vchart/fix-data-prop-with-spec-4657.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@visactor/openinula-vchart", - "comment": "fix: honor the data prop when spec is supplied, including data-only updates", + "comment": "fix: honor the data prop when spec is supplied, including id-less updates and restoring spec.data when the prop is removed", "type": "patch" } ], diff --git a/packages/openinula-vchart/.eslintrc.cjs b/packages/openinula-vchart/.eslintrc.cjs index 21e89960a8..31dc454074 100644 --- a/packages/openinula-vchart/.eslintrc.cjs +++ b/packages/openinula-vchart/.eslintrc.cjs @@ -9,6 +9,15 @@ module.exports = { }, parserOptions: { tsconfigRootDir: __dirname, project: './tsconfig.eslint.json' }, // ignorePatterns: [], + overrides: [ + { + files: ['jest.config.js', 'jest.setup.js'], + parserOptions: { + ecmaVersion: 2020, + sourceType: 'script' + } + } + ], rules: { "@typescript-eslint/no-unused-vars": "warn", "react/display-name": "off", diff --git a/packages/openinula-vchart/__tests__/base-chart-data-prop.test.tsx b/packages/openinula-vchart/__tests__/base-chart-data-prop.test.tsx new file mode 100644 index 0000000000..abdbc99ca7 --- /dev/null +++ b/packages/openinula-vchart/__tests__/base-chart-data-prop.test.tsx @@ -0,0 +1,135 @@ +import React, { act, render, unmountComponentAtNode } from 'openinula'; +import { createRequire } from 'module'; +import path from 'path'; +import { VChart } from '../src/VChart'; + +const requireFromVChart = createRequire(path.resolve(__dirname, '../../vchart/package.json')); +const Canvas = requireFromVChart('canvas'); + +const chartOptions = { + mode: 'node' as const, + modeParams: Canvas, + animation: false +}; + +type Datum = { x: string; y: number }; +type SeriesData = { latestData?: Datum[] }; +type SeriesLike = { getRawData: () => SeriesData }; +type ChartLike = { + getChart: () => { getAllSeries: () => SeriesLike[] }; + release: () => void; +}; + +const readYValues = (chart: ChartLike) => { + const series = chart.getChart().getAllSeries(); + const latestData = series[0]?.getRawData()?.latestData; + if (!Array.isArray(latestData)) { + throw new Error('bar series did not expose rendered data'); + } + return latestData.map(datum => datum.y); +}; + +describe('openinula VChart data prop', () => { + let container: HTMLDivElement; + let chart: ChartLike | null; + + const setChart = (instance: ChartLike | null) => { + if (instance) { + chart = instance; + } + }; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + chart = null; + }); + + afterEach(() => { + act(() => { + unmountComponentAtNode(container); + }); + container.remove(); + chart = null; + }); + + const renderChart = async (element: React.ReactElement) => { + await act(() => { + render(element, container); + }); + if (!chart) { + throw new Error('VChart did not finish its render/act lifecycle'); + } + return chart; + }; + + it('updates data that has no matchable id', async () => { + const spec = { + type: 'bar' as const, + width: 400, + height: 300, + animation: false, + xField: 'x', + yField: 'y', + data: [{ values: [{ x: 'A', y: 1 }] }] + }; + + await renderChart( + + ); + expect(readYValues(chart as ChartLike)).toEqual([10]); + + await renderChart( + + ); + expect(readYValues(chart as ChartLike)).toEqual([99]); + }); + + it('restores spec.data when the data override is removed', async () => { + const spec = { + type: 'bar' as const, + width: 400, + height: 300, + animation: false, + xField: 'x', + yField: 'y', + data: [{ id: 'id0', values: [{ x: 'A', y: 1 }] }] + }; + const override = [{ id: 'id0', values: [{ x: 'A', y: 10 }] }]; + + await renderChart(); + expect(readYValues(chart as ChartLike)).toEqual([10]); + + await renderChart(); + expect(readYValues(chart as ChartLike)).toEqual([1]); + + act(() => { + unmountComponentAtNode(container); + }); + chart = null; + await renderChart(); + expect(readYValues(chart as ChartLike)).toEqual([1]); + }); + + it('still updates data when every dataset id already exists', async () => { + const spec = { + type: 'bar' as const, + width: 400, + height: 300, + animation: false, + xField: 'x', + yField: 'y', + data: [{ id: 'id0', values: [{ x: 'A', y: 1 }] }] + }; + + await renderChart( + + ); + expect(readYValues(chart as ChartLike)).toEqual([10]); + + await renderChart( + + ); + expect(readYValues(chart as ChartLike)).toEqual([99]); + }); +}); diff --git a/packages/openinula-vchart/jest.config.js b/packages/openinula-vchart/jest.config.js new file mode 100644 index 0000000000..0d83bd0b83 --- /dev/null +++ b/packages/openinula-vchart/jest.config.js @@ -0,0 +1,147 @@ +const fs = require('fs'); +const path = require('path'); + +function escapeRegex(value) { + return value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +const packageRoots = [ + path.resolve(__dirname, 'node_modules'), + path.resolve(__dirname, '../vchart/node_modules'), + path.resolve(__dirname, '../../common/temp/node_modules') +]; + +function getNodeModulePackageJson(packageName) { + const relativePath = path.join(...packageName.split('/'), 'package.json'); + for (let i = 0; i < packageRoots.length; i++) { + const candidate = path.resolve(packageRoots[i], relativePath); + if (fs.existsSync(candidate)) { + return candidate; + } + } + return null; +} + +function mapPackageExportsToCjs(packageName) { + const packageJsonPath = getNodeModulePackageJson(packageName); + if (!packageJsonPath) { + return {}; + } + const packageJson = require(packageJsonPath); + const packageRoot = path.dirname(packageJsonPath); + + return Object.entries(packageJson.exports ?? {}).reduce((mappers, [subpath, target]) => { + if (subpath === '.' || !target || typeof target !== 'object' || !target.require) { + return mappers; + } + + const exportPath = subpath.slice(2); + mappers[`^${escapeRegex(packageName)}\\/${escapeRegex(exportPath)}$`] = path.resolve( + packageRoot, + target.require.replace(/\.js$/, '') + ); + + return mappers; + }, {}); +} + +function resolvePackageFile(packageName, relativeFile) { + const packageJsonPath = getNodeModulePackageJson(packageName); + if (!packageJsonPath) { + return null; + } + const resolved = path.resolve(path.dirname(packageJsonPath), relativeFile); + return fs.existsSync(resolved) || fs.existsSync(`${resolved}.js`) ? resolved : null; +} + +function assignMapper(mappers, pattern, target) { + if (target) { + mappers[pattern] = target; + } +} + +const vrenderPackageExportMappers = { + ...mapPackageExportsToCjs('@visactor/vrender'), + ...mapPackageExportsToCjs('@visactor/vrender-core'), + ...mapPackageExportsToCjs('@visactor/vrender-animate'), + ...mapPackageExportsToCjs('@visactor/vrender-components'), + ...mapPackageExportsToCjs('@visactor/vrender-kits') +}; + +module.exports = { + preset: 'ts-jest', + testEnvironment: 'jsdom', + testRegex: '/__tests__/.*\\.test\\.(js|ts|tsx)$', + setupFiles: ['./jest.setup.js'], + testTimeout: 60000, + globals: { + 'ts-jest': { + diagnostics: false, + isolatedModules: true, + tsconfig: { + jsx: 'react', + esModuleInterop: true, + allowJs: true, + target: 'ES2019', + module: 'commonjs', + strict: false, + skipLibCheck: true, + sourceMap: true, + composite: false, + declaration: false, + declarationMap: false, + rootDir: path.resolve(__dirname, '../..') + } + } + }, + moduleNameMapper: (() => { + const mappers = { + '^@visactor/vchart$': path.resolve(__dirname, '../vchart/src/index.ts'), + '^@visactor/vutils-extension$': path.resolve(__dirname, '../vutils-extension/src/index.ts'), + ...vrenderPackageExportMappers + }; + assignMapper(mappers, '^d3-color$', resolvePackageFile('d3-color', 'dist/d3-color.min.js')); + assignMapper(mappers, '^d3-array$', resolvePackageFile('d3-array', 'dist/d3-array.min.js')); + assignMapper(mappers, '^d3-geo$', resolvePackageFile('d3-geo', 'dist/d3-geo.min.js')); + assignMapper(mappers, '^d3-dsv$', resolvePackageFile('d3-dsv', 'dist/d3-dsv.min.js')); + assignMapper(mappers, '^d3-hexbin$', resolvePackageFile('d3-hexbin', 'build/d3-hexbin.min.js')); + assignMapper(mappers, '^d3-hierarchy$', resolvePackageFile('d3-hierarchy', 'dist/d3-hierarchy.min.js')); + assignMapper(mappers, '^@visactor/vrender$', resolvePackageFile('@visactor/vrender', 'cjs/index')); + assignMapper(mappers, '^@visactor/vrender-core$', resolvePackageFile('@visactor/vrender-core', 'cjs/index')); + assignMapper(mappers, '^@visactor/vrender-animate$', resolvePackageFile('@visactor/vrender-animate', 'cjs/index')); + assignMapper( + mappers, + '^@visactor/vrender-components$', + resolvePackageFile('@visactor/vrender-components', 'cjs/index') + ); + assignMapper(mappers, '^@visactor/vrender-kits$', resolvePackageFile('@visactor/vrender-kits', 'cjs/index-node')); + assignMapper( + mappers, + '^@visactor/vrender/(.*)$', + resolvePackageFile('@visactor/vrender', 'cjs') && `${resolvePackageFile('@visactor/vrender', 'cjs')}/$1` + ); + assignMapper( + mappers, + '^@visactor/vrender-core/(.*)$', + resolvePackageFile('@visactor/vrender-core', 'cjs') && `${resolvePackageFile('@visactor/vrender-core', 'cjs')}/$1` + ); + assignMapper( + mappers, + '^@visactor/vrender-animate/(.*)$', + resolvePackageFile('@visactor/vrender-animate', 'cjs') && + `${resolvePackageFile('@visactor/vrender-animate', 'cjs')}/$1` + ); + assignMapper( + mappers, + '^@visactor/vrender-components/(.*)$', + resolvePackageFile('@visactor/vrender-components', 'cjs') && + `${resolvePackageFile('@visactor/vrender-components', 'cjs')}/$1` + ); + assignMapper( + mappers, + '^@visactor/vrender-kits/(.*)$', + resolvePackageFile('@visactor/vrender-kits', 'cjs') && `${resolvePackageFile('@visactor/vrender-kits', 'cjs')}/$1` + ); + return mappers; + })() +}; diff --git a/packages/openinula-vchart/jest.setup.js b/packages/openinula-vchart/jest.setup.js new file mode 100644 index 0000000000..1f151aabda --- /dev/null +++ b/packages/openinula-vchart/jest.setup.js @@ -0,0 +1,11 @@ +global.__DEV__ = true; +global.__VERSION__ = 'test'; + +const originalConsoleError = console.error; +console.error = (...args) => { + const message = args.map(arg => (arg instanceof Error ? arg.stack || arg.message : String(arg))).join(' '); + if (message.includes('HTMLCanvasElement.prototype.getContext')) { + return; + } + originalConsoleError.apply(console, args); +}; diff --git a/packages/openinula-vchart/package.json b/packages/openinula-vchart/package.json index bf6e62fa76..21a8d0544a 100644 --- a/packages/openinula-vchart/package.json +++ b/packages/openinula-vchart/package.json @@ -25,7 +25,8 @@ "scripts": { "compile": "tsc --noEmit", "start": "vite ./demo", - "build": "bundle --clean" + "build": "bundle --clean", + "test": "jest" }, "dependencies": { "@visactor/vchart": "workspace:2.1.7", diff --git a/packages/openinula-vchart/src/charts/BaseChart.tsx b/packages/openinula-vchart/src/charts/BaseChart.tsx index 61db08625e..0db999c8bd 100644 --- a/packages/openinula-vchart/src/charts/BaseChart.tsx +++ b/packages/openinula-vchart/src/charts/BaseChart.tsx @@ -89,6 +89,40 @@ const getComponentId = (child: React.ReactNode, index: number) => { return `${componentName}-${index}`; }; +/** + * `updateFullDataSync` only replaces datasets that already exist under the same id. + * Valid data without a matchable id must go through the spec update path instead. + */ +const canUpdateFullDataByExistingId = (chart: IVChart, data?: IData) => { + if (!isValid(data)) { + return false; + } + + const dataSet = chart.getDataSet(); + if (!dataSet) { + return false; + } + + const list = Array.isArray(data) ? data : [data]; + if (!list.length) { + return false; + } + + for (let i = 0; i < list.length; i++) { + const id = (list[i] as { id?: unknown } | null)?.id; + if ((typeof id !== 'string' && typeof id !== 'number') || id === '' || !dataSet.getDataView(id)) { + return false; + } + } + + return true; +}; + +const SPEC_UPDATE_MORPH = { + morph: false, + enableExitAnimation: false +}; + const parseSpecFromChildren = (props: Props) => { const specFromChildren: Omit = {}; @@ -211,13 +245,16 @@ const BaseChart: React.FC = React.forwardRef((props, ref) => { if (hasSpec) { if (!isEqual(eventsBinded.current.spec, props.spec, { skipFunction: skipFunctionDiff })) { eventsBinded.current = props; - chartContext.current.chart.updateSpecSync(parseSpec(props), undefined, { - morph: false, - enableExitAnimation: false - }); + chartContext.current.chart.updateSpecSync(parseSpec(props), undefined, SPEC_UPDATE_MORPH); handleChartRender(); } else if (eventsBinded.current.data !== props.data) { - chartContext.current.chart.updateFullDataSync(props.data as any); + // Removing the data prop, or passing data with no existing id, must re-parse + // the effective spec so the result matches the first render's data priority. + if (canUpdateFullDataByExistingId(chartContext.current.chart, props.data)) { + chartContext.current.chart.updateFullDataSync(props.data as any); + } else { + chartContext.current.chart.updateSpecSync(parseSpec(props), undefined, SPEC_UPDATE_MORPH); + } handleChartRender(); eventsBinded.current = props; } diff --git a/packages/openinula-vchart/tsconfig.eslint.json b/packages/openinula-vchart/tsconfig.eslint.json index 24d3e50728..06ae518114 100644 --- a/packages/openinula-vchart/tsconfig.eslint.json +++ b/packages/openinula-vchart/tsconfig.eslint.json @@ -6,5 +6,5 @@ "baseUrl": "./", "rootDir": "./" }, - "include": ["src", "demo"] + "include": ["src", "demo", "__tests__"] }