Frontmatter
| id | 8476 |
| title | Refactor Gallery to remove manual afterSetId ID assignment |
| state | Closed |
| labels | airefactoringcore |
| assignees | tobiu |
| createdAt | Jan 9, 2026, 6:07 PM |
| updatedAt | Jan 9, 2026, 6:10 PM |
| githubUrl | https://github.com/neomjs/neo/issues/8476 |
| author | tobiu |
| commentsCount | 1 |
| parentIssue | 8469 |
| subIssues | [] |
| subIssuesCompleted | 0 |
| subIssuesTotal | 0 |
| blockedBy | [] |
| blocking | [] |
| closedAt | Jan 9, 2026, 6:10 PM |
Refactor Gallery to remove manual afterSetId ID assignment
tobiu added parent issue #8469 on Jan 9, 2026, 6:07 PM
tobiu assigned to @tobiu on Jan 9, 2026, 6:09 PM

tobiu
Jan 9, 2026, 6:10 PM
Input from Gemini 3 Pro:
✦ I have refactored
src/component/Gallery.mjsto useensureStableIdsinstead ofafterSetId.Changes:
- Replaced
afterSetIdwithensureStableIdsoverride.- This maintains the current behavior (stable IDs on internal nodes) but aligns with the new architectural pattern.
Next Steps (as per latest instruction):
- The ultimate goal is to remove ID assignment completely.
- This requires refactoring
onSelectionChange(which relies onid + '__dolly') andcreateItem(which relies ongetItemVnodeId) to work without hardcoded ID lookups.- I will proceed with this deeper refactoring in the next steps.
Commit: c4c89ce31 (#8476)
tobiu closed this issue on Jan 9, 2026, 6:10 PM
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.