Skip to content

Commit 659c116

Browse files
committed
PR Feedback
1 parent ad4c929 commit 659c116

3 files changed

Lines changed: 178 additions & 191 deletions

File tree

docs/rules/template-no-invalid-link-text.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,4 @@ Examples of **correct** code for this rule:
7272

7373
- [WebAIM: Link Text and Appearance](https://webaim.org/techniques/hypertext/link_text)
7474
- [WCAG 2.4.4: Link Purpose (In Context)](https://www.w3.org/WAI/WCAG21/Understanding/link-purpose-in-context.html)
75-
- [eslint-plugin-ember template-no-invalid-link-text](https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/template-no-invalid-link-text.md)
75+
- [ember-template-lint: no-invalid-link-text](https://github.com/ember-template-lint/ember-template-lint/blob/main/docs/rule/no-invalid-link-text.md)

lib/rules/template-no-invalid-link-text.js

Lines changed: 70 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -22,53 +22,49 @@ function getTextContentResult(node) {
2222
return { text: '', hasDynamic: false };
2323
}
2424

25+
function isDynamicValue(value) {
26+
return value?.type === 'GlimmerMustacheStatement' || value?.type === 'GlimmerConcatStatement';
27+
}
28+
2529
/**
26-
* Check if the node has a valid aria-label or aria-labelledby that
27-
* exempts it from link text validation.
30+
* Checks aria-labelledby and aria-label attributes.
31+
* Returns:
32+
* { skip: true } — has valid accessible name, skip element
33+
* { report: true, text: string } — aria-label is itself a disallowed text, report it
34+
* { skip: false } — no valid aria override, check text content
2835
*/
29-
function hasValidAriaLabelOrLabelledby(node) {
30-
const attrs = node.attributes || [];
31-
32-
// Check aria-labelledby
36+
function checkAriaAttributes(attrs) {
3337
const ariaLabelledby = attrs.find((a) => a.name === 'aria-labelledby');
3438
if (ariaLabelledby) {
35-
if (ariaLabelledby.value && ariaLabelledby.value.type === 'GlimmerTextNode') {
36-
const val = ariaLabelledby.value.chars.trim();
37-
// Only valid if non-empty
38-
return val.length > 0;
39+
if (isDynamicValue(ariaLabelledby.value)) {
40+
return { skip: true };
3941
}
40-
// Dynamic value — assume valid
41-
if (
42-
ariaLabelledby.value &&
43-
(ariaLabelledby.value.type === 'GlimmerMustacheStatement' ||
44-
ariaLabelledby.value.type === 'GlimmerConcatStatement')
45-
) {
46-
return true;
42+
if (ariaLabelledby.value?.type === 'GlimmerTextNode') {
43+
if (ariaLabelledby.value.chars.trim().length > 0) {
44+
return { skip: true }; // valid non-empty labelledby
45+
}
4746
}
48-
// No value or empty — not valid
49-
return false;
47+
// empty aria-labelledby → fall through
48+
return { skip: false };
5049
}
5150

52-
// Check aria-label
5351
const ariaLabel = attrs.find((a) => a.name === 'aria-label');
5452
if (ariaLabel) {
55-
// Dynamic value — assume valid
56-
if (
57-
ariaLabel.value &&
58-
(ariaLabel.value.type === 'GlimmerMustacheStatement' ||
59-
ariaLabel.value.type === 'GlimmerConcatStatement')
60-
) {
61-
return true;
53+
if (isDynamicValue(ariaLabel.value)) {
54+
return { skip: true };
6255
}
63-
if (ariaLabel.value && ariaLabel.value.type === 'GlimmerTextNode') {
56+
if (ariaLabel.value?.type === 'GlimmerTextNode') {
6457
const val = ariaLabel.value.chars.replaceAll(' ', ' ').toLowerCase().trim();
65-
// aria-label itself must not be disallowed text
66-
return val.length > 0 && !DISALLOWED_LINK_TEXTS.has(val);
58+
if (val.length > 0 && !DISALLOWED_LINK_TEXTS.has(val)) {
59+
return { skip: true }; // valid aria-label
60+
}
61+
if (val.length > 0) {
62+
return { skip: true, report: true, text: val }; // aria-label itself is disallowed
63+
}
6764
}
68-
return false;
6965
}
7066

71-
return false;
67+
return { skip: false };
7268
}
7369

7470
/** @type {import('eslint').Rule.RuleModule} */
@@ -108,28 +104,46 @@ module.exports = {
108104
const options = context.options[0] || {};
109105
const allowEmptyLinks = options.allowEmptyLinks || false;
110106
const customLinkComponents = options.linkComponents || [];
111-
const linkTags = new Set(['a', 'LinkTo', ...customLinkComponents]);
107+
108+
const filename = context.filename ?? context.getFilename();
109+
const isStrictMode = filename.endsWith('.gjs') || filename.endsWith('.gts');
110+
111+
// In HBS, LinkTo always refers to Ember's router link component.
112+
// In GJS/GTS, LinkTo must be explicitly imported from '@ember/routing'.
113+
// local alias → true (any truthy value marks it as a tracked link component)
114+
const importedLinkComponents = new Map();
115+
116+
const linkTags = new Set(['a', ...customLinkComponents]);
117+
if (!isStrictMode) {
118+
linkTags.add('LinkTo');
119+
}
112120

113121
function checkLinkContent(node, children) {
114-
// Skip if has aria-hidden
115-
const ariaHidden = (node.attributes || []).find((a) => a.name === 'aria-hidden');
122+
const attrs = node.attributes || [];
123+
124+
// Skip if aria-hidden="true"
125+
const ariaHidden = attrs.find((a) => a.name === 'aria-hidden');
116126
if (ariaHidden?.value?.type === 'GlimmerTextNode' && ariaHidden.value.chars === 'true') {
117127
return;
118128
}
119129

120-
// Skip if has hidden attribute
121-
if ((node.attributes || []).some((a) => a.name === 'hidden')) {
130+
// Skip if hidden attribute present
131+
if (attrs.some((a) => a.name === 'hidden')) {
122132
return;
123133
}
124134

125-
// Check aria-label / aria-labelledby
126-
if (hasValidAriaLabelOrLabelledby(node)) {
135+
const ariaResult = checkAriaAttributes(attrs);
136+
if (ariaResult.report) {
137+
context.report({ node, messageId: 'invalidText', data: { text: ariaResult.text } });
138+
return;
139+
}
140+
if (ariaResult.skip) {
127141
return;
128142
}
129143

144+
// Check text content
130145
let fullText = '';
131146
let hasDynamic = false;
132-
133147
for (const child of children || []) {
134148
const result = getTextContentResult(child);
135149
fullText += result.text;
@@ -138,50 +152,46 @@ module.exports = {
138152
}
139153
}
140154

141-
// If there's dynamic content, skip (can't validate)
142155
if (hasDynamic) {
143-
return;
156+
return; // can't validate dynamic content
144157
}
145158

146159
const normalized = fullText.trim().toLowerCase().replaceAll(/\s+/g, ' ');
147160

148-
// Empty link check
149161
if (!normalized.replaceAll(' ', '')) {
150162
if (!allowEmptyLinks) {
151-
context.report({
152-
node,
153-
messageId: 'invalidText',
154-
data: { text: '(empty)' },
155-
});
163+
context.report({ node, messageId: 'invalidText', data: { text: '(empty)' } });
156164
}
157165
return;
158166
}
159167

160168
if (DISALLOWED_LINK_TEXTS.has(normalized)) {
161-
context.report({
162-
node,
163-
messageId: 'invalidText',
164-
data: { text: normalized },
165-
});
169+
context.report({ node, messageId: 'invalidText', data: { text: normalized } });
166170
}
167171
}
168172

169173
return {
174+
ImportDeclaration(node) {
175+
if (node.source.value === '@ember/routing') {
176+
for (const specifier of node.specifiers) {
177+
if (specifier.type === 'ImportSpecifier' && specifier.imported.name === 'LinkTo') {
178+
importedLinkComponents.set(specifier.local.name, true);
179+
linkTags.add(specifier.local.name);
180+
}
181+
}
182+
}
183+
},
184+
170185
GlimmerElementNode(node) {
171186
if (!linkTags.has(node.tag)) {
172187
return;
173188
}
174-
175189
checkLinkContent(node, node.children);
176190
},
177191

178192
GlimmerBlockStatement(node) {
179-
if (
180-
node.path &&
181-
node.path.type === 'GlimmerPathExpression' &&
182-
node.path.original === 'link-to'
183-
) {
184-
checkLinkContent(node, node.program && node.program.body);
193+
if (node.path?.type === 'GlimmerPathExpression' && node.path.original === 'link-to') {
194+
checkLinkContent(node, node.program?.body);
185195
}
186196
},
187197
};

0 commit comments

Comments
 (0)