Post-merge-review: Fix template-no-passed-in-event-handlers: correct event list, fix ignore-config format, broaden component detection#2662
Open
johanrd wants to merge 1 commit intoember-cli:masterfrom
Conversation
…e config with upstream - Remove mouseMove, mouseEnter, mouseLeave from event list (not in upstream) - Use bare names (without @) in ignore config for angle bracket invocations - Remove unnecessary PascalCase gate on ElementNode visitor
be7fce9 to
050d2bf
Compare
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
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.
What's broken on
masterTwo independent issues:
ignoreconfig format broken for angle-bracket invocations. The port comparesignoredAttrs.includes(attr.name)whereattr.name === '@click'. The docs say to configure{ Foo: ['click'] }(bare name, no@) — but with the current code that never matches. Upstream slices the@off first (no-passed-in-event-handlers.jsL107–112) and explicitly rejects@-prefixed ignore entries as invalid config (L58)./^[A-Z]/.test(node.tag)gate silently skips<this.X>,<@x>, and<ns.X>— all valid GTS component invocations per the template tag format guide. The port needs some HTML-vs-component discrimination because it has no JS scope tracker.Fix
argName(the@-stripped form).@OR starts withthis.OR contains.. Lowercase/kebab-case tags (<my-button>) are treated as HTML — correct in GTS (imports can't have dashes) and a reasonable approximation in HBS (@arg={{handler}}on a true HTML element is nonsensical).Test plan
<this.MyComponent>,<@someComponent>,<ns.Widget>fail on master (PascalCase gate silently skips them)<my-button>/<custom-el>confirm HTML elements are not checked['@click']to['click'](the documented format). All 8 fail on master with the format fix reverted.Co-written by Claude.