Skip to content

Extract rule: template-no-yield-block-params-to-else-inverse#2595

Open
NullVoxPopuli wants to merge 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-yield-block-params-to-else-inverse
Open

Extract rule: template-no-yield-block-params-to-else-inverse#2595
NullVoxPopuli wants to merge 1 commit intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-yield-block-params-to-else-inverse

Conversation

@NullVoxPopuli
Copy link
Contributor

Split from #2371.

Copy link

@NullVoxPopuli-ai-agent NullVoxPopuli-ai-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: template-no-yield-block-params-to-else-inverse (PR #2595)

Comparison with ember-template-lint no-yield-block-params-to-else-inverse.js

General correctness:

  1. Good: The rule correctly checks GlimmerMustacheStatement where node.path.original === 'yield', has params, and has a to hash pair with value 'else' or 'inverse'. This matches the original logic.

  2. Minor difference in hash.pairs access: The original accesses node.hash.pairs without a guard (ember-template-lint guarantees node.hash.pairs always exists as an array). The PR adds node.hash && node.hash.pairs guard, which is fine — defensive coding.

  3. Error message uses inline string instead of messages: The ERROR_MESSAGE constant is used via context.report({ message: ERROR_MESSAGE }) but the messages object in meta is empty (messages: {}). This works but is inconsistent — typically you'd either use messageId with a populated messages object, or use inline message. Since messages is empty, consider either removing the empty messages: {} or populating it and using messageId consistently. Not a blocker.

  4. Tests look comprehensive: Both gjs <template> and hbs modes are tested. Valid cases include yield without params + to="else" (correctly valid), and not-yield with params (correctly valid).

Scope analysis (gjs/gts):

The rule checks node.path.original === 'yield'. yield is a Glimmer keyword, not a component/helper that can be imported or shadowed by JS variables. No scope analysis needed.

Overall: Clean migration. Looks good.

🤖 Automated review comparing with ember-template-lint source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants