Resolve Element for TOC components (satisfies TOC<S> and : TOC<S> forms)#16
Merged
Resolve Element for TOC components (satisfies TOC<S> and : TOC<S> forms)#16
Element for TOC components (satisfies TOC<S> and : TOC<S> forms)#16Conversation
…<S>` / `: TOC<S>`
Glint's TOC overload reaches `emitComponent(...).element` but surfaces
the type as `unknown` (or `any`) for both the satisfies form
`const X = <template>...</template> satisfies TOC<{Element: T}>;`
and the type-annotation form
`const X: TOC<{Element: T}> = <template>...</template>;`
even though `T` is statically known on the value's declaration.
Without resolution the component fell through to transparent blanking,
floating its children to the actual parent — e.g. a `<li>`-yielding
TOC inside `<ul>` would leak its body to `<ul>` directly and
`element-permitted-content` would FP-fire on legal content like `<p>`
or `<blockquote>`.
When `.element` is unknown/any, walk back to the component's variable
declaration, find the `TOC<S>` TypeReference (in either the type
annotation or the SatisfiesExpression's `.type`), pull its first type
argument, and read `Element` off it. Map the result through the same
`elementTypeToTag` table the class form uses. Falls through to
transparent for genuinely yields-only TOCs (no `Element` declared).
Surfaced by ecosystem CI on ember-a11y-testing's `ViolationsGridItem`
and HDS's various internal list-item TOCs. Fixture
`toc-list-item.gts` mirrors the satisfies form ember-a11y-testing
uses; `toc-list-item-consumer.gts` exercises it inside `<ul>`.
There was a problem hiding this comment.
Pull request overview
Improves Glint-based component element resolution for template-only components (TOC) when Glint surfaces emitComponent(...).element as unknown/any for satisfies TOC<…> and : TOC<…> = declarations, preventing incorrect “transparent” fallback that can cause content-model false positives.
Changes:
- Adds a recovery path that walks from the emitted
resolve(Comp)reference back to the variable declaration to readTOC<S>’s type argument and deriveS['Element']. - Refactors element-type-to-tag matching into a shared helper (
matchElementTypeToTag) used by both the normal and recovery paths. - Adds cross-file fixtures + two new tests covering both
satisfies TOC<…>and: TOC<…> =TOC declaration forms.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/glint.ts | Adds TOC Element recovery when .element is unknown/any; extracts shared tag-matching helper. |
| test/glint.test.ts | Adds two integration tests asserting TOC → 'li' resolution for satisfies + annotation forms. |
| test/glint-fixtures/toc-list-item.gts | New TOC fixture using satisfies TOC<{ Element: HTMLLIElement; … }> form. |
| test/glint-fixtures/toc-list-item-consumer.gts | New consumer fixture that imports and invokes the satisfies-form TOC. |
| test/glint-fixtures/toc-annotated-list-item.gts | New TOC fixture using const X: TOC<S> = <template>…</template>; form. |
| test/glint-fixtures/toc-annotated-list-item-consumer.gts | New consumer fixture that imports and invokes the annotation-form TOC. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced May 7, 2026
Per Copilot review on #16: the inline comment in resolveComponentElement said Glint 'tends to surface as unknown' for satisfies-TOC forms while the doc on resolveElementFromSatisfiesTOC said 'ends up as any'. Both shapes can occur (unknown most often, any in cascading-error files; the code already handles both via the (Unknown | Any) check). Aligned the doc comment to match.
johanrd
added a commit
that referenced
this pull request
May 7, 2026
Per Copilot review on #18: 1. Removed the reference to resolveElementFromSatisfiesTOC from the inline comment — that helper exists on a sibling branch (fix/fp-toc-satisfies-element / PR #16) but not on this one. Stale on its own. 2. Added a fallback path for when TOC's type-args land on a TypeReference rather than aliasTypeArguments (i.e., when TOC is declared as `interface TOC<S>` instead of `type TOC<S> = …`). The public `checker.getTypeArguments(refType as TypeReference)` covers that case. aliasTypeArguments handles the type-alias form.
Per Copilot review on #16: 1. Renamed resolveElementFromSatisfiesTOC → resolveElementFromTOCDeclaration. The helper handles both the `satisfies TOC<…>` and `: TOC<…> =` annotation forms; the old name only mentioned satisfies and was misleading. 2. Added an isTOCTypeName check before reading Element off the type-arg. Without it, an unrelated generic annotation that happens to have an `Element` property could be mis-resolved when emitComponent's .element is unknown/any. Recognizes both the short form `TOC` and the long form `TemplateOnlyComponent`.
Per Copilot review on #16: 1. Three stale references to resolveElementFromSatisfiesTOC (in test/glint.test.ts and test/glint-fixtures/toc-annotated-list-item.gts) updated to the renamed resolveElementFromTOCDeclaration. 2. The isTOCTypeName comment said qualified names match the "leftmost identifier" but the code uses name.right (rightmost). The code was correct — for `Ember.TOC` we want to match `TOC`, the actual type name. Comment updated to say rightmost.
Per Copilot review on #16: 1. `findComponentDeclSourceFile` and `resolveElementFromTOCDeclaration` both navigated the same emitComponent → resolve(Comp) AST shape and ran the same alias-resolution dance. Extracted both to a shared helper, `getComponentSymbolFromEmitCall`. Both callers now stay in sync if Glint's emitted shape ever changes. 2. The comment above `matchElementTypeToTag` (line 603) said unions "take the first branch's mapping". The code actually picks the first *matching* branch (skipping ones with no DOM mapping). Updated to match. No behavioral change. 129/129 pass.
Owner
Author
|
@copilot resolve the merge conflicts in this pull request |
Co-authored-by: johanrd <4601554+johanrd@users.noreply.github.com>
Agent-Logs-Url: https://github.com/johanrd/html-validate-ember/sessions/76c531bf-c8d7-423a-85cf-339f25075e6b Co-authored-by: johanrd <4601554+johanrd@users.noreply.github.com>
This reverts commit be1dd2b. Co-authored-by: johanrd <4601554+johanrd@users.noreply.github.com>
Contributor
Done — I merged |
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
Glint's TOC overload reaches
emitComponent(...).elementbut surfaces the type asunknown(orany) for both the satisfies formand the type-annotation form
even though
Tis statically known on the value's declaration. Without resolution the component fell through to transparent blanking, floating its children to the actual parent — e.g. a<li>-yielding TOC inside<ul>would leak its body to<ul>directly andelement-permitted-contentwould FP-fire on legal content like<p>or<blockquote>.When
.elementisunknown/any, walk the component reference back to its variable declaration, find theTOC<S>TypeReference(in eitherdecl.typefor the annotation form ordecl.initializer.typefor the SatisfiesExpression form), pull its first type argument, and readElementoff it. Map the result through the sameelementTypeToTagtable the class form uses. Falls through to'transparent'for genuinely yields-only TOCs (noElementdeclared).A small refactor extracted
matchElementTypeToTagso both the regular path and the satisfies-recovery path share the union-branch lookup.Surfaced by
Ecosystem CI on:
ember-a11y/ember-a11y-testing—ViolationsGridItem(TOC declaringElement: HTMLLIElementvia satisfies); 4×element-permitted-contenton<button>/<input>/<code>/<img>flagged as not permitted under<ul>(because the runtime<li>was missing from the static analysis).hashicorp/design-system— multiple internal list-item TOCs.Test plan
satisfies TOC<…>) — fixture mirrorsViolationsGridItem's shape: TOC<…> =) — annotation-only declarationnpm test— 129/129 passing (was 127, +2 new tests)npm run typecheck:tests— cleannpm run build— clean