Extract rule: template-no-yield-to-default#2597
Conversation
NullVoxPopuli-ai-agent
left a comment
There was a problem hiding this comment.
Review: template-no-yield-to-default (PR #2597)
Comparison with ember-template-lint no-yield-to-default.js
General correctness:
-
Good coverage: The rule correctly handles both
yield to="default"(viaGlimmerMustacheStatement) and block-param keywords likehas-block "default",has-block-params "default",hasBlock "default",hasBlockParams "default"(via bothGlimmerMustacheStatementandGlimmerSubExpression). This matches the original's scope. -
Different error messages: The original uses a single message
'A block named "default" is not valid'for all cases. The PR uses two different messages:noYieldToDefaultfor the yield case andnoExplicitDefaultBlockfor the block-param-keyword case. This is actually an improvement in clarity — the two messages give more specific guidance. -
Report node difference: The original reports on
toHashPair(the hash pair node) for yield cases andtoParam(the string literal) for block-param cases. The PR reports on the entirenode(the MustacheStatement) for yield cases and the entirenodefor block-param cases. This means the error location/range will be different (broader). Not incorrect, but a deliberate choice worth noting. -
Tests are thorough: Both gjs and hbs modes tested. SubExpression cases (e.g.,
{{if (has-block "default")}}and{{#if (has-block "default")}}{{/if}}) are covered.
Scope analysis (gjs/gts):
yieldis a Glimmer keyword — cannot be shadowed.has-block,has-block-params,hasBlock,hasBlockParamsare Glimmer keywords — cannot be shadowed by JS imports.
No scope analysis needed.
Overall: Clean, thorough migration with improved error messages. Looks good.
🤖 Automated review comparing with ember-template-lint source
b192bef to
cb105f8
Compare
Split from #2371.