Repository navigation
fix: compare Taro chart specs safely #4723
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| const assert = require('node:assert/strict'); | ||
| const fs = require('node:fs'); | ||
| const path = require('node:path'); | ||
| const typescript = require('typescript'); | ||
|
|
||
| const source = fs.readFileSync(path.join(__dirname, '../src/utils/spec-equal.ts'), 'utf8'); | ||
| const { outputText, diagnostics = [] } = typescript.transpileModule(source, { | ||
| compilerOptions: { | ||
| module: typescript.ModuleKind.CommonJS, | ||
| target: typescript.ScriptTarget.ES2015 | ||
| }, | ||
| reportDiagnostics: true | ||
| }); | ||
| assert.equal(diagnostics.length, 0, 'spec comparator should transpile without diagnostics'); | ||
|
|
||
| const compiledModule = { exports: {} }; | ||
| new Function('exports', 'require', 'module', outputText)(compiledModule.exports, require, compiledModule); | ||
| const { isSpecEqual } = compiledModule.exports; | ||
|
|
||
| const firstFormatter = () => 'first'; | ||
| const secondFormatter = () => 'second'; | ||
| assert.equal(isSpecEqual({ formatter: firstFormatter }, { formatter: secondFormatter }), false); | ||
| assert.equal(isSpecEqual({ formatter: firstFormatter }, { formatter: firstFormatter }), true); | ||
| assert.equal(isSpecEqual({ axis: { visible: true } }, { axis: { visible: true } }), true); | ||
| assert.equal(isSpecEqual({ date: new Date(0) }, { date: new Date(0) }), true); | ||
| assert.equal(isSpecEqual({ date: new Date(0) }, { date: new Date(1) }), false); | ||
|
|
||
| function cyclicSpec(value) { | ||
| const spec = { value }; | ||
| spec.self = spec; | ||
| return spec; | ||
| } | ||
| assert.equal(isSpecEqual(cyclicSpec(1), cyclicSpec(1)), true); | ||
| assert.equal(isSpecEqual(cyclicSpec(1), cyclicSpec(2)), false); | ||
|
|
||
| console.log('Taro VChart spec equality checks passed'); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| export { TTCanvas } from './tt-canvas'; | ||
| export { isSpecEqual } from './spec-equal'; | ||
|
|
||
| export { style_cs_canvas, style_cs_canvas_hidden, style_cs_tooltip_canvas, style_container } from './container-styles'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| function isPlainObject(value: object): boolean { | ||
| const prototype = Object.getPrototypeOf(value); | ||
| return prototype === Object.prototype || prototype === null; | ||
| } | ||
|
|
||
| function equalSpecValue(left: unknown, right: unknown, active: WeakMap<object, object>): boolean { | ||
| if (left === right) return true; | ||
| if (typeof left === 'number' && typeof right === 'number' && Number.isNaN(left) && Number.isNaN(right)) { | ||
| return true; | ||
| } | ||
| if (typeof left !== typeof right || left === null || right === null) return false; | ||
| if (typeof left !== 'object' || typeof right !== 'object') return false; | ||
|
|
||
| if (left instanceof Date || right instanceof Date) { | ||
| if (!(left instanceof Date) || !(right instanceof Date)) return false; | ||
| const leftTime = left.getTime(); | ||
| const rightTime = right.getTime(); | ||
| return leftTime === rightTime || (Number.isNaN(leftTime) && Number.isNaN(rightTime)); | ||
| } | ||
|
|
||
| const leftIsArray = Array.isArray(left); | ||
| if (leftIsArray !== Array.isArray(right)) return false; | ||
| if (!leftIsArray && (!isPlainObject(left) || !isPlainObject(right))) return false; | ||
|
|
||
| const leftObject = left as object; | ||
| const rightObject = right as object; | ||
| const activeRight = active.get(leftObject); | ||
| if (activeRight) return activeRight === rightObject; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Distinguish cyclic data graphs that share a right-hand node
|
||
| active.set(leftObject, rightObject); | ||
|
|
||
| if (leftIsArray) { | ||
| const leftArray = left as unknown[]; | ||
| const rightArray = right as unknown[]; | ||
| if (leftArray.length !== rightArray.length) { | ||
| active.delete(leftObject); | ||
| return false; | ||
| } | ||
| for (let index = leftArray.length - 1; index >= 0; index--) { | ||
| if (!equalSpecValue(leftArray[index], rightArray[index], active)) { | ||
| active.delete(leftObject); | ||
| return false; | ||
| } | ||
| } | ||
| active.delete(leftObject); | ||
| return true; | ||
| } | ||
|
|
||
| const leftRecord = left as Record<string, unknown>; | ||
| const rightRecord = right as Record<string, unknown>; | ||
| const leftKeys = Object.keys(leftRecord); | ||
| const rightKeys = Object.keys(rightRecord); | ||
| if (leftKeys.length !== rightKeys.length) { | ||
| active.delete(leftObject); | ||
| return false; | ||
| } | ||
| for (const key of leftKeys) { | ||
| if ( | ||
| !Object.prototype.hasOwnProperty.call(rightRecord, key) || | ||
| !equalSpecValue(leftRecord[key], rightRecord[key], active) | ||
| ) { | ||
| active.delete(leftObject); | ||
| return false; | ||
| } | ||
| } | ||
| active.delete(leftObject); | ||
| return true; | ||
| } | ||
|
|
||
| /** Compare chart specs without serializing callback values or cyclic data. */ | ||
| export function isSpecEqual(left: unknown, right: unknown): boolean { | ||
| return equalSpecValue(left, right, new WeakMap()); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Avoid treating recreated class-valued data rows as changed specs
With freshly constructed data rows having the same enumerable fields (e.g.
new Row('A', 3)on each render), this line returns false at the first row. VChart's datum shape accepts arbitrary fields, and both wrappers interpret false as a reason to callupdateSpec(..., true). A Node 22 reproduction using this comparator returned unequal on 100/100 equivalent rerenders, whereas the former JSON guard returned equal. Large datasets or dashboards consequently merge/render unchanged charts repeatedly. Compare enumerable fields of supported row objects, or handle this case at the wrapper boundary. This reproduces the comparator/guard, not a full Taro integration test.