Don't resolve generic HTMLElement / MathMLElement to a phantom tag#12
Open
Don't resolve generic HTMLElement / MathMLElement to a phantom tag#12
HTMLElement / MathMLElement to a phantom tag#12Conversation
…hantom tags A component declaring `Signature['Element'] = HTMLElement` (the bare generic, not a specific subclass) was being Glint-resolved to <abbr>. Reason: TS's lib.dom.d.ts HTMLElementTagNameMap has many entries mapping to bare HTMLElement (`abbr`, `address`, `b`, `cite`, `code`, ...) and our inversion picked whichever appeared first. Downstream rules then FP-fired element-permitted-content on legal content because they thought the ancestor was an `<abbr>`. Exclude bare HTMLElement / SVGElement / MathMLElement from the inversion so a generic Element declaration falls through to 'transparent' (children float to the actual parent — the right behaviour when a component renders "some generic container"). Surfaced by ecosystem CI on ember-power-select (1× <div> not permitted under <abbr> at power-select.gts:1443) and HDS (8× <div> under <abbr> + 3× <option> under <abbr> + 1× <abbr> under <ol>, all from one generic-Element declaration somewhere in the component tree).
There was a problem hiding this comment.
Pull request overview
This PR fixes a Glint element-type resolution edge case where a component declaring a generic base element type (e.g. Signature['Element'] = HTMLElement) could be incorrectly resolved to an arbitrary concrete tag (e.g. <abbr>), leading to downstream false positives in content-model validation.
Changes:
- Exclude generic base element types (
HTMLElement,SVGElement,MathMLElement) from the*TagNameMapinversion used to map DOM element types back to tag names. - Add a regression test to ensure generic
HTMLElementdoes not resolve to a “phantom” concrete tag like<abbr>. - Add new
.gtsfixtures to exercise cross-file resolution for the generic-HTMLElementcase.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
lib/glint.ts |
Skips inverting tag-name-map entries whose value is a generic base element type, preventing arbitrary tag selection like HTMLElement → abbr. |
test/glint.test.ts |
Adds regression coverage ensuring Element: HTMLElement does not produce a phantom resolved tag. |
test/glint-fixtures/generic-html-element.gts |
New fixture component declaring Element: HTMLElement to reproduce the prior resolution behavior. |
test/glint-fixtures/generic-html-element-consumer.gts |
New consumer fixture to force cross-file resolution of the generic-HTMLElement component. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ent types Per Copilot review on #12: returning null for generic HTMLElement / SVGElement / MathMLElement let the built-in name-based fallbacks in blank.ts fire (e.g. user-defined <Input> incorrectly substituted as <input>). 'transparent' signals 'Glint resolved this; we just don't know which specific tag', and blank.ts handles transparent by floating children to the actual parent — the right semantic. Test now asserts the 'transparent' entry is recorded (in addition to 'abbr' not appearing).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A component declaring
Signature['Element'] = HTMLElement(the bare generic, not a specific subclass) was being Glint-resolved to<abbr>. Reason: TS'slib.dom.d.tsHTMLElementTagNameMaphas many tags mapping to bareHTMLElement—abbr,address,b,cite,code, … — and our inversion picked the first one alphabetically. Downstream rules then FP-firedelement-permitted-contenton legal content because they thought the ancestor was an<abbr>.This change excludes the bare base classes (
HTMLElement,SVGElement,MathMLElement) from the inversion, so a component with a genericElementfalls through to'transparent'(children float to the actual parent — the right behaviour for "I render some generic container").Surfaced by
Ecosystem CI:
<div>not permitted under<abbr>atpower-select.gts:1443<div>under<abbr>, 3×<option>under<abbr>, 1×<abbr>under<ol>— all from one generic-Elementdeclaration somewhere in the component treeTest plan
test/glint.test.tsassertsSignature['Element'] = HTMLElementdoes NOT resolve to<abbr>MathMLElementexclusion is needed (e.g.annotation,maction,mathmap to bareMathMLElement);SVGElementexclusion is preventive (no bare-typed SVG entries today, but symmetric for future-proofing)npm test— 128/128 (was 127, +1 new regression test)npm run typecheck:tests— cleannpm run build— clean