Skip to content

Commit 9ae9216

Browse files
nmergetmfranzke
andauthored
fix: issue with eslint rule for notification (#6173)
* fix: issue with eslint rule for notification * Update .changeset/fix-notification-closeable-eslint.md Co-authored-by: Maximilian Franzke <[email protected]> * fix: issues from PR --------- Co-authored-by: Maximilian Franzke <[email protected]>
1 parent d85bd86 commit 9ae9216

15 files changed

Lines changed: 506 additions & 142 deletions

File tree

.amazonq/rules/eslint-plugin-development.md

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ const iconChild = node.children?.find(
127127
128128
## Rule Implementation Pattern
129129
130+
### Single Component Rule
131+
130132
```typescript
131133
import {
132134
createAngularVisitors,
@@ -160,7 +162,8 @@ export default {
160162
);
161163
context.report({
162164
loc,
163-
messageId: MESSAGE_IDS.YOUR_MESSAGE_ID
165+
messageId: MESSAGE_IDS.YOUR_MESSAGE_ID,
166+
data: { component: node.name } // Use node.name for kebab-case
164167
});
165168
}
166169
};
@@ -182,7 +185,11 @@ export default {
182185
if (value === null || value === "") {
183186
context.report({
184187
node: openingElement,
185-
messageId: MESSAGE_IDS.YOUR_MESSAGE_ID
188+
messageId: MESSAGE_IDS.YOUR_MESSAGE_ID,
189+
data: {
190+
component:
191+
openingElement.name?.name || openingElement.rawName
192+
}
186193
});
187194
}
188195
};
@@ -196,6 +203,64 @@ export default {
196203
};
197204
```
198205
206+
### Multiple Components Rule
207+
208+
**CRITICAL**: When checking multiple components, collect ALL Angular visitors before returning:
209+
210+
```typescript
211+
const COMPONENTS_TO_CHECK = ["DBButton", "DBLink", "DBInput"];
212+
213+
export default {
214+
meta: {
215+
/* ... */
216+
},
217+
create(context: any) {
218+
const angularHandler = (node: any, parserServices: any) => {
219+
const component = COMPONENTS_TO_CHECK.find((comp) =>
220+
isDBComponent(node, comp)
221+
);
222+
if (!component) return;
223+
224+
// Your validation logic here
225+
};
226+
227+
// ❌ WRONG - Returns after first component, others never registered
228+
for (const comp of COMPONENTS_TO_CHECK) {
229+
const angularVisitors = createAngularVisitors(
230+
context,
231+
comp,
232+
angularHandler
233+
);
234+
if (angularVisitors) return angularVisitors; // ❌ Early return!
235+
}
236+
237+
// ✅ CORRECT - Collects all visitors before returning
238+
const angularVisitors: any = {};
239+
for (const comp of COMPONENTS_TO_CHECK) {
240+
const visitors = createAngularVisitors(
241+
context,
242+
comp,
243+
angularHandler
244+
);
245+
if (visitors) {
246+
Object.assign(angularVisitors, visitors);
247+
}
248+
}
249+
if (Object.keys(angularVisitors).length > 0) return angularVisitors;
250+
251+
const checkComponent = (node: any) => {
252+
// React/Vue handler
253+
};
254+
255+
return defineTemplateBodyVisitor(
256+
context,
257+
{ VElement: checkComponent, Element: checkComponent },
258+
{ JSXElement: checkComponent }
259+
);
260+
}
261+
};
262+
```
263+
199264
## Adding New Messages
200265
201266
1. Add message to `MESSAGES` object in `src/shared/constants.ts`
@@ -257,10 +322,13 @@ invalid: [
257322
258323
- Angular boolean attributes return empty string `''` - handle with `attr.value === null || attr.value === ''`
259324
- Use `createAngularVisitors` for Angular support - it handles kebab-case conversion for components starting with `DB`
325+
- **CRITICAL**: When checking multiple components, collect ALL Angular visitors using `Object.assign()` before returning
260326
- Always use `COMPONENTS` constants instead of hardcoded strings
261327
- Always use `MESSAGE_IDS` and `MESSAGES` from constants
262328
- For Angular, use `parserServices.convertNodeSourceSpanToLoc(node.sourceSpan)` for location
329+
- For Angular, use `node.name` in error data to preserve kebab-case (e.g., `db-button`)
263330
- For React/Vue, use `node: openingElement` for location
331+
- For React/Vue, use `openingElement.name?.name || openingElement.rawName` for component name
264332
- Angular template parser uses both `Element` and `Element$1` types
265333
- Vue sometimes uses `Element` as fallback instead of `VElement`
266334
- When traversing parents/children, always check for `JSXElement`, `VElement`, and `Element` types
@@ -335,6 +403,8 @@ Before submitting a new rule:
335403
- [ ] Rule `fixable` uses `as const` assertion (if present)
336404
- [ ] Test uses `languageOptions` configuration (not `parser` property)
337405
- [ ] Angular support via `createAngularVisitors`
406+
- [ ] **If checking multiple components**: Angular visitors collected with `Object.assign()` before returning
407+
- [ ] Angular error data uses `node.name` for kebab-case component names
338408
- [ ] Vue/React support via `defineTemplateBodyVisitor`
339409
- [ ] All attribute checks use `=== null` (not `!value`)
340410
- [ ] Required text attributes check `=== null || === ''`
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@db-ux/core-eslint-plugin": patch
3+
---
4+
5+
fix(`DBNotification`): `close-button-text-required` rule now only requires `closeButtonText` when `closeable` attribute is set.
6+
7+
fix(angular): resolve bug in parsing where checking multiple components would only lint the first component in the array due to an early return. ESLint rules that target multiple components now correctly lint all components in the array instead of stopping after the first match.

.github/copilot-instructions.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,3 +343,9 @@ Or add it to your MCP client config:
343343
### GitHub Actions / Pipelines
344344

345345
- Use `!cancelled()` instead of `always()` for controlling the step execution in GitHub Actions. This ensures that steps are skipped if the workflow run has been cancelled, preventing unnecessary execution and resource usage.
346+
347+
## Additional Resources
348+
349+
### ESLint Plugin
350+
351+
- Use this file as a reference for the custom ESLint plugin used in this repository: `.amazonq/rules/eslint-plugin-development.md`

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/eslint-plugin/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
"test:update": "vitest run --config vitest.config.ts --update"
3535
},
3636
"peerDependencies": {
37-
"eslint": "^9.0.0 || ^10.0.0"
37+
"eslint": ">=9.0.0"
3838
},
3939
"dependencies": {
4040
"@angular-eslint/utils": "21.3.1",

packages/eslint-plugin/src/rules/close-button/close-button-text-required.ts

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,31 @@ export default {
2727
},
2828
create(context: any) {
2929
const angularHandler = (node: any, parserServices: any) => {
30-
const componentName = node.name;
3130
const component = Object.keys(COMPONENTS_WITH_CLOSE_BUTTON).find(
3231
(comp) => isDBComponent(node, comp)
3332
);
3433

3534
if (!component) return;
3635

36+
if (component === 'DBNotification') {
37+
const input = node.inputs?.find(
38+
(i: any) => i.name === 'closeable'
39+
);
40+
// Check for [closeable]="false" - Angular AST structure
41+
if (input) {
42+
const val = input.value;
43+
if (val?.type === 'LiteralPrimitive' && val.value === false)
44+
return;
45+
if (val?.source === 'false') return;
46+
} else {
47+
// Check for plain attribute closeable (no binding)
48+
const attr = node.attributes?.find(
49+
(a: any) => a.name === 'closeable'
50+
);
51+
if (!attr) return;
52+
}
53+
}
54+
3755
const attribute =
3856
COMPONENTS_WITH_CLOSE_BUTTON[
3957
component as keyof typeof COMPONENTS_WITH_CLOSE_BUTTON
@@ -47,19 +65,23 @@ export default {
4765
context.report({
4866
loc,
4967
messageId: MESSAGE_IDS.CLOSE_BUTTON_TEXT_REQUIRED,
50-
data: { component: componentName, attribute }
68+
data: { component: node.name, attribute }
5169
});
5270
}
5371
};
5472

73+
const angularVisitors: any = {};
5574
for (const comp of Object.keys(COMPONENTS_WITH_CLOSE_BUTTON)) {
56-
const angularVisitors = createAngularVisitors(
75+
const visitors = createAngularVisitors(
5776
context,
5877
comp,
5978
angularHandler
6079
);
61-
if (angularVisitors) return angularVisitors;
80+
if (visitors) {
81+
Object.assign(angularVisitors, visitors);
82+
}
6283
}
84+
if (Object.keys(angularVisitors).length > 0) return angularVisitors;
6385

6486
const checkComponent = (node: any) => {
6587
const openingElement = node.openingElement || node;
@@ -69,6 +91,60 @@ export default {
6991

7092
if (!component) return;
7193

94+
if (component === 'DBNotification') {
95+
// React: closeable={false}
96+
const closeableAttr = openingElement.attributes?.find(
97+
(a: any) =>
98+
a.type === 'JSXAttribute' && a.name.name === 'closeable'
99+
);
100+
if (
101+
closeableAttr?.value?.type === 'JSXExpressionContainer' &&
102+
closeableAttr.value.expression?.type === 'Literal' &&
103+
closeableAttr.value.expression.value === false
104+
)
105+
return;
106+
107+
// Vue: key.name can be a VIdentifier object (key.name.name === 'bind')
108+
// or a plain string for non-directive attributes
109+
const isVueCloseableBind = (a: any) => {
110+
const keyName =
111+
typeof a.key?.name === 'string'
112+
? a.key.name
113+
: a.key?.name?.name;
114+
return (
115+
keyName === 'bind' &&
116+
a.key?.argument?.name === 'closeable'
117+
);
118+
};
119+
const isVueCloseableStatic = (a: any) => {
120+
const keyName =
121+
typeof a.key?.name === 'string'
122+
? a.key.name
123+
: a.key?.name?.name;
124+
return keyName === 'closeable';
125+
};
126+
127+
const vueBindAttr =
128+
openingElement.startTag?.attributes?.find(
129+
isVueCloseableBind
130+
);
131+
if (
132+
vueBindAttr &&
133+
(vueBindAttr.value?.value === 'false' ||
134+
vueBindAttr.value?.expression?.value === false)
135+
)
136+
return;
137+
138+
// Only skip if closeable attribute/binding doesn't exist
139+
const hasCloseable =
140+
closeableAttr ||
141+
openingElement.startTag?.attributes?.some(
142+
(a: any) =>
143+
isVueCloseableStatic(a) || isVueCloseableBind(a)
144+
);
145+
if (!hasCloseable) return;
146+
}
147+
72148
const componentName =
73149
openingElement.name?.name || openingElement.rawName;
74150

packages/eslint-plugin/src/rules/content/text-or-children-required.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,18 @@ export default {
5858
}
5959
};
6060

61+
const angularVisitors: any = {};
6162
for (const comp of COMPONENTS_REQUIRING_CONTENT) {
62-
const angularVisitors = createAngularVisitors(
63+
const visitors = createAngularVisitors(
6364
context,
6465
comp,
6566
angularHandler
6667
);
67-
if (angularVisitors) return angularVisitors;
68+
if (visitors) {
69+
Object.assign(angularVisitors, visitors);
70+
}
6871
}
72+
if (Object.keys(angularVisitors).length > 0) return angularVisitors;
6973

7074
const checkComponent = (node: any) => {
7175
const openingElement = node.openingElement || node;

packages/eslint-plugin/src/rules/form/form-label-required.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,18 @@ export default {
6363
}
6464
};
6565

66+
const angularVisitors: any = {};
6667
for (const comp of FORM_COMPONENTS) {
67-
const angularVisitors = createAngularVisitors(
68+
const visitors = createAngularVisitors(
6869
context,
6970
comp,
7071
angularHandler
7172
);
72-
if (angularVisitors) return angularVisitors;
73+
if (visitors) {
74+
Object.assign(angularVisitors, visitors);
75+
}
7376
}
77+
if (Object.keys(angularVisitors).length > 0) return angularVisitors;
7478

7579
const checkFormComponent = (node: any) => {
7680
const openingElement = node.openingElement || node;

packages/eslint-plugin/src/rules/icon/prefer-icon-attribute.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ export default {
5050
);
5151

5252
if (iconChild) {
53-
const iconValue = getAttributeValue(iconChild, 'icon');
5453
const loc = parserServices.convertNodeSourceSpanToLoc(
5554
iconChild.sourceSpan
5655
);
@@ -62,14 +61,18 @@ export default {
6261
}
6362
};
6463

64+
const angularVisitors: any = {};
6565
for (const comp of COMPONENTS_WITH_ICON_ATTR) {
66-
const angularVisitors = createAngularVisitors(
66+
const visitors = createAngularVisitors(
6767
context,
6868
comp,
6969
angularHandler
7070
);
71-
if (angularVisitors) return angularVisitors;
71+
if (visitors) {
72+
Object.assign(angularVisitors, visitors);
73+
}
7274
}
75+
if (Object.keys(angularVisitors).length > 0) return angularVisitors;
7376

7477
const checkComponent = (node: any) => {
7578
const openingElement = node.openingElement || node;

0 commit comments

Comments
 (0)