[feat]: Scope vueNodesMap per LogicFlow instance to support nested scenarios#2378
[feat]: Scope vueNodesMap per LogicFlow instance to support nested scenarios#2378EralChen wants to merge 1 commit intodidi:masterfrom
Conversation
Store Vue node registrations in a `WeakMap<GraphModel, ...>` and add `getVueNodeConfig(type, graphModel)` so node configs are resolved per LogicFlow instance instead of from a shared global map. Updated `VueNodeView` to read configs through the new scoped getter, which prevents cross-instance registration collisions in multi-instance usage. Kept populating the legacy `vueNodesMap` (marked deprecated) for backward compatibility.fix(vue-node-registry): isolate node config per LF instance Store Vue node registrations in a `WeakMap<GraphModel, ...>` and add `getVueNodeConfig(type, graphModel)` so node configs are resolved per LogicFlow instance instead of from a shared global map. Updated `VueNodeView` to read configs through the new scoped getter, which prevents cross-instance registration collisions in multi-instance usage. Kept populating the legacy `vueNodesMap` (marked deprecated) for backward compatibility.
|
|
About #2377 |
There was a problem hiding this comment.
Pull request overview
This PR scopes Vue node registrations to a specific LogicFlow instance to avoid cross-instance collisions (notably in nested / multi-instance scenarios), by introducing a WeakMap<GraphModel, ...> registry and updating VueNodeView to resolve configs through a scoped getter.
Changes:
- Added per-
GraphModelVue node registry stored in a module-levelWeakMapplusgetVueNodeConfig(type, graphModel). - Deprecated (but still populated) the legacy global
vueNodesMapfor backward compatibility. - Updated
VueNodeViewto load the node component viagetVueNodeConfig(...)instead of the global map.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/vue-node-registry/src/view.ts | Switches runtime config lookup to the new per-instance getter. |
| packages/vue-node-registry/src/registry.ts | Introduces WeakMap-scoped registry, adds getter, keeps legacy global map populated. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (root) { | ||
| const { component } = vueNodesMap[model.type] | ||
| const nodeConfig = getVueNodeConfig(model.type, graphModel) | ||
| const { component } = nodeConfig! |
There was a problem hiding this comment.
getVueNodeConfig() can return undefined, but the code uses nodeConfig! and immediately destructures component, which will throw at runtime if the type wasn’t registered for this graphModel (or if callers previously populated the deprecated vueNodesMap directly). Handle the undefined case (early return / warning) and consider falling back to the legacy global map to preserve backward compatibility.
| const { component } = nodeConfig! | |
| if (!nodeConfig) { | |
| // 未找到对应的 Vue 节点配置，避免运行时异常 | |
| if (process.env && process.env.NODE_ENV !== 'production') { | |
| console.warn( | |
| '[VueNodeView] No vue node config registered for type:', | |
| model.type, | |
| ) | |
| } | |
| return | |
| } | |
| const { component } = nodeConfig |
| type: string, | ||
| graphModel: GraphModel, | ||
| ): VueNodeEntry | undefined { | ||
| return vueNodesMaps.get(graphModel)?.[type] |
There was a problem hiding this comment.
getVueNodeConfig() only checks the per-instance WeakMap. Given vueNodesMap is still exported for backward compatibility, consider falling back to vueNodesMap[type] when no scoped entry exists. This avoids breaking consumers that previously relied on the legacy map and also prevents downstream callers from needing non-null assertions.
| return vueNodesMaps.get(graphModel)?.[type] | |
| const scopedMap = vueNodesMaps.get(graphModel) | |
| if (scopedMap && scopedMap[type]) { | |
| return scopedMap[type] | |
| } | |
| // Fallback to the legacy global map for backward compatibility | |
| return vueNodesMap[type] |
| let map = vueNodesMaps.get(lf.graphModel) | ||
| if (!map) { | ||
| map = {} | ||
| vueNodesMaps.set(lf.graphModel, map) | ||
| } | ||
| map[type] = entry |
There was a problem hiding this comment.
Both the per-instance registry (map) and the deprecated global vueNodesMap use plain objects indexed by type. If type is not strictly controlled, keys like __proto__/constructor can lead to prototype pollution. Prefer Map<string, VueNodeEntry> (and store it in the WeakMap), or initialize with Object.create(null) and reject dangerous keys.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Store Vue node registrations in a
WeakMap<GraphModel, ...>and addgetVueNodeConfig(type, graphModel)so node configs are resolved per LogicFlow instance instead of from a shared global map.Updated
VueNodeViewto read configs through the new scoped getter, which prevents cross-instance registration collisions in multi-instance usage. Kept populating the legacyvueNodesMap(marked deprecated) for backward compatibility.fix(vue-node-registry): isolate node config per LF instanceStore Vue node registrations in a
WeakMap<GraphModel, ...>and addgetVueNodeConfig(type, graphModel)so node configs are resolved per LogicFlow instance instead of from a shared global map.Updated
VueNodeViewto read configs through the new scoped getter, which prevents cross-instance registration collisions in multi-instance usage. Kept populating the legacyvueNodesMap(marked deprecated) for backward compatibility.