[feat]: Scope vueNodesMap per LogicFlow instance to support nested scenarios#2378
[feat]: Scope vueNodesMap per LogicFlow instance to support nested scenarios#2378DymoneLewis merged 4 commits intodidi:masterfrom
Conversation
|
|
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.
| 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
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
That’s strange
it doesn’t seem to be running. Could you please manually add a commit?
…support nested scenarios
EralChen
force-pushed
the
feat/vue_nodes_scoped
branch
from
bdc5da7 to
d391adf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Thank you for your contribution. I will release a version later, and the version number will be synchronized here after the release is complete.ღ( ´･ᴗ･` ) |
|
@EralChen Good afternoon, friend. I just released version @logicflow/vue-node-registry@1.1.13, which includes the changes you submitted in your recent PR. You can upgrade and try it out. |
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.