Extract rule: template-no-whitespace-for-layout#2592
Extract rule: template-no-whitespace-for-layout#2592NullVoxPopuli wants to merge 1 commit intoember-cli:masterfrom
Conversation
NullVoxPopuli-ai-agent
left a comment
There was a problem hiding this comment.
Review: template-no-whitespace-for-layout
Compared against the original ember-template-lint rule no-whitespace-for-layout.
What's done well
- Logic is a faithful port. The line-by-line splitting, trimming, and regex check for two consecutive spaces or
entities matches the original exactly. - The regex is identical to the original:
/(( )|( ))(( )|( ))/. - All original test cases are ported -- both good and bad cases match 1:1.
- Both gjs and hbs parser modes are tested.
- Additional gjs-specific valid cases like multiline
<div>with attributes are a nice addition.
Observations and potential issues
-
Error message differs slightly. The original uses
"Excess whitespace detected."while the new rule uses"Unexpected use of whitespace for layout. Use CSS for spacing instead of multiple spaces."The new message is more descriptive and actionable, which is an improvement. Just noting the difference. -
Uses
context.getSourceCode()(deprecated in ESLint 9+). The rule callscontext.getSourceCode()without thecontext.sourceCodefallback pattern used in some of the other PRs in this series. Consider usingconst sourceCode = context.sourceCode || context.getSourceCode();for consistency and forward compatibility, as done in PR #2591. -
Uses
.test()instead of.match(). The original uses.match()and checks for!== null, while the new rule uses.test(). This is actually a minor improvement --.test()is more semantically correct and slightly more performant when you only need a boolean result. -
fixableis not set (implicitlynull), matching the original which also doesn't provide autofix. Consistent. -
No handling of
entities in the valid test forStart to finish. This is ported from the original and tests that a single between words is allowed (only two consecutive whitespace characters trigger the rule).
Summary
Very clean, nearly line-for-line port. The only things to consider are the getSourceCode() deprecation and whether the more verbose error message is intentional (it's a good change, just different from the original).
Automated review comparing with ember-template-lint source
Split from #2371.