Skip to content

Commit 08358cb

Browse files
committed
refactor: remove redundant afterSetId from Split and GridBody (#8473)
1 parent 64cdb76 commit 08358cb

5 files changed

Lines changed: 98 additions & 32 deletions

File tree

resources/content/.sync-metadata.json

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"lastSync": "2026-01-09T15:28:13.060Z",
3-
"releasesLastFetched": "2026-01-09T15:28:13.077Z",
2+
"lastSync": "2026-01-09T16:10:24.472Z",
3+
"releasesLastFetched": "2026-01-09T16:10:24.487Z",
44
"pushFailures": [],
55
"issues": {
66
"3789": {
@@ -15316,8 +15316,22 @@
1531615316
"state": "OPEN",
1531715317
"path": "resources/content/issues/issue-8469.md",
1531815318
"closedAt": null,
15319-
"updatedAt": "2026-01-09T15:17:26Z",
15320-
"contentHash": "b21f598236f1c533b0995819bad99dab540c2c531e4951345838962770a67fac"
15319+
"updatedAt": "2026-01-09T16:09:50Z",
15320+
"contentHash": "e776ab3a77bdc4e729d13be854bd31c389a4a11321dbc32e71af45f93b302a31"
15321+
},
15322+
"8470": {
15323+
"state": "CLOSED",
15324+
"path": "resources/content/issues/issue-8470.md",
15325+
"closedAt": "2026-01-09T15:39:30Z",
15326+
"updatedAt": "2026-01-09T15:39:30Z",
15327+
"contentHash": "e2234f16db3b15f3a7e413d5008e01c1a255e9cb06ac0c49466755d47c9e2991"
15328+
},
15329+
"8471": {
15330+
"state": "CLOSED",
15331+
"path": "resources/content/issues/issue-8471.md",
15332+
"closedAt": "2026-01-09T16:09:32Z",
15333+
"updatedAt": "2026-01-09T16:09:32Z",
15334+
"contentHash": "0745fbde29f33df83be23928301a52fa8c9237bf62fa165c3a9ca454f7edbcef"
1532115335
}
1532215336
},
1532315337
"releases": {

resources/content/issues/issue-8469.md

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,18 @@ labels:
77
- ai
88
- refactoring
99
- core
10-
assignees: []
10+
assignees:
11+
- tobiu
1112
createdAt: '2026-01-09T15:17:26Z'
12-
updatedAt: '2026-01-09T15:17:26Z'
13+
updatedAt: '2026-01-09T16:09:50Z'
1314
githubUrl: 'https://github.com/neomjs/neo/issues/8469'
1415
author: tobiu
15-
commentsCount: 0
16+
commentsCount: 1
1617
parentIssue: null
17-
subIssues: []
18-
subIssuesCompleted: 0
19-
subIssuesTotal: 0
18+
subIssues:
19+
- '[x] 8471 Refactor afterSetId in src/button/Base.mjs'
20+
subIssuesCompleted: 1
21+
subIssuesTotal: 1
2022
blockedBy: []
2123
blocking: []
2224
---
@@ -43,10 +45,22 @@ We discovered that `Component.Base#afterSetId` was previously mutating the class
4345
- `src/grid/Body.mjs`
4446

4547

48+
## Comments
49+
50+
### @tobiu - 2026-01-09 16:09
51+
52+
**Input from Gemini 3 Pro:**
53+
54+
> ✦ Completed refactoring of `src/button/Base.mjs` in sub-ticket #8471.
55+
> - Replaced `afterSetId` with `ensureStableIds`.
56+
> - Verified prototype safety with new unit tests.
57+
4658
## Activity Log
4759

4860
- 2026-01-09 @tobiu added the `epic` label
4961
- 2026-01-09 @tobiu added the `ai` label
5062
- 2026-01-09 @tobiu added the `refactoring` label
5163
- 2026-01-09 @tobiu added the `core` label
64+
- 2026-01-09 @tobiu assigned to @tobiu
65+
- 2026-01-09 @tobiu added sub-issue #8471
5266

src/button/Split.mjs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,18 +116,7 @@ class Split extends Button {
116116
super.afterSetDisabled(value, oldValue);
117117
}
118118

119-
/**
120-
* Triggered after the id config got changed
121-
* @param {String} value
122-
* @param {String} oldValue
123-
* @protected
124-
*/
125-
afterSetId(value, oldValue) {
126-
this.vdom.id = value + '__wrapper';
127119

128-
// silent vdom update, the super call will trigger the engine
129-
super.afterSetId(value, oldValue);
130-
}
131120

132121
/**
133122
* Triggered after the hideTriggerButton config got changed

src/grid/Body.mjs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -329,18 +329,7 @@ class GridBody extends Component {
329329
value > 0 && this.updateMountedAndVisibleColumns()
330330
}
331331

332-
/**
333-
* Triggered after the id config got changed
334-
* @param {String} value
335-
* @param {String} oldValue
336-
* @protected
337-
*/
338-
afterSetId(value, oldValue) {
339-
this.vdom.id = value + '__wrapper';
340332

341-
// silent vdom update, the super call will trigger the engine
342-
super.afterSetId(value, oldValue);
343-
}
344333

345334
/**
346335
* Triggered after the isScrolling config got changed
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import {setup} from '../../setup.mjs';
2+
3+
const appName = 'WrapperIdCheck';
4+
5+
setup({
6+
neoConfig: {
7+
allowVdomUpdatesInTests: true,
8+
useDomApiRenderer : true,
9+
workerId: 'main'
10+
},
11+
appConfig: {
12+
name: appName
13+
}
14+
});
15+
16+
import {test, expect} from '@playwright/test';
17+
import Neo from '../../../../src/Neo.mjs';
18+
import * as core from '../../../../src/core/_export.mjs';
19+
import SplitButton from '../../../../src/button/Split.mjs';
20+
import GridBody from '../../../../src/grid/Body.mjs';
21+
22+
test.describe('Wrapper Component IDs', () => {
23+
24+
test('SplitButton should have correct wrapper ID', async () => {
25+
const btn = Neo.create(SplitButton, {
26+
appName,
27+
id: 'my-split'
28+
});
29+
30+
// The root vdom node (the wrapper) should have the wrapper suffix
31+
expect(btn.vdom.id).toBe('my-split__wrapper');
32+
// The getVdomRoot() node (the actual button) should have the component ID
33+
expect(btn.getVdomRoot().id).toBe('my-split');
34+
35+
btn.destroy();
36+
});
37+
38+
test('GridBody should have correct wrapper ID', async () => {
39+
const mockGrid = {
40+
id: 'mock-grid',
41+
on: () => {},
42+
isClass: true,
43+
vdom: {id: 'mock-grid'}
44+
};
45+
46+
const gridBody = Neo.create(GridBody, {
47+
appName,
48+
id: 'my-grid-body',
49+
parentComponent: mockGrid
50+
});
51+
52+
// The root vdom node (the wrapper) should have the wrapper suffix
53+
expect(gridBody.vdom.id).toBe('my-grid-body__wrapper');
54+
// The getVdomRoot() node should have the component ID
55+
expect(gridBody.getVdomRoot().id).toBe('my-grid-body');
56+
57+
// Unregister manually to avoid destroy() complexity with mock parent
58+
Neo.manager.Component.unregister(gridBody);
59+
});
60+
});

0 commit comments

Comments
 (0)