fix(template-no-yield-only): filter comments/whitespace before length check - #4
Open
NullVoxPopuli-ai-agent wants to merge 1 commit into
Open
fix(template-no-yield-only): filter comments/whitespace before length check#4NullVoxPopuli-ai-agent wants to merge 1 commit into
NullVoxPopuli-ai-agent wants to merge 1 commit into
Conversation
… check
ember-eslint-parser 0.11 keeps `{{! ... }}` and `<!-- ... -->` nodes in the
template body so that `template-no-html-comments` and `{{! eslint-disable }}`
inline-config scanning continue to work. Rules that count "meaningful"
children must now filter them explicitly, as `template-no-bare-yield`
already does.
- Extract `isEmptyNode` (whitespace-only GlimmerTextNode + both comment
node types) to a new `lib/utils/glimmer-ast.js`, with a docstring
documenting the parser contract.
- Apply it in `template-no-yield-only` before the `length === 1` check.
Fixes the two `{{! ... }}` invalid-case regressions.
- Replace the inline copy of the same helper in `template-no-bare-yield`
with the shared util.
- Unit-test the util.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
🏎️ Benchmark Comparison
Full mitata output |
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
2 participants
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.
Fix proposed for ember-cli#2735, which is currently failing on
build (ubuntu, 24.x)with twotemplate-no-yield-onlyinvalid-case failures (the ones containing{{! some comment }}).Why it's failing
ember-eslint-parser@0.11(viaember-estree@0.6.x) intentionally keepsGlimmerMustacheCommentStatementandGlimmerCommentStatementnodes in their parent'sbody/children. Whitespace-onlyGlimmerTextNodes still get stripped, but comments don't — because:template-no-html-commentsflags HTML comments viasourceCode.getAllComments(), which is populated by an AST-traversal-based collector inember-eslint-parser. If comments were stripped frombodyupstream inember-estree, the collector would never see them →ast.commentsempties → that rule silently stops working, and{{! eslint-disable }}/<!-- eslint-disable -->inline directives in templates also stop working. Verified by patchingremoveFromParent(comments)inember-estree/src/transforms.js— 6 other tests then break.template-block-indentation,template-table-groups,template-no-bare-yieldalready handle in-body comment nodes explicitly.So the parser contract is: comments live in
bodyAND inast.comments(template-lint tradition, not ESLint-strict). Rules counting structural children must filter.template-no-bare-yieldalready does — it has a localisEmptyNode(node)helper.template-no-yield-onlydidn't.What this PR does
isEmptyNodetolib/utils/glimmer-ast.js— whitespace-onlyGlimmerTextNode+ both comment node types, with a docstring pinning down the parser contract so future rule authors know.template-no-yield-only— filter ignorable nodes before thelength === 1gate. The four existing invalid cases (including the two{{! ... }}ones) all pass in both gjs and hbs modes.template-no-bare-yield— drop its inline copy of the helper, import from the shared util.Verification
npx vitest run→ 9107 / 9107 pass (was 9099 / 9101 onnvp/update-parsertip).pnpm lint:prettierclean; no lint errors on the changed files.How to pull into ember-cli#2735
Latent issue (out of scope, separate fix)
template-no-only-default-slot.js:34doesparent.children.length === 1and would also silently miss detection when an adjacent{{! comment }}is present. No test covers that case today. Worth a follow-up.🤖 Generated with Claude Code