Skip to content

Commit 822a3ab

Browse files
committed
refactor+fix: migrate utils, narrow focusability check, normalize aria-hidden (Copilot review)
1 parent e834954 commit 822a3ab

2 files changed

Lines changed: 170 additions & 0 deletions

File tree

lib/utils/is-native-element.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
'use strict';
2+
3+
const htmlTags = require('html-tags');
4+
const svgTags = require('svg-tags');
5+
const { mathmlTagNames } = require('mathml-tag-names');
6+
7+
// Authoritative set of native element tag names. Mirrors the approach
8+
// established by #2689 (template-no-block-params-for-html-elements), which
9+
// the maintainer requires for component-vs-element discrimination in this
10+
// plugin. Heuristic approaches (PascalCase detection, etc.) were explicitly
11+
// rejected there because a lowercase tag CAN be a component in GJS/GTS when
12+
// the name is bound in scope (e.g. `const div = MyComponent; <div />`).
13+
const ELEMENT_TAGS = new Set([...htmlTags, ...svgTags, ...mathmlTagNames]);
14+
15+
/**
16+
* Returns true if the Glimmer element node is a native HTML / SVG / MathML
17+
* element — i.e. the tag name is in the authoritative list AND is not
18+
* shadowed by an in-scope binding.
19+
*
20+
* "Native" here means **spec-registered tag name** (in the HTML, SVG, or
21+
* MathML spec registries, reached via the `html-tags` / `svg-tags` /
22+
* `mathml-tag-names` packages). It is NOT the same as:
23+
*
24+
* - "native accessibility" / "widget-ness" — see `interactive-roles.js`
25+
* (aria-query widget taxonomy; an ARIA-tree-semantics question)
26+
* - "native interactive content" / "focus behavior" — see
27+
* `html-interactive-content.js` (HTML §3.2.5.2.7; an HTML-content-model
28+
* question about which tags can be nested inside what)
29+
* - "natively focusable" / sequential-focus — see HTML §6.6.3
30+
*
31+
* This util answers only: "is this tag a first-class built-in element of one
32+
* of the three markup-language standards, rather than a component invocation
33+
* or a shadowed local binding?" Callers compose it with the other utils
34+
* above when they need a more specific question (see e.g. `template-no-
35+
* noninteractive-tabindex`, which consults both this and
36+
* `html-interactive-content`).
37+
*
38+
* Returns false for:
39+
* - components (PascalCase, dotted, @-prefixed, this.-prefixed, ::-namespaced —
40+
* none of these tag names appear in the HTML/SVG/MathML lists)
41+
* - custom elements (`<my-widget>`) — accepted false negative; the web-
42+
* components namespace is open and can't be enumerated
43+
* - scope-bound identifiers (`<div>` when `div` is a local `let` / `const` /
44+
* import / block-param in the enclosing scope)
45+
*
46+
* @param {object} node - GlimmerElementNode
47+
* @param {object} [sourceCode] - ESLint SourceCode, for scope lookup. When
48+
* omitted, the scope check is skipped (the result is then list-based only —
49+
* suitable for unit tests).
50+
*/
51+
function isNativeElement(node, sourceCode) {
52+
if (!node || typeof node.tag !== 'string') {
53+
return false;
54+
}
55+
if (!ELEMENT_TAGS.has(node.tag)) {
56+
return false;
57+
}
58+
if (!sourceCode || !node.parent) {
59+
return true;
60+
}
61+
const scope = sourceCode.getScope(node.parent);
62+
const firstPart = node.parts && node.parts[0];
63+
if (firstPart && scope.references.some((ref) => ref.identifier === firstPart)) {
64+
return false;
65+
}
66+
return true;
67+
}
68+
69+
/**
70+
* Inverse of {@link isNativeElement}. Returns true when the node should NOT
71+
* be treated as a native HTML element — either because it's a component
72+
* invocation (PascalCase, dotted, @-prefixed, this.-prefixed, custom element)
73+
* OR a tag name that's shadowed by a scope binding.
74+
*/
75+
function isComponentInvocation(node, sourceCode) {
76+
return !isNativeElement(node, sourceCode);
77+
}
78+
79+
module.exports = { isNativeElement, isComponentInvocation, ELEMENT_TAGS };
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
'use strict';
2+
3+
const { isNativeElement, ELEMENT_TAGS } = require('../../../lib/utils/is-native-element');
4+
5+
// Tests exercise the list-lookup path only. Scope-based shadowing is covered
6+
// by the rule-level test suites (see tests/lib/rules/template-no-block-params-
7+
// for-html-elements.js and siblings) because it requires a real ESLint
8+
// SourceCode / scope manager that's only built up by the rule tester.
9+
10+
describe('isNativeElement — list-only behavior (no sourceCode)', () => {
11+
it('returns true for lowercase HTML tag names', () => {
12+
expect(isNativeElement({ tag: 'div' })).toBe(true);
13+
expect(isNativeElement({ tag: 'article' })).toBe(true);
14+
expect(isNativeElement({ tag: 'h1' })).toBe(true);
15+
expect(isNativeElement({ tag: 'button' })).toBe(true);
16+
expect(isNativeElement({ tag: 'form' })).toBe(true);
17+
expect(isNativeElement({ tag: 'section' })).toBe(true);
18+
});
19+
20+
it('returns true for SVG tag names', () => {
21+
expect(isNativeElement({ tag: 'svg' })).toBe(true);
22+
expect(isNativeElement({ tag: 'circle' })).toBe(true);
23+
expect(isNativeElement({ tag: 'path' })).toBe(true);
24+
});
25+
26+
it('returns true for MathML tag names', () => {
27+
expect(isNativeElement({ tag: 'mfrac' })).toBe(true);
28+
expect(isNativeElement({ tag: 'math' })).toBe(true);
29+
});
30+
31+
it('returns false for PascalCase component tags', () => {
32+
expect(isNativeElement({ tag: 'Button' })).toBe(false);
33+
expect(isNativeElement({ tag: 'MyWidget' })).toBe(false);
34+
// Native-tag names in PascalCase — the core bug case.
35+
expect(isNativeElement({ tag: 'Article' })).toBe(false);
36+
expect(isNativeElement({ tag: 'Form' })).toBe(false);
37+
expect(isNativeElement({ tag: 'Main' })).toBe(false);
38+
expect(isNativeElement({ tag: 'Nav' })).toBe(false);
39+
expect(isNativeElement({ tag: 'Section' })).toBe(false);
40+
expect(isNativeElement({ tag: 'Table' })).toBe(false);
41+
});
42+
43+
it('returns false for named-arg invocations', () => {
44+
expect(isNativeElement({ tag: '@heading' })).toBe(false);
45+
expect(isNativeElement({ tag: '@tag.foo' })).toBe(false);
46+
});
47+
48+
it('returns false for this-path invocations', () => {
49+
expect(isNativeElement({ tag: 'this.myComponent' })).toBe(false);
50+
expect(isNativeElement({ tag: 'this.comp.sub' })).toBe(false);
51+
});
52+
53+
it('returns false for dot-path invocations', () => {
54+
expect(isNativeElement({ tag: 'foo.bar' })).toBe(false);
55+
expect(isNativeElement({ tag: 'ns.widget' })).toBe(false);
56+
});
57+
58+
it('returns false for named-block / namespaced invocations', () => {
59+
expect(isNativeElement({ tag: 'foo::bar' })).toBe(false);
60+
expect(isNativeElement({ tag: 'Foo::Bar' })).toBe(false);
61+
});
62+
63+
it('returns false for custom elements (accepted false negative)', () => {
64+
// Custom elements aren't in the html-tags/svg-tags/mathml-tag-names
65+
// allowlists. They're treated as "not a native element" so downstream
66+
// rules skip them — matches the convention established in PR #2689.
67+
expect(isNativeElement({ tag: 'my-element' })).toBe(false);
68+
expect(isNativeElement({ tag: 'x-foo' })).toBe(false);
69+
});
70+
71+
it('returns false for empty / missing / non-string tag', () => {
72+
expect(isNativeElement({ tag: '' })).toBe(false);
73+
expect(isNativeElement({ tag: undefined })).toBe(false);
74+
expect(isNativeElement({ tag: null })).toBe(false);
75+
expect(isNativeElement({ tag: 123 })).toBe(false);
76+
expect(isNativeElement({})).toBe(false);
77+
expect(isNativeElement()).toBe(false);
78+
expect(isNativeElement(null)).toBe(false);
79+
});
80+
});
81+
82+
describe('ELEMENT_TAGS', () => {
83+
it('includes all HTML, SVG, and MathML tag names', () => {
84+
// Sanity check — if this ever drops below a reasonable size, one of the
85+
// underlying packages has changed contract.
86+
expect(ELEMENT_TAGS.size).toBeGreaterThan(200);
87+
expect(ELEMENT_TAGS.has('div')).toBe(true);
88+
expect(ELEMENT_TAGS.has('circle')).toBe(true);
89+
expect(ELEMENT_TAGS.has('mfrac')).toBe(true);
90+
});
91+
});

0 commit comments

Comments
 (0)