Skip to content

Extract rule: template-no-down-event-binding#2418

Merged
NullVoxPopuli merged 3 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-down-event-binding
Mar 13, 2026
Merged

Extract rule: template-no-down-event-binding#2418
NullVoxPopuli merged 3 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-down-event-binding

Conversation

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

Split from #2371.

@NullVoxPopuli NullVoxPopuli marked this pull request as draft February 18, 2026 00:24
@NullVoxPopuli
Copy link
Copy Markdown
Contributor Author

TODO: this rule isn't correct

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-down-event-binding branch 2 times, most recently from 571a39b to 769f2e3 Compare March 6, 2026 00:32
@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-down-event-binding branch from 769f2e3 to c23e330 Compare March 10, 2026 22:26
@johanrd
Copy link
Copy Markdown
Contributor

johanrd commented Mar 12, 2026

Claude Review: comparing against ember-template-lint no-pointer-down-event-binding

Wrong event set. The rule targets mousedown + touchstart, but the original targets mousedown, onmousedown, pointerdown, onpointerdown. touchstart is not part of the original rule — remove it.
Add pointerdown and onpointerdown (they're the reason the original is called no-pointer-down-event-binding).

// current                                                                                                                                                                                                         
const DOWN_EVENTS = new Set(['mousedown', 'touchstart']);                                                                                                                                                          
                                                                                                                                                                                                                   
// should be                                                                                                                                                                                                       
const DOWN_EVENTS = new Set(['mousedown', 'onmousedown', 'pointerdown', 'onpointerdown']);                                                                                                                         
                                                                                                                                                                                                                   
Missing {{action}} modifier handling. The original detects <div {{action this.handler on="mousedown"}}> via the on hash pair. This PR only handles {{on}} modifiers and HTML attributes. Add an {{action}} check:  
                                                                                                                                                                                                                   
// inside the modifier loop:                                                                                                                                                                                       
if (modifier.path.original === 'action') {
  const onPair = modifier.hash?.pairs?.find(p => p.key === 'on');
  if (onPair && DOWN_EVENTS.has(onPair.value.value?.toLowerCase())) {                                                                                                                                              
    context.report({ node: modifier, messageId: 'unexpected' });                                                                                                                                                   
  }                                                                                                                                                                                                                
}                                                                                                                                                                                                                  

Missing case-insensitive matching. The original calls .toLowerCase() before checking the set. Add .toLowerCase() to both the {{on}} event param check and the attribute name check.

Attribute check is fragile. attr.name.replace('on', '') replaces the first occurrence of "on" anywhere in the string, not just the prefix. Use instead:

if (attr.name.startsWith('on') && DOWN_EVENTS.has(attr.name.slice(2).toLowerCase())) {

Error message diverges. The original says "Avoid binding to a pointer down event; bind to a pointer up event instead". The PR recommends click/keydown which is different guidance. Suggest matching the original
message.

Docs should also be updated to match — remove touchstart references, add pointerdown examples, fix the recommended alternatives from click/keydown to mouseup/pointerup.

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-down-event-binding branch from c23e330 to 5486047 Compare March 13, 2026 19:55
@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review March 13, 2026 20:20
@NullVoxPopuli NullVoxPopuli merged commit 1c9a858 into ember-cli:master Mar 13, 2026
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-no-down-event-binding branch March 13, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants