Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handlebars: Fix incorrect classic component syntax formatting #8593

Merged
merged 9 commits into from
Jun 24, 2020
Merged

Handlebars: Fix incorrect classic component syntax formatting #8593

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3cad8b2
Test: Add test addressing #8584
mikoscz Jun 18, 2020
1026749
Fix: Resolves problem with incorrect component formatting (#8584)
mikoscz Jun 18, 2020
a3a3486
Docs: Add changelog note
mikoscz Jun 18, 2020
44c215a
Test: Add test for yield formatting case
mikoscz Jun 19, 2020
7901dbe
Fix: Fix handlebars yield opening tag break
mikoscz Jun 19, 2020
cefe608
Test: Add case for any helper with params
mikoscz Jun 19, 2020
2d58523
Fix: Fix opening tag for every helper with params
mikoscz Jun 19, 2020
10e195d
Chore: Revert whitespaces
mikoscz Jun 19, 2020
8512568
Chore: Revert whitespaces in tests
mikoscz Jun 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 35 additions & 0 deletions changelog_unreleased/handlebars/pr-8593.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#### Fix formatting of classic components inside element ([#8593](https://github.com/prettier/prettier/pull/8593) by [@mikoscz](https://github.com/mikoscz)

<!-- prettier-ignore -->
```hbs
{{!-- Input --}}
<div>
{{classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>

{{!-- Prettier stable --}}
<div>
{{
classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>

{{!-- Prettier master --}}
<div>
{{classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>
```
8 changes: 7 additions & 1 deletion src/language-handlebars/printer-glimmer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,13 @@ function print(path, options, print) {
);
}
case "MustacheStatement": {
const shouldBreakOpeningMustache = isParentOfSomeType(path, [
const isParentOfSpecifiedTypes = isParentOfSomeType(path, [
"AttrNode",
"ConcatStatement",
"ElementNode",
]);
const shouldBreakOpeningMustache =
isParentOfSpecifiedTypes && doesNotHaveHashParams(n);
mikoscz marked this conversation as resolved.
Show resolved Hide resolved

return group(
concat([
Expand Down Expand Up @@ -649,6 +651,10 @@ function locationToOffset(source, line, column) {
}
}

function doesNotHaveHashParams(node) {
return node.hash.pairs.length === 0;
Copy link
Member

Choose a reason for hiding this comment

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

It is possible when node doesn't have hash or hash.pairs properites?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to glimmer MustacheStatement interface, this is not possible.
https://github.com/glimmerjs/glimmer-vm/blob/cba8afd3e665087ce9aa1457181b1955cfee91f0/packages/%40glimmer/syntax/lib/types/nodes.ts#L92
https://github.com/glimmerjs/glimmer-vm/blob/cba8afd3e665087ce9aa1457181b1955cfee91f0/packages/%40glimmer/syntax/lib/types/nodes.ts#L226

}

module.exports = {
print,
massageAstNode: clean,
Expand Down
60 changes: 50 additions & 10 deletions tests/handlebars/mustache-statement/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,30 @@ singleQuote: true
<div
class="a-first-class
{{if this.optionOne "optionOne"}}

{{if this.optionTwo "optionTwo"}}

{{if this.optionThree "optionThree"}}

{{if this.optionFour "optionFour"}}

{{if this.optionFive "optionFive"}}

{{this.class}}"
...attributes >
</div>

<div>
{{classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>



=====================================output=====================================
<GlimmerComponent
@progress={{
Expand Down Expand Up @@ -116,6 +127,15 @@ singleQuote: true
...attributes
>
</div>

<div>
{{classic-component-with-many-properties
class='hello'
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>
================================================================================
`;

Expand Down Expand Up @@ -155,19 +175,30 @@ printWidth: 80
<div
class="a-first-class
{{if this.optionOne "optionOne"}}

{{if this.optionTwo "optionTwo"}}

{{if this.optionThree "optionThree"}}

{{if this.optionFour "optionFour"}}

{{if this.optionFive "optionFive"}}

{{this.class}}"
...attributes >
</div>

<div>
{{classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>



=====================================output=====================================
<GlimmerComponent
@progress={{
Expand Down Expand Up @@ -234,6 +265,15 @@ printWidth: 80
...attributes
>
</div>

<div>
{{classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>
================================================================================
`;

Expand Down
21 changes: 16 additions & 5 deletions tests/handlebars/mustache-statement/basics.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,26 @@
<div
class="a-first-class
{{if this.optionOne "optionOne"}}

mikoscz marked this conversation as resolved.
Show resolved Hide resolved
{{if this.optionTwo "optionTwo"}}

{{if this.optionThree "optionThree"}}

{{if this.optionFour "optionFour"}}

{{if this.optionFive "optionFive"}}

{{this.class}}"
...attributes >
</div>

<div>
{{classic-component-with-many-properties
class="hello"
param=this.someValue
secondParam=this.someValue
thirdParam=this.someValue
}}
</div>