Feat: validate VChart with VRender 1.1.0#4574
Merged
Merged
Conversation
Use the published VRender alpha.2 packages for the VChart validation branch. Keep the minimal app-aware stage path and external ownership fixes in review scope. Add regression coverage for VRender patches and final attrs. Constraint: Published npm alpha is the validation source Constraint: option.stage and option.app remain external-owned Rejected: Local linked VRender build | final evidence must use npm alpha Rejected: Broad env/bootstrap migration | this branch keeps stage changes minimal Confidence: high Scope-risk: moderate Directive: Do not downgrade @visactor/vrender* or bypass VRender state patches Tested: rush install --check-only Tested: rush test --only @visactor/vchart Tested: rush build --to @visactor/vchart Tested: eslint touched VChart files Tested: browser strict smoke for basicBar/basicLine/textHeavy hover/select Not-tested: full miniApp/wx/tt/harmony/lynx app factory migration
Use root app factories for VChart-created stages. Publish ordinary mark state styles as shared VRender state definitions. Keep external stage and app ownership outside VChart release. Constraint: Do not modify VRender or use local VRender2 links Constraint: Keep this as a minimal VChart migration Rejected: Manual app bootstrap | root VRender app creators are public API Rejected: stateProxy normal path | VRender now owns resolvedStatePatch Confidence: high Scope-risk: moderate Directive: Do not use direct addState/removeState for VChart state changes Tested: install --check-only Tested: test --only @visactor/vchart Tested: build --to @visactor/vchart Tested: eslint touched VChart files Not-tested: Browser smoke lacked a local browser driver or harness
Consume published VRender alpha.8 packages for PR validation. Refresh VChart direct deps and Rush lockfile only. Constraint: Use npm registry @visactor/vrender*@1.1.0-alpha.8. Constraint: No local VRender2 link, file, workspace, yalc, or pack. Rejected: Local VRender2 linking | PR must test published packages. Confidence: high Scope-risk: moderate Directive: Keep VRender release validation on npm artifacts. Tested: rush update; rush install --check-only. Tested: rush test --only @visactor/vchart. Tested: rush build --to @visactor/vchart; browser smoke.
xuefei1313
changed the title
Contributor
Author
|
🦞 Aime Bot Review 改动摘要：升级所有直接使用 VRender 的依赖版本到 1.1.0-alpha.8，并刷新 Rush pnpm-lock；引入 stage-app 抽象，通过 VRender App 创建和复用 Stage，区分外部 stage 和内部 stage 的生命周期；将 mark、interaction、brush、sankey 等位置的状态切换统一封装到 addGraphicState/removeGraphicState，适配 VRender 1.1 的 useStates/sharedStateDefinitions；为状态补丁和 stage/app 生命周期补充了单元测试。 代码层面观察与建议：1）VRender App 和 Stage 管理方面，Compiler.initStage 通过 resolveVRenderApp 和 createStageFromApp 支持外部 app 与内部 fallback app 复用，并在 release 中基于 _isExternalStage 决定是否释放 stage，同时释放 releaseVRenderAppRef 以回收默认 app，整体设计清晰，生命周期划分合理；外部 stage 场景下仅移除 rootGroup 而不释放 stage 本身符合期望，请确认外部传入的 stage.defaultLayer 一定存在且 rootGroup 总是挂在 defaultLayer 上，否则 removeChild(rootGroup) 可能无效或抛错。2）共享状态定义与状态补丁方面，BaseMark._applySharedStateDefinitions 将 encoderOfState（排除 group/update）转成 sharedStateDefinitions，并根据 _stateSort 计算 priority，resolver 中直接调用 _runEncoderOfGraphic，配合 _initGraphicState 中移除 stateProxy、改用 useStates，能够让 VRender 负责状态补丁合成；新增的 vchart-event.test 针对柱图、折线图和 customMark 的交互状态验证了 sharedStateDefinitions.resolver 和 resolvedStatePatch 与 attribute 的行为，看起来覆盖了主要路径，请确认在存在多个状态同时生效的场景（例如 hover 加 selected）时，VRender 对 sharedStateDefinitions.priority 的合并顺序与之前手动合并 encoder 的语义保持一致，避免样式覆盖顺序发生变化。3）graphic 状态封装方面，addGraphicState/removeGraphicState 统一通过 graphic.useStates 驱动状态切换，并基于 currentStates 做去重和过滤，能够减少直接操作 addState/removeState 的分支逻辑，Interaction、Brush、SankeySeries 以及各类 element 触发器均已切换到该工具函数，实现上没有发现明显遗漏，语义上也与原有逻辑一致，请确认 runtimeState 与交互状态组合使用时，currentStates 与 runtimeStateCache 的配合在 VRender 1.1 下不会产生意外的状态残留。4）stage 和 app 复用与释放策略方面，stage-app 中通过 defaultVRenderApps 和 refCount 复用默认 app，并在最后一个使用者释放时调用 app.release，配合 getDefaultVRenderApp 的已释放则重新创建逻辑，整体符合预期，vchart.test 中针对外部 stage 复用、外部 app 创建 stage、fallback app 生命周期以及 DOM 渲染路径的用例比较完善，能够较好防止回归。测试与验证：本次改动中新增加和更新的单测（vchart-event.test.ts、vchart.test.ts）覆盖了状态补丁、sharedStateDefinitions 以及 stage/app 生命周期的关键路径，覆盖范围看起来比较合理，Notes 中列出的安装、单测、构建与浏览器冒烟验证步骤建议在后续 Ready 阶段持续执行，以确保在大图集下也没有兼容性回归。合并建议：当前 PR 标题标记为 WIP，且主要目的是验证 VChart 在 VRender 1.1.0-alpha.8 下的完整渲染链路，基于当前改动与用例，新实现整体合理，未发现明显阻塞问题；由于处于 WIP 状态，暂不给出合并建议，待后续标记为 Ready 并完成一轮完整渲染回归后再评估是否可合入。 |
Add deterministic regression coverage for VRender alpha animation behavior. It exercises VChart with ManualTicker instead of browser timing. Constraint: Uses npm VRender through VChart exports. Confidence: high Scope-risk: narrow Tested: eslint touched animation test --quiet Tested: jest manual-ticker.test.ts --runInBand --silent=false Tested: rush test --only @visactor/vchart Tested: rush install --check-only Tested: rush build --to @visactor/vchart
VRender alpha leaves a shadow-root fallback require inside the generated VChart miniapp bundle. The lark and wx second-pass Rollup builds should leave that fallback untouched instead of converting it into an unresolved proxy module. Constraint: Do not patch VRender or generated miniapp bundles. Rejected: Commit regenerated miniapp bundles | generated outputs are not source. Confidence: high Scope-risk: narrow Tested: node common/scripts/install-run-rush.js build --to @visactor/vchart Tested: node common/scripts/install-run-rush.js build --only @visactor/lark-vchart Tested: node common/scripts/install-run-rush.js build --only @visactor/wx-vchart Tested: eslint lark/wx rollup configs --quiet Tested: node common/scripts/install-run-rush.js install --check-only
Consume the npm-published VRender alpha.9 packages and lock the stackCornerRadius repeated legend filtering regression with a manual ticker test. The test interrupts update animations and checks final bar plus clip-path static truth. Constraint: Registry packages only; no local VRender link or pack. Rejected: Browser-only manual repro | timing regressions need deterministic checks. Confidence: high Scope-risk: moderate Directive: Keep this regression on manual ticker timing. Tested: npm test -- --runInBand __tests__/unit/animation/manual-ticker.test.ts Tested: node common/scripts/install-run-rush.js install --check-only Tested: eslint packages/vchart/__tests__/unit/animation/manual-ticker.test.ts --quiet Tested: node common/scripts/install-run-rush.js test --only @visactor/vchart Tested: node common/scripts/install-run-rush.js build --to @visactor/vchart Not-tested: Full browser visual smoke in this turn.
Keep the alpha.15 dependency bump. Add focused coverage for the two active update-animation bugs. Constraint: Manual-ticker expansion still includes unfinished cases. Rejected: Wait for all drafted cases | user asked to push current work. Confidence: medium Scope-risk: moderate Tested: rush install --check-only Tested: targeted manual-ticker Jest for two fixed bugs Tested: update-spec Jest Tested: touched-file eslint Not-tested: Full manual-ticker has known unfinished failing cases.
Use VRender shared state definitions, patch-first constants, and setStates as the state sync boundary. Use teardown-only detach while disposing chart-owned products. Keep ordinary runtime remove paths on default removeChild behavior. Use release detach only during chart teardown and releaseGrammar cleanup. Include the local render performance harness for reproducible VRender/VChart comparisons. Constraint: VChart consumes local VRender2 during animation validation Constraint: Hot paths optimize documented usage, not broad fallback behavior Rejected: Use detach for all removeProduct calls | live remove needs setStage Confidence: medium Scope-risk: moderate Directive: Keep removeChild(..., true) limited to release or bulk teardown Directive: Validate live remove semantics before widening detach usage Tested: jest vchart-event.test.ts vchart.test.ts --runInBand Tested: eslint --quiet touched source/test files Tested: tsc --noEmit --project packages/vchart/tsconfig.json Tested: rush install --check-only Tested: perf compare scatter-10k,multi-chart-grid, 7 samples, 2 warmups Not-tested: Full vchart unit gate is blocked by manual-ticker cases
Marker components were released when their model was cleared. That detached the VRender tree before exit animation sampling. The clear path now runs marker exit animation while the old component remains attached. It tracks only animations created by that exit call and releases the old tree once done. Constraint: Keep VChart on VRender marker animation contracts without modifying VRender2 Rejected: Add per-marker opacity fallback | would mask the lifecycle issue Rejected: Wait for pre-existing marker animations | unrelated to exit cleanup Confidence: high Scope-risk: moderate Directive: Do not release marker components before configured exit animations run Tested: jest manual-ticker --runInBand Tested: tsc --noEmit --project tsconfig.json Tested: eslint --quiet src/component/marker/base-marker.ts Tested: git diff --check Not-tested: Browser runtime visual animation replay
fix(vchart): let marker exits finish fade-out
Current updateSpec flags conflate diffing, lifecycle, and pipeline decisions. Record the effect-scope design before broad implementation. Constraint: Keep public updateSpec behavior compatible during migration Rejected: More only-X-changed branches | they would add coupling Confidence: high Scope-risk: narrow Directive: Preserve old flags and add negative regression tests Tested: Documentation placeholder scan Not-tested: Runtime tests not applicable to docs-only change
Add optional scoped effects on updateSpec results. Merge explicit effects and expose lazy legacy conversion. Constraint: Do not change chart update execution order Rejected: Normalize on every merge | hot-path allocation Confidence: high Scope-risk: narrow Directive: Keep old flags as behavior source until pipeline migration Tested: jest update-result and update-spec Tested: jest manual-ticker Tested: eslint touched files; tsc noEmit Not-tested: rush package test
Use scoped effects to skip render and model dataflow for explicit local-only updates. Legacy flags still drive the existing update behavior when localOnly is not declared. Constraint: Do not change existing component or series compare semantics Rejected: Infer local-only from change=false | unsafe for old callers Confidence: high Scope-risk: narrow Directive: Only return localOnly when the model owns all required local work Tested: jest update-result update-effects update-spec Tested: jest manual-ticker Tested: eslint touched files; tsc noEmit Not-tested: rush package test
Marker array removal now exits before unrelated component and series updates. The update result uses scoped local-only component effects so VRender owns marker exit animation. Constraint: Keep non-marker updateSpec behavior on the legacy flag path Rejected: Let axes update then infer local-only | unchanged axes set reRender Rejected: Mutating local-only predicate | pollutes legacy _updateSpec result contract Confidence: high Scope-risk: moderate Directive: Do not remove the early marker removal path until component diffing is scoped Tested: jest manual-ticker update-spec update-effects --runInBand Tested: jest word-cloud --runInBand Tested: eslint touched files; tsc noEmit; git diff --check Not-tested: rush package test
Title text updates now declare scoped component layout and render effects. BaseChart returns before series, data, and domain work for pure component changes. Constraint: Preserve legacy title visible and orient remake behavior Constraint: Effects predicates must not mutate legacy update results Rejected: Blind component skip | mixed title and series updates would drop work Rejected: Predicate normalization | pollutes exact _updateSpec result contracts Confidence: high Scope-risk: moderate Directive: Gate component-only skips with spec-key diffing Tested: jest update-effects update-spec manual-ticker --runInBand Tested: jest word-cloud --runInBand Tested: eslint touched files; tsc noEmit; git diff --check Not-tested: rush package test
Component-only model updates now reinitialize the target model without chart dataflow. This aligns updateModelSpec with the scoped effects path used by full updateSpec. Constraint: Preserve explicit local-only behavior with no model reInit Rejected: Treat component-only as local-only | target model still needs reInit Confidence: high Scope-risk: narrow Directive: Keep dataflow gated by non-component effects in updateModelSpec Tested: jest update-effects update-spec manual-ticker --runInBand Tested: full package jest --runInBand Tested: eslint touched files; tsc noEmit; git diff --check Not-tested: rush test blocked by active rush start lock
Axis appearance-only changes now declare component layout and render effects. The chart can skip unrelated series, data, and domain work for visual axis edits. Constraint: Axis domain and binding changes must keep the legacy dataflow path Rejected: Mark every axis update component-only | min and max updates broke scale domains Confidence: high Scope-risk: moderate Directive: Add scale-domain keys to the deny path before broadening axis effects Tested: jest update-effects update-spec vchart manual-ticker --runInBand Tested: full package jest --runInBand Tested: eslint touched files; tsc noEmit; git diff --check Not-tested: rush test blocked by active rush start lock
Legend item, title, pager, and background changes now declare component layout and render effects. This keeps appearance-only legend edits from reinitializing series or rerunning chart dataflow. Constraint: Legend data, filtering, binding, visibility, and orient changes keep the legacy path Rejected: Mark every legend update component-only | data and filter changes affect series state Confidence: high Scope-risk: moderate Directive: Do not add cached layout fields here without component reInit coverage Tested: jest update-effects update-spec vchart manual-ticker --runInBand Tested: full package jest --runInBand Tested: eslint touched files; tsc noEmit; git diff --check
Component-only chart updates now reinitialize only the components that declared scoped effects. This refreshes cached component fields such as legend position without rerunning series dataflow. Constraint: Full chart reInit remains required for series, data, scale, and compile changes Rejected: ReInit every model on component-only updates | it replays series labels unnecessarily Confidence: high Scope-risk: moderate Directive: Keep component-only effects limited to specs whose reInit is component-local Tested: jest update-effects update-spec vchart manual-ticker --runInBand Tested: full package jest --runInBand Tested: eslint touched files; tsc noEmit; git diff --check
Component release now clears VRender components before model release nulls shared options. Synchronous cleanup also walks the component tree so forced release detaches exiting children. Constraint: Use VRender public releaseWithExitAnimation through the shared release helper Rejected: Restore marker-specific release bookkeeping | duplicates the new component lifecycle Confidence: high Scope-risk: moderate Directive: Keep exit cleanup in the shared component release path, not marker internals Tested: jest manual-ticker update-effects update-spec vchart --runInBand Tested: eslint touched files; tsc noEmit; git diff --check
VChart now consumes the published VRender alpha.24 packages. This keeps the current branch changes together for remote submission. Constraint: Rush requires one direct VRender alpha version across the workspace. Constraint: Manifests and shrinkwrap point at normal 1.1.0-alpha.24 packages. Rejected: Upgrade only @visactor/vchart | rush update rejected mixed alpha versions. Confidence: high Scope-risk: moderate Tested: node common/scripts/install-run-rush.js check Tested: node common/scripts/install-run-rush.js test --only @visactor/vchart Not-tested: full monorepo build across every package
Hidden marks can be absent from the instantiated mark set when visible is false. Their state, style, and visible updates still fit the existing mark compile-only contract. Compare against the declared series mark map as well as instantiated marks. Constraint: Reuse shared updateSpec effect rules instead of adding local special cases. Rejected: Special-case line point state.filter | leaves the same gap for other hidden marks. Confidence: high Scope-risk: moderate Directive: Classify mark spec updates from declared series marks, not only live graphics. Tested: packages/vchart jest __tests__/unit/core/update-effects.test.ts --runInBand Tested: packages/vchart jest __tests__/unit/animation/manual-ticker.test.ts --runInBand Tested: packages/vchart eslint touched files --quiet Tested: packages/vchart tsc --noEmit --project tsconfig.json Tested: rush test --only @visactor/vchart
VChart uses tsconfig paths to point TypeScript at local node_modules entries. Those aliases are compile-time resolution aids, not valid publication paths. The bundler now keeps node_modules-backed aliases as package imports while still rewriting local source aliases. Rollup's TypeScript pass also disables composite when declaration emit is disabled. Constraint: Rollup TypeScript rejects composite without declarations Constraint: VChart output must resolve dependencies through package names Rejected: Remove VRender paths from tsconfig | still needed for TS resolution Confidence: high Scope-risk: narrow Directive: Do not serialize node_modules tsconfig paths into module output Tested: tools/bundler rushx test Tested: tools/bundler rushx build Tested: packages/vchart rushx build:es and rushx build:cjs Tested: packages/react-vchart rushx build no longer reports TS6304 Not-tested: full rush build --only tag:package
VRender 1.1 alpha removes the old graphic stateProxy surface. It also routes shared browser entries through package export conditions. VChart now consumes the published alpha.26 packages, delegates state patches to VRender shared state definitions, avoids direct animate internals, and keeps the line bundle analysis entry available. Constraint: VRender 1.1.0-alpha.26 is the published package baseline. Constraint: Browser UMD entries need the browser export condition. Constraint: lark/wx/harmony entries must keep multi-env behavior. Rejected: Keep local VRender2 symlinks | requested baseline is alpha.26. Confidence: high Scope-risk: moderate Directive: Do not reintroduce graphic.stateProxy in VChart mark paths. Directive: Do not access target.animates directly from VChart. Tested: packages/vchart tsc --noEmit Tested: focused VChart Jest suites for stage-app, line, mark, animation, morph, and event Tested: rush test --only @visactor/vchart Tested: npm run analyze-line Tested: npm run build:schema and schema JSON parse Not-tested: full repository build across every platform wrapper
Inline updateSpec data fields are now replaced instead of merged. Glyph event targets resolve back to their hosted mark graphic before state lookup. The event regression test now avoids cross-package FederatedEvent construction. Constraint: Keep fixes narrow and avoid hot-path defensive fallbacks. Rejected: Force remake | Hides the stale DataView field root cause. Rejected: Sankey-only patch | Mark identity lookup is the shared boundary. Confidence: high Scope-risk: narrow Tested: eslint touched files Tested: tsc --noEmit --project tsconfig.json Tested: jest vchart-event update-spec-data-fields update-spec sankey-emphasis-state sankey Not-tested: full @visactor/vchart suite before amend
VRender state updates bypass old glyph before-update hooks. VChart now syncs inherited state styles to rendered glyph subgraphics. Pictogram no longer refits the SVG root against filtered bounds after legend interaction. Constraint: Keep the fix VChart-side and avoid VRender runtime changes Rejected: Patch Sankey adjacency only | state already reached the parent glyph Rejected: Refit pictogram from filtered bounds | legend changes visibility, not layout Confidence: high Scope-risk: moderate Directive: Keep glyph subgraphic state propagation centralized in GlyphMark Tested: vchart targeted jest, 4 suites and 4 tests passed Tested: vchart touched eslint passed Tested: vchart-extension pictogram eslint passed Tested: vchart tsc --noEmit passed Tested: git diff --check passed Not-tested: vchart-extension tsc; existing non-pictogram type errors remain
GlyphMark reused one inherited style object for every child graphic. Merging child-specific encoder output could let one child overwrite sibling child attributes. Sankey linkPath exposed this as the back path receiving the front path and drawing the link twice. Constraint: Keep the fix VChart-side; VRender glyph rendering is expected Rejected: Special-case Sankey back path | sharing affects all GlyphMark children Confidence: high Scope-risk: narrow Directive: Do not share mutable attr maps across glyph subgraphics Tested: jest sankey-link-opacity sankey-emphasis sankey tooltip sankey transform Tested: jest update-effects and box-plot transform Tested: packages/vchart touched eslint Tested: packages/vchart tsc --noEmit Tested: git diff --check Not-tested: Browser pixel-sampling of the full user Sankey data
Pictogram legend filtering rerenders the root mark without a layout resize. The previous imperative fit could be overwritten by the root SVG transform. Move the fitted postMatrix into the root mark style and compute it from the SVG viewBox plus current layout rect so filter rerenders and resize use the same source of truth. Constraint: Preserve pictogram resize behavior while avoiding VRender-side changes Rejected: Cache by layoutRect | filtering rerenders marks without resizing Confidence: high Scope-risk: narrow Directive: Do not refit pictogram from runtime root AABBBounds Tested: packages/vchart jest __tests__/unit/extension/pictogram-size.test.ts --runInBand Tested: packages/vchart tsc --noEmit --project tsconfig.json Tested: touched-file eslint for pictogram series and unit test Not-tested: Manual browser interaction smoke
VRender alpha.29 moves poptip runtime installer binding into loadPoptip(). VChart can rely on the existing full-entry registerPoptip path instead of carrying an app-scoped plugin workaround. The dependency set is upgraded consistently across VChart packages, docs, and story-player. Focused regression tests lock the expected full-entry and simple-entry behavior. Constraint: VChart simple entry must not register poptip by default Rejected: VChart-side app installer registry | VRender owns runtime binding Confidence: high Scope-risk: narrow Directive: Do not add VChart-side poptip app installation without VRender review Tested: node common/scripts/install-run-rush.js check Tested: npx eslint src/component/poptip/index.ts poptip tests Tested: npx jest poptip default/simple tests --runInBand Tested: npm run compile Tested: git diff --check Not-tested: Full monorepo test suite
Resolve the develop merge conflict in discrete legend attribute handling. Develop adds function-based maxRow/maxCol evaluation with layout orient. This branch adds legend shape texture defaults before passing attrs to VRender. The resolved file keeps both behaviors. Constraint: Keep legend texture callback defaults for VRender pattern creation. Constraint: Keep develop maxRow/maxCol callback semantics. Rejected: Take either side wholesale | would drop a validated legend behavior. Confidence: high Scope-risk: narrow Tested: jest discrete-legend, pie, pictogram-size --runInBand Tested: eslint touched legend and test files --quiet Tested: tsc --noEmit --project packages/vchart/tsconfig.json Tested: git diff --check
xuefei1313
changed the title
skie1997
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Notes