Skip to content

fix: ignore comment + whitespace nodes in template-no-yield-only#2736

Merged
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
johanrd:fix/template-no-yield-only-filter-comments
Apr 24, 2026
Merged

fix: ignore comment + whitespace nodes in template-no-yield-only#2736
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
johanrd:fix/template-no-yield-only-filter-comments

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 24, 2026

Cowritten by claude:

Summary

  • template-no-yield-only flags yield-only templates via templateNodes.length === 1 && isYieldOnly(templateNodes[0]). That holds only while the parser strips template comments out of the body, which [email protected] happened to do.
  • Upstream [email protected] (NullVoxPopuli/ember-estree#31) started keeping MustacheCommentStatement nodes in the body (the native Glimmer AST shape). Under that parser a template like <template>{{! x }}{{yield}}</template> has a body of length 2 and is silently no longer flagged. This is the rule failure that blocks Update ember-eslint-parser to 0.11 #2735's CI.
  • Fix: filter comment + whitespace-only text nodes before the length check, via the same isEmptyNode helper that upstream ember-template-lint and the sibling rule template-no-bare-yield.js already use.

Why fix it here rather than in the parser / estree

[email protected]'s change is the correct AST: it matches the pure-hbs parser (ember-eslint-parser/hbs) and the native Glimmer AST, both of which have always kept comment nodes in the template body. Re-hiding those nodes at the parser layer would re-introduce the bug that #2733 guards against ({{! eslint-disable-* }} directives would again be invisible to tooling that needs to see them). The semantic decision "should a comment-plus-yield template count as yield-only?" belongs to the rule.

What this does not fix

Latent bug still remain after this PR (candidates for follow-up)

  1. {{yield to="inverse"}} is still wrongly flagged. The initial Extract rule: template-no-yield-only #2596 commit had (!node.hash || !node.hash.pairs || node.hash.pairs.length === 0) guarding against this — the AI correctly called it "an
    improvement" over upstream. The sync commit dropped it. A template whose only body is {{yield to="inverse"}} is not truly "yield-only" — it's explicitly delegating to the else / named <:inverse> block, which is
    the idiomatic way to do that. Upstream has this bug; we inherited it on the sync commit; we could diverge better.

separate follow-up?

Notes

  • template-no-bare-yield.js and template-no-yield-only.js are near-duplicate ports of ember-template-lint/lib/rules/no-yield-only.js; only the former had kept pace with upstream's empty-node filter. This closes that drift.
  • The filter is a no-op on the currently-pinned [email protected], so this is safe to land independently of Update ember-eslint-parser to 0.11 #2735 and paves the way for that PR's CI to go green once rebased.
  • This also fixes a latent bug the rule had on 0.10: <!-- html -->{{yield}} was not being flagged as yield-only; now it is.

Test plan

  • pnpm test tests/lib/rules/template-no-yield-only.js — 22 passed (18 original + 4 new assertions: {{!-- long --}}{{yield}} and <!-- x -->{{yield}} against both the gjs and hbs parsers)
  • npx prettier --check + npx eslint clean on both modified files
  • CI green

The rule flagged yield-only templates by asserting
`templateNodes.length === 1 && isYieldOnly(templateNodes[0])`. That
works only while the parser strips template comments out of the body,
which ember-estree 0.4.2 happened to do. Upstream ember-estree 0.4.3
(#31) began keeping MustacheCommentStatement nodes in the body, so a
template like

    <template>{{! some comment }}{{yield}}</template>

now yields a body of length 2 and silently stops being flagged — which
is also why PR ember-cli#2735 (upgrade to ember-eslint-parser 0.11) fails this
rule's CI.

Ignoring comment and whitespace-only text nodes before the length check
aligns with:

- upstream ember-template-lint's `no-yield-only.js` (unchanged there
  since comments-in-body is the native Glimmer AST shape anyway)
- the sibling rule `template-no-bare-yield.js`, which already uses the
  same `isEmptyNode` filter

The fix is a no-op on the currently-pinned [email protected] and
fixes the rule under any parser version that preserves comment nodes in
the template body. The rule also now correctly catches yield-only
templates that contain HTML comments (`<!-- x -->{{yield}}`), which it
was silently missing before.
@NullVoxPopuli NullVoxPopuli merged commit c4e5929 into ember-cli:master Apr 24, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants