Repository navigation
Conversation
SaSteffen
marked this pull request as draft
October 9, 2026 09:06
SaSteffen
force-pushed
the
fix/vue-model-api-deleted-reference-targets
branch
2 times, most recently
from
October 9, 2026 10:45
4111fbf to
d2c85bc
Compare
SaSteffen
force-pushed
the
fix/vue-model-api-deleted-reference-targets
branch
from
October 9, 2026 11:21
d2c85bc to
61c3460
Compare
…d nodes A removed node is reported to JS change handlers only as a change to the children of its parent. A handler that keeps state per node, like a cache, cannot tell which nodes are gone. Handlers can now opt in to also get every node of an added or removed subtree. Design decision: this is opt-in, so existing handlers keep their event count. The changes for added and removed nodes are only created when a handler opted in. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…d nodes Removing a node is reported only as a change to the children of its parent, so a reference to the removed node, or to one of its descendants, keeps its cached target. A view that shows such a reference keeps showing the removed node and does not update. With the new option `updateReferencesToRemovedNodes`, `getReferenceTargetNode` reads such a reference as unset. The option is off by default, because it costs work for every added or removed node. When the node is added again with the same reference, for example by an undo, the reference resolves again if it was read while the node existed. Known limit: a reference that is first read, or changed, while the node is removed stays unset after the node is added again, until the reference changes again. Design decision: a removed node is flagged, instead of tracking the references to it. Only nodes that were read as a reference target get the flag, so other nodes cost nothing extra. `useReadonlyVersion` takes no option, because a readonly version never changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
SaSteffen
force-pushed
the
fix/vue-model-api-deleted-reference-targets
branch
from
October 12, 2026 10:20
61c3460 to
5fa05e9
Compare
SaSteffen
marked this pull request as ready for review
October 12, 2026 12:26
This branch has not been deployed
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
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.
Problem
ChildrenChangedfor the parent of a removed node. The references to it do not change, so nothing tellsvue-model-apithat its cached target is gone.Impact
addListenerhandlers get the same events in the same order as before.model-clientandvue-model-apichange. The model server, the JVM client and the MPS plugin are not affected.feat. Two breaks remain for code that implements or matches on the exported types. See the open question below.Changes
model-client: newMutableModelTreeJs.addListenerIncludingAddedAndRemovedNodes. A handler added with it gets all events thataddListenerhandlers get. It also getsNodeAddedandNodeRemoved, for every node of an added or removed subtree. These two come from a separate tree listener. It is only registered for such a handler and is removed with it. Their order relative to the other events is not specified.vue-model-api:useReplicatedModel,useReplicatedModelsanduseModelsFromJsonget an optional last parameter of the new typeReactiveModelOptions. WithupdateReferencesToRemovedNodes: true, the hook builds its nodes from a subclass of the reactive node and uses the new listener.getReferenceTargetNodethen reads a reference to a removed node as unset, the same as after a reload.getReferenceTargetRefstill returns the reference. Without the option, the hook uses the same classes and listener as before.useReadonlyVersiongets no option. Its tree never sends change events.Cost
Without the opt-in:
model-client: the change listener and the per-change path are the same code as on main. One more field per branch,nulluntil a handler opts in. Onenullcheck inremoveListener.vue-model-api: the reactive node, the cache andhandleChangerun the same code as on main. Each hook makes one type check when it registers its handler, not per event. No new fields, objects or checks per node, per read or per event.With the opt-in, per added or removed node:
model-client: the tree changes are diffed once more per opt-in handler, by its own listener. One event per added or removed node, with the same short-lived node wrappers as any other change event.vue-model-api: the node's reference is turned into a string and looked up in the cache, as for any other change event. If the node was read as a reference target, one flag is written. Only the views that read a reference to that node update.shallowReffor each cached node that was read as a reference target. It is freed with the cached node.How it scales with the opt-in:
Known limits
test.failing) describes the wanted behavior.Open question: two breaks for typed consumers
NodeAddedandNodeRemovedare new subtypes of the sealedChangeJS. An exhaustivewhenoverChangeJSin Kotlin code that uses the JS klib no longer compiles.addListenerIncludingAddedAndRemovedNodesis a new abstract member of the exportedMutableModelTreeJs. A TypeScript or Kotlin class that implements it no longer compiles. Code that only calls it is not affected.ChangeJS. They would get their own handler type, for example(node: INodeJS, removed: Boolean) -> Unit, and their own method to register it. The cost is more API, andvue-model-apiwould register two handlers instead of one. The second break stays, because that method is also a new interface member.feat, or do you prefer the alternative?Design decision
ChildrenChangedwas rejected. It would cost time on every edit to children, even edits that neither add nor remove nodes.NodeAddedandNodeRemovedto every handler was rejected. Existing handlers would get one call per node of a removed subtree.Test plan
MutableModelTreeJsTest:addListenerstill gets one event for a removal.removeListenerremoves it.ReactiveINodeJS.test.ts:computed, and undo.useReplicatedModelanduseReplicatedModelsuse onlyaddListenerby default, and only the new listener with the option. Changing the options object after the call does not change the listener used on reconnect.useModelsFromJsonhas no hook test.MutableModelTreeJsTest15 of 15 pass. JestReactiveINodeJS.test.ts,useReplicatedModel.test.tsanduseReplicatedModels.test.ts36 of 36 pass.tsc --noEmitpasses, also withuseDefineForClassFields. pre-commit passes; detekt shows no new findings in the changed files.🤖 Generated with Claude Code