Best Approach: Keep the logic but ensure it uses the safe instance VDOM.
Wait, the user asked to remove it.
"goal => remove it".
If we remove it, we must ensure the nodes still get the IDs, OR update the code to find them differently (e.g. by reference).
But promiseMessage requires an ID string.
So the nodes must have IDs.
Can we assign them in construct? construct calls super.construct which calls afterSetId.
If we remove afterSetId, they won't get IDs on creation.
Can we use onConstructed?
onConstructed runs after initConfig. This is a safer place for one-time setup.
BUT, id can change at runtime. afterSetId handles runtime changes.
If we remove afterSetId, we break runtime ID changes (rare, but supported).
Maybe we don't need to assign them to the VDOM at all?
If we don't assign id to dolly in VDOM, it won't have an ID in the DOM.
Then promiseMessage(..., {id: id + '__dolly'}) will fail to find the node.
So the nodes must have IDs in the DOM.
User's Goal: "remove it".
This implies there might be a way to avoid manual ID assignment.
Does the framework auto-assign IDs to children? No.
Perhaps the user implies we should move this logic to a better place or rely on something else?
Or maybe Gallery shouldn't be using hardcoded ID lookups?
If I look at onSelectionChange, it does Neo.currentWorker.promiseMessage. This is low-level DOM access.
If I simply remove afterSetId, the Gallery will break.
Therefore, I must replace it with something equivalent but cleaner, OR prove it's not needed.
Hypothesis: The user might be thinking about ensureStableIds covering this, but ensureStableIds only covers the wrapper.
Let's look at createItem. It uses getItemVnodeId.
Decision: I will create a test to see if it breaks.
If I remove it, and the test fails (can't find node), then I know.
Test Plan:
- Create a Gallery.
- Check if
#my-gallery__dolly exists in the VDOM.
- If it exists with
afterSetId and fails without, then removing it is a breaking change unless we fix the dependency.
I suspect the "goal => remove it" implies we should refactor the code that needs these IDs to find the nodes differently, or use vdom.cn[0].cn[0]...?
But promiseMessage crosses threads. It needs an ID.
Maybe we can use vnode finding? VNodeUtil.find(this.vnode, ...)?
If we find the node in VNode tree, we can get its ID.
But if the ID wasn't assigned in VDOM, the DOM node won't have one (or will have an auto-generated one).
Auto-generated IDs are unstable. promiseMessage needs a stable ID to target the exact node.
Conclusion: The nodes need stable IDs.
If we remove afterSetId, we must assign them elsewhere.
updateVdom? onConstructed?
Actually, src/component/Base.mjs construct now clones the VDOM eagerly.
So afterSetId is safe from pollution.
But is it "good code"?
Maybe the user wants us to use onConstructed?
Or maybe createVdomReference?
Let's assume the goal is to remove the manual string concatenation pattern in afterSetId.
I will create the test first to confirm the current behavior.
Context:
src/component/Gallery.mjsimplementsafterSetIdto manually assign IDs to its internal VDOM structure (origin,camera,dolly,view).afterSetId(value, oldValue) { super.afterSetId(value, oldValue); let me = this, origin = me.vdom.cn[0], camera = origin.cn[0], dolly = camera.cn[0], view = dolly.cn[0], prefix = me.id + '__'; camera.id = prefix + 'camera'; dolly .id = prefix + 'dolly'; origin.id = prefix + 'origin'; view .id = prefix + 'view'; me.update() }This manual assignment is a pattern we are moving away from in favor of declarative ID management or relying on the framework's stability mechanisms where possible. However,
Gallery.mjsis a complex component with 3D transforms (dolly,camera) and DOM reading logic that might rely on these specific IDs.Analysis:
onSelectionChangeusesid + '__dolly'to apply transforms and read computed styles.createItemusesgetItemVnodeId(this.id + '__' + id) to assign IDs to items.afterSetId, we must ensure that these nodes still get these IDs, or we must update the logic that consumes them.Proposal: We cannot simply delete it without breaking
onSelectionChangewhich doesNeo.currentWorker.promiseMessage('main', { action: 'updateDom', ... id: id + '__dolly' ... }). The 3D gallery relies on direct DOM manipulation via these specific IDs.Strategy:
Safety Check:
ensureStableIds(inVdomLifecycle) only handles the root and wrapper. It does not handle deep internal nodes likecamera,dolly, etc.Refactoring: We can move this logic to a more appropriate place, or keep it if it's strictly necessary for the component's internal mechanics. However, since
afterSetIdruns beforeinitConfig(due to theconstructsequence), this is another potential prototype pollution vector if not for our recentconstructfix inBase.mjs. With theconstructfix,this.vdomis safe. But is it "clean" to have this here? SinceGalleryhas a fixed VDOM structure defined in_vdom, these IDs are structural.Alternative: We can define these IDs dynamically in
getVdomRoot? No, they are deep children.Best Approach: Keep the logic but ensure it uses the safe instance VDOM. Wait, the user asked to remove it. "goal => remove it". If we remove it, we must ensure the nodes still get the IDs, OR update the code to find them differently (e.g. by reference). But
promiseMessagerequires an ID string. So the nodes must have IDs.Can we assign them in
construct?constructcallssuper.constructwhich callsafterSetId. If we removeafterSetId, they won't get IDs on creation.Can we use
onConstructed?onConstructedruns afterinitConfig. This is a safer place for one-time setup. BUT,idcan change at runtime.afterSetIdhandles runtime changes.If we remove
afterSetId, we break runtime ID changes (rare, but supported).Maybe we don't need to assign them to the VDOM at all? If we don't assign
idtodollyin VDOM, it won't have an ID in the DOM. ThenpromiseMessage(..., {id: id + '__dolly'})will fail to find the node.So the nodes must have IDs in the DOM.
User's Goal: "remove it". This implies there might be a way to avoid manual ID assignment. Does the framework auto-assign IDs to children? No.
Perhaps the user implies we should move this logic to a better place or rely on something else? Or maybe
Galleryshouldn't be using hardcoded ID lookups?If I look at
onSelectionChange, it doesNeo.currentWorker.promiseMessage. This is low-level DOM access.If I simply remove
afterSetId, the Gallery will break. Therefore, I must replace it with something equivalent but cleaner, OR prove it's not needed.Hypothesis: The user might be thinking about
ensureStableIdscovering this, butensureStableIdsonly covers the wrapper.Let's look at
createItem. It usesgetItemVnodeId.Decision: I will create a test to see if it breaks. If I remove it, and the test fails (can't find node), then I know.
Test Plan:
#my-gallery__dollyexists in the VDOM.afterSetIdand fails without, then removing it is a breaking change unless we fix the dependency.I suspect the "goal => remove it" implies we should refactor the code that needs these IDs to find the nodes differently, or use
vdom.cn[0].cn[0]...? ButpromiseMessagecrosses threads. It needs an ID.Maybe we can use
vnodefinding?VNodeUtil.find(this.vnode, ...)? If we find the node in VNode tree, we can get its ID. But if the ID wasn't assigned in VDOM, the DOM node won't have one (or will have an auto-generated one). Auto-generated IDs are unstable.promiseMessageneeds a stable ID to target the exact node.Conclusion: The nodes need stable IDs. If we remove
afterSetId, we must assign them elsewhere.updateVdom?onConstructed?Actually,
src/component/Base.mjsconstructnow clones the VDOM eagerly. SoafterSetIdis safe from pollution. But is it "good code"?Maybe the user wants us to use
onConstructed? Or maybecreateVdomReference?Let's assume the goal is to remove the manual string concatenation pattern in
afterSetId.I will create the test first to confirm the current behavior.