Add ecosystem CI: regression-test the plugin against real-world Ember repos - #24
Merged
Conversation
Pinned-snapshot validation against 10 public Ember repos (super-rentals, ember-website, ember-power-select, ember-modifier, ember-simple-auth, aria-voyager, ember-a11y-testing, hashicorp/design-system, ember-primitives, NullVoxPopuli/limber). Each target lives in `ecosystem/targets.json` with a pinned SHA, glob, and per-target Glint flag. The runner clones, installs deps (best-effort, lockfile-driven), validates with the same `:gts-recommended` config that `validate-gts` uses, and diffs against committed baselines. A non-empty diff fails the workflow. `ecosystem/run.ts` is single-file, runs via `tsx`, no heavy deps. Glint extraction is gated by `glint` flag per target (default true) and falls back to no-Glint when install fails so the run continues. Per-target triage docs live under `ecosystem/triage/`: - `<name>/triage.md` — every finding classified - `<name>/issue.md` — draft to file upstream when warranted - `FP-FIX-REPORT.md` — aggregate FPs across targets - `STYLISTIC-FINDINGS.md` — input for default-policy review super-rentals triaged. 11 TP, 2 FP-plugin (img splat → required-attr missing), 0 FP-rule. The 2 FPs are the first entry in FP-FIX-REPORT. `vitest.config.ts` scopes test discovery to `test/**` so cloned repos' test files don't get picked up. `tsconfig.test.json` typechecks `ecosystem/**`.
ember-modifier has a single trivial template (`{{outlet}}`) — no
findings; kept as a smoke target.
ember-simple-auth surfaces 6 real findings in the demo test-app, all
a11y-related: 3× anchor-as-button (`<a role='button' href='#'>`
pattern), 1× missing autocomplete on password input, 1× form without
submit, 1× implicit button type. No FPs.
The demo app contains deliberate a11y fixtures (`violations.gts`,
`ignored-image-alt.gts`); html-validate's reports on those are correct
but by-design — marked TP-intentional, no upstream issue.
The 4 element-permitted-content findings are all one root cause:
`ViolationsGridItem` is a TOC declared via
`<template>...</template> satisfies TOC<{ Element: HTMLLIElement }>`
which our Glint resolver doesn't recognize (works for `class extends
Component<Sig>` but not the TOC `satisfies` form). Without resolution,
the children float to <ul> instead of being correctly seen as inside
<li>.
New entry in FP-FIX-REPORT for the TOC-Element-resolution gap.
13 findings on 58 files.
Real bugs (3): non-semantic heading `<div role="heading">` should be
`<h*>`; non-semantic `<div role="progressbar">` should be `<progress>`;
`readonly` on `<input type="radio">` is invalid (use `disabled`).
Stylistic (5): inline styles in docs landing, region-as-div in
accordion content. Tracked in STYLISTIC-FINDINGS for default-policy
review.
Strictness (1): `<style>` inside `<fieldset>` for SR-only CSS;
debatable per spec.
FP-plugin (4): wcag/h32 on `<form ...attributes>{{yield}}</form>` and
wcag/h71 on `<fieldset ...attributes>{{yield ...}}</fieldset>`. The
plugin blanks the yield and html-validate sees empty body. At runtime
the consumer provides the required submit-button / legend. New entry
in FP-FIX-REPORT — same shape as multipass yield-only-branch but for
non-multipass templates.
All 13 findings are prefer-native-element on `<div role="listbox">` patterns in rendering tests. The addon's whole purpose is providing listbox/menu/tablist keyboard+ARIA behavior on non-native elements; replacing with <select> would defeat the tests. No issue to file, no plugin fix. Triage doc notes this is an applicability question (not FP, not stylistic-default) — addons that enhance ARIA-pattern widgets should disable prefer-native-element in their tests. Worth a "recommended overrides per project type" section in the plugin README.
34 findings on 114 files. Three real-bug clusters: - 12× <div role="button"> tabs in code-example.gts (anchor/div-as- button anti-pattern, or arguably should be role="tab" since they're inside <nav class="code-example-tabs">). - 14× misnested <p> containing <ul>/<dl> (7 implicit-close + 7 stray-</p> pairs, same root cause). - 1× <br> inside <ol> for visual spacing (should be CSS). 6× no-inline-style in demo placeholders — tracked stylistic. 1× FP-plugin-suspect: html-validate reports <div> not permitted under <abbr> at power-select.gts:1443 but the file has no literal <abbr>. A component's Signature['Element'] = HTMLElement (the generic) is likely being resolved by our plugin to "abbr" via a fallthrough in the element-class → tag mapping. Added to FP-FIX-REPORT for scoped investigation.
39 findings on 67 files. No new FP-plugin patterns — all real bugs or stylistic. Real bugs (28): - 20× misnested <p> containing <ul>/<dl> (10 blocks, two findings each — same root cause as ember-power-select's docs). - 2× aria-label on <span> with no role (silently ignored by ATs). - 2× unlabeled <footer> landmarks in same component (unique-landmark). - 2× <input> missing type, 1× <button> missing type, 1× wcag/h32 form without submit. Stylistic (10): no-inline-style across repl/components and templates. no-inline-style is now 20 across 3 targets — strong default-off candidate per STYLISTIC-FINDINGS. Strictness (1): <style> inside <label> in apps/tutorial selection.gts. Same shape as ember-primitives <style>-in-<fieldset>; defensible in practice (component-scoped CSS).
279 findings on 73 files; the largest target after HDS. Stylistic cluster (163) dominated by 153× void-style — the site's .hbs templates use omitted void-element form (<br>) over self-closing (<br />). Plugin's :gts-recommended forces selfclosing. After this target void-style is the largest stylistic cluster across all ecosystem targets — strong default-off candidate. Real bugs (17): <image> typo for <img>, 6× form-dup-name on commission form, 4× obsolete IE conditional comments, 2× deprecated attrs (frameborder, align), 1× misnested <p> with <ul>, 1× aria-labelledby on element that doesn't accept it, 1× <pre> in <code>, 1× ad-hoc inline style. FP-plugin (98): classic Ember addon component <EsCard> from ember-styleguide renders <li> but our plugin can't resolve the root tag without JS-side type info — its template is .hbs (no Signature, no `satisfies TOC`). Transparent-blanking floats children to the parent <ul> and element-permitted-content fires. New entry in FP-FIX-REPORT for the classic-addon-template-resolution gap. Substantial extension of existing splatted-root resolution to walk .hbs files.
303 findings on 859 files. Per-finding triage at this scale is infeasible; classified by-rule + by-pattern. By far the largest cluster is plugin-side limitations (~70%): - 107× <option> under <div> — yielded curried sub-components like `<HdsFormSelectBase as |C|><C.Options>...</C.Options>` lose Glint resolution. New entry in FP-FIX-REPORT for yielded-curried-component resolution. - 39× <iframe> missing title — `<ShwFrame @Label={{...}}>` provides title via arg-binding; plugin's splatted-root extractor only handles literal values. New entry for arg-bound-attribute resolution. - 25× <div> under <ul> — same EsCard-class addon-list-item pattern as ember-website (now bumped on existing entry). - 8× <div> under <abbr> + cascading abbr-flagged children — bumps the ember-power-select abbr-mystery entry significantly. - 10× wcag/h32 + wcag/h71 on yield-only HdsForm/HdsFormFieldset (bumps the ember-primitives form/fieldset entry). Real bugs (~52): 14× <div> in <button>, 5× <input> in <a>, 2× nested <a>, 28× prefer-native-element, 6× implicit button-type, 8× aria-label misuse warnings, plus misc. Notable defensive pattern: 15× role="list" on <ul> — html-validate flags as redundant but it's a deliberate Safari/VoiceOver fallback for when list-style:none strips the implicit list role. Marked TP-rule- context (correct per spec, intentional in practice). issue.md is selective — filing 303 on HDS is unrealistic; surfaces the ~10 highest-value real findings.
# Conflicts: # lib/glint.ts
# Conflicts: # lib/glint.ts # test/glint.test.ts
…ixes # Conflicts: # test/glint.test.ts
…tl, vertical-collection, ember-resources, cardstack-ui-components, discourse)
Merged combined-fp-fixes into ecosystem-ci, reseeded all 17 baselines against the latest plugin so the diff isolates the FP-fix impact. Existing 10 targets: super-rentals -1 element-required-attributes (img splat FP, PR #13) ember-primitives -2 wcag/h32 (yield-only form FP, PR #17) hds-design-system -2 wcag/h71 (yield-only fieldset FP, PR #17) +1 prefer-native-element (newly visible after other fixes resolve a previously-blanked region) others (7) unchanged 7 new targets seeded (file count / finding count): ember-concurrency 56 / 32 (element-permitted, no-inline-style) warp-drive 8 / 0 ember-intl 79 / 0 vertical-collection 14 / 10 (no-implicit-button-type) ember-resources 3 / 0 cardstack-ui-components 31 / 21 (void-style, no-implicit-button-type) discourse 1309 / 495 (Glint disabled — too heavy to install; mostly TPs in legacy .hbs) Baselines reflect current findings; CI guards against future regressions.
Re-baseline after the comma-vs-space fix in transform.ts. The HDS
`form/index.gts:81` wcag/h32 was the canonical multipass case that
exposed the bug — both arms of `{{#if (eq this.tag "form")}}` carry
yield, so multipass injects a directive carrying both
`no-unused-disable` and `wcag/h32`. With space-separated rules only
the first was disabled and h32 leaked through; comma-separated now
suppresses both correctly.
No other baselines moved.
Goal: ":recommended" should give "pure value" — content-model bugs,
a11y issues, missing required attributes — not pedantic style
preferences. Trim the html-validate-recommended noise that fires on
legitimate Ember/Glimmer patterns:
- `no-inline-style: off` — bans `style=` attribute. Breaks legitimate
runtime style-binding (`<div style={{this.computedStyle}}>`). Use a
separate stylelint pipeline for inline-style policy if needed.
- `void-style: off` — html-validate defaults to omit, Ember/Glimmer
convention is selfclosing, mixing is harmless. Previously the
`:gts-recommended` preset overrode to `selfclosing`, which fought
projects using the omit form. Now disabled entirely; teams enforce
via `ember-template-lint` if they want that policy.
- `prefer-native-element: warn` — kept on but demoted from error.
Real a11y signal (`<div role="button">` should usually be
`<button>`), but design systems intentionally wrap generic elements
with role+keyboard handling. Surfaced as a warning so it doesn't
fail builds; teams can promote back to error per-project.
`:recommended` and `:gts-recommended` now resolve to identical rule
sets. The alias stays for backwards compatibility — existing
consumers' `extends` keep working.
151/151 pass.
Removed 329 baseline entries across 11 targets (0 new findings) by
disabling stylistic rules from html-validate's recommended preset that
fire on legitimate Ember/Glimmer code without catching real bugs:
void-style -170 (disabled — Ember mixes both styles)
no-inline-style -87 (disabled — runtime style-binding pattern)
prefer-native-element -72 (demoted to warn — surfaces in lint
output, doesn't fail builds, design
systems can intentionally wrap divs)
The 5 already-removed FPs from the plugin fix-branches (super-rentals
img splat, ember-primitives 2x wcag/h32, hds 2x wcag/h71) plus the
hds form/index.gts:81 wcag/h32 cleared by the directive comma fix
all stayed cleared.
The runner previously skipped any file where `report.valid === true`,
which silently dropped warning-only files from the baseline — every
`prefer-native-element` warning on an otherwise-clean file was
invisible to CI, and the next run that introduced a warning to such
a file wouldn't show in the diff.
Changes:
- `Finding` interface gains a `severity: 'error' | 'warning'` field
derived from html-validate's numeric `m.severity` (1=warn, 2=error).
- Drop the `if (report.valid) continue;` early-out so warning-only
files reach the recording loop.
- `findingKey` and the sort comparator include severity, so a finding
whose severity flips between runs counts as a real diff.
- Per-target stderr summary now reads
`findings: N (errors: X, warnings: Y)`.
- `summarizeFindings` prefixes each line with `[severity]`.
All 17 baselines re-seeded. Net delta vs the previous baselines:
aria-voyager +13 warnings (previously fully skipped:
0 errors → report.valid → file dropped)
cardstack-ui-components +1
discourse +33 (49 warnings now; 16 were on
mixed-error files and already recorded)
ember-concurrency +1
ember-power-select +12 (warnings on previously-clean files)
ember-primitives +3
ember-simple-auth +3
hds-design-system +10 (29 warnings now; 19 previously on
mixed-error files)
No error-severity findings changed (the rule trim already settled
those). Warnings-only files now visible to baseline diffs.
…ber-intl, warp-drive)
These four were noise-only against the trimmed ruleset:
aria-voyager 13 warnings, all `<div role="listbox">` patterns
— intentional ARIA wrappers, not bugs
ember-resources 0 errors, 0 warnings
ember-intl 0 errors, 0 warnings
warp-drive 0 errors, 0 warnings
Three were always-clean (no signal to guard against), one is
intentionally-different (signal we'd never want to fix). Removing
them shrinks the per-PR CI runtime, focuses the baseline diff on
targets where regressions actually matter, and removes the
"baseline at zero forever" noise from PR review.
Down from 17 → 13 targets.
After the per-element migration in this PR, no detection branch
populates `fileLevel` / `BlankResult.disableForRules` — that field
was always empty. Remove it and the dead transform.ts plumbing:
- blank.ts: drop `disableForRules` from BlankResult / BlankErrorResult.
`detectSuppressions` now returns `Map<offset, Set<rule>>` directly
(no `{ fileLevel, perElement }` wrapper). Rewrite the docstring
above `detectSuppressions` to enumerate the 7 FP classes actually
covered today and frame them all as per-element.
- transform.ts: .hbs source path drops the buildDisableDirective
call (no rules to add). Multipass .gts path inlines
`buildDisableDirective(['no-unused-disable'])` instead of
iterating an empty `disableForRules`. buildDisableDirective doc
updated to reflect that it carries only no-unused-disable today.
- test/blank.test.ts: `suppressesRule` helper drops the
disableForRules check (always empty post-migration). Rationale
comment updated; pointer to per-offset assertion added.
- test/integration.test.ts: three test rationale comments that still
referenced `disableForRules` updated to describe the per-element
map.
# Conflicts: # blank.ts
…ession) The fix/38 merge changed which findings the ecosystem targets produce. Re-baselined the 4 affected targets: - discourse: -600/+80 lines — bulk wcag/h63 / element-permitted-content FPs removed; the few additions are real upstream bugs (e.g. <div> under <ul>, <button> under <a>, <th> directly under <thead>) that the per-element migration unmasked once file-level over-suppression was dropped. - hds-design-system: same shape — many FPs removed, a handful of real <div>-under-<ul> bugs in showcase demo code surfaced. - ember-website: -1 (element-name FP on <image> fixed). - limber: -1 (wcag/h32 FP fixed). Generated locally with node 24 (CI runs node 22); discourse is deterministic no-Glint, and the Glint targets use frozen lockfiles + pinned TS, so cross-node drift should be nil. If a subsequent CI run still diffs, re-baseline from the CI output.
cardstack-ui-components pins yarn via package.json#packageManager; the runner's global yarn (1.22.22) rejects it with a Corepack error, so installDeps falls back to no-Glint validation. `corepack enable` routes the yarn/pnpm shims to the pinned version, restoring Glint- resolved findings for those targets.
run.ts: add a per-target `build` step (run before validation, after a fresh install, with a re-inject) so source-only/unbuilt workspace deps resolve — HDS's @hashicorp/design-system-components ships only dist/declarations via its `files` allowlist, absent until its rollup build runs, so without this PascalCase components blanked transparent and structural rules couldn't see the rendered DOM. Also add a `--no-glint` flag (keeps install+build, disables Glint extraction) to measure Glint's contribution vs the canonical resolver. targets.json: build @hashicorp/design-system-components for HDS. Re-baseline HDS with built deps + the merged resolver fixes (exports resolution, re-yield h71 fix, dotted-transparent): 95 -> 271. The FP classes are resolved (wcag/h71 74 -> 17 via the re-yield fix; no element-permitted-content flood). Remaining findings are largely real (e.g. block-in-<p> tooltips, prefer-tbody).
Comment on lines
+124
to
+125
| See [ECOSYSTEM-OVERLAP.md](https://github.com/johanrd/html-validate-ember/blob/main/ECOSYSTEM-OVERLAP.md) | ||
|
|
The void-style / no-inline-style / prefer-native-element suppressions live in the ecosystem runner's config (ECOSYSTEM_RULE_OVERRIDES) instead of the plugin's shipped :recommended / :gts-recommended presets, which return to their original form. Net rule set applied to the targets is unchanged, so baselines are untouched (verified: ember-modifier, super-rentals, ember-primitives all clean vs baseline). Whether the plugin's recommended set should adopt the same suppressions is tracked in PR #47, not here.
johanrd
changed the title
Comment on lines
+167
to
+173
| } else if (fs.existsSync(path.join(repoDir, 'yarn.lock'))) { | ||
| // Works for yarn classic; yarn berry rejects `--frozen-lockfile` and | ||
| // wants `--immutable`. We don't retry — install failure is non-fatal | ||
| // and the caller logs it then continues without Glint. | ||
| cmd = 'yarn'; | ||
| args = ['install', '--frozen-lockfile', '--ignore-scripts']; | ||
| } else if (fs.existsSync(path.join(repoDir, 'package-lock.json'))) { |
Comment on lines
+479
to
+485
| const haveDeps = wantGlint && fs.existsSync(path.join(repoDir, 'node_modules')); | ||
| let didInstall = false; | ||
| let installed = haveDeps; | ||
| if (wantGlint && !haveDeps && !args.noClone) { | ||
| installed = installDeps(repoDir, target); | ||
| didInstall = installed; | ||
| } |
| | `eslint-plugin-ember` / `ember-template-lint` | Is this idiomatic Ember? Invocation style, reactivity, modifier API, built-in components, plus JS/TS-side rules from eslint-plugin-ember and some overlap of HTML spec and ARIA rules | | ||
| | `html-validate-ember` (this plugin) | Is the rendered HTML spec-correct and accessible? — content model, ARIA / WCAG, form correctness, attribute validity, duplicate IDs, unique landmarks. | | ||
|
|
||
| See [ECOSYSTEM-OVERLAP.md](https://github.com/johanrd/html-validate-ember/blob/main/ECOSYSTEM-OVERLAP.md) |
Comment on lines
+19
to
+22
| # Needed so the job can push refreshed baselines back to the PR branch. | ||
| permissions: | ||
| contents: write | ||
|
|
Addresses Copilot review on #24: - explicitly gate the commit-back step on github.event.pull_request.head.repo.full_name == github.repository (fork PRs get a read-only token; makes the trust model explicit) - 'Glouped' -> 'Grouped' in ember-power-select triage note
Cowritten by Claude
# Conflicts: # package.json # vitest.config.ts
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.
What this adds
An ecosystem-CI harness that regression-tests the plugin against pinned snapshots of 12 real-world Ember repos (super-rentals, ember-website, ember-power-select, ember-modifier, ember-simple-auth, hds-design-system, ember-primitives, limber, ember-concurrency, vertical-collection, cardstack-ui-components, discourse).
ecosystem/run.ts— clones each target at a pinned SHA, installs its deps (so Glint type-aware extraction works), validates the templates, and diffs findings against committed baselines.ecosystem/baselines/*.json— the golden snapshot per target; CI flags drift.ecosystem/targets.json— pinned repos (SHA + globs + per-target options)..github/workflows/ecosystem.yml— the workflow.ecosystem/triage/**,ECOSYSTEM-OVERLAP.md— triage notes / draft upstream issues produced while reviewing findings.How it runs (opt-in)
It's a ~20-min clone-and-install job, so it does not run on every PR. Add the
run-ecosystem-cilabel to a PR (or dispatch manually) to run it. On drift it refreshes the baselines, commits them back to the PR for inline review, and fails the job. Seeecosystem/README.md.Does not change plugin behavior
The stylistic-rule suppressions (
void-style/no-inline-style/prefer-native-element) live in the ecosystem runner config (ECOSYSTEM_RULE_OVERRIDES), not the plugin's shipped presets — so this PR leaves:recommended/:gts-recommendeduntouched for users. Whether the plugin should adopt the same suppressions is proposed separately in #47.Labeling
Internal tooling — should get the
internalrelease-plan label (no user-facing changelog entry).