Skip to content

fix(printer-glimmer): preserve path literal segments #15605

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

Merged
merged 5 commits into from
Dec 13, 2023
Merged

fix(printer-glimmer): preserve path literal segments #15605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ab54670
fix(glimmer): preserve path literal segments
maxpowa Nov 8, 2023
90de7c0
chore: add changelog
maxpowa Nov 8, 2023
84228dc
chore: fix formatting
maxpowa Nov 14, 2023
740b789
Extract printPathExpression
fisker Nov 27, 2023
40fd225
Update changelog
fisker Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions changelog_unreleased/handlebars/15605.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#### Preserve path literal segments (#15605 by @maxpowa)

Fixes scenarios where an input handlebars file containing literal segments would be reformatted to unwrap the literal segments, causing syntax errors in the resulting output.

<!-- prettier-ignore -->
```hbs
<!-- Input -->
{{input.[funky<api!response]}}
{{input.[this one has spaces]}}
{{input.[anotherone].[0]}}

<!-- Prettier stable -->
{{input.funky<api!response}}
{{input.this one has spaces}}
{{input.anotherone.0}}

<!-- Prettier main -->
{{input.[funky<api!response]}}
{{input.[this one has spaces]}}
{{input.anotherone.[0]}}
```
38 changes: 37 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 @@ -366,7 +366,7 @@ function print(path, options, print) {
];
}
case "PathExpression":
return node.original;
return printPathExpression(node);

case "BooleanLiteral":
return String(node.value);
Expand Down Expand Up @@ -764,6 +764,42 @@ function printBlockParams(node) {
return ["as |", node.blockParams.join(" "), "|"];
}

// https://handlebarsjs.com/guide/expressions.html#literal-segments
const PATH_EXPRESSION_FORBIDDEN_CHARACTERS = new Set(
"!\"#%&'()*+,./;<=>@[\\]^`{|}~",
);
const PATH_EXPRESSION_FORBIDDEN_IN_FIRST_PART = new Set([
"true",
"false",
"null",
"undefined",
]);
const isPathExpressionPartNeedBrackets = (part, index) =>
(index !== 0 && PATH_EXPRESSION_FORBIDDEN_IN_FIRST_PART.has(part)) ||
/\s/.test(part) ||
/^\d/.test(part) ||
Array.prototype.some.call(part, (character) =>
PATH_EXPRESSION_FORBIDDEN_CHARACTERS.has(character),
);
function printPathExpression(node) {
if (node.data || (node.parts.length === 1 && node.original.includes("/"))) {
// check if node has data, or
// check if node is a legacy path expression (and leave it alone)
return node.original;
}

let { parts } = node;
if (node.this) {
parts = ["this", ...parts];
}

return parts
.map((part, index) =>
isPathExpressionPartNeedBrackets(part, index) ? `[${part}]` : part,
)
.join(".");
}

const printer = {
print,
massageAstNode: clean,
Expand Down
119 changes: 119 additions & 0 deletions tests/format/handlebars/path-expressions/__snapshots__/jsfmt.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`keyword-expressions.hbs format 1`] = `
====================================options=====================================
parsers: ["glimmer"]
printWidth: 80
| printWidth
=====================================input======================================
<div>
{{true.foo}}
{{foo.[true]}}
{{false.foo}}
{{foo.[false]}}
{{null.foo}}
{{foo.[null]}}
{{undefined.foo}}
{{foo.[undefined]}}
</div>

=====================================output=====================================
<div>
{{true.foo}}
{{foo.[true]}}
{{false.foo}}
{{foo.[false]}}
{{null.foo}}
{{foo.[null]}}
{{undefined.foo}}
{{foo.[undefined]}}
</div>
================================================================================
`;

exports[`literal-expressions.hbs format 1`] = `
====================================options=====================================
parsers: ["glimmer"]
printWidth: 80
| printWidth
=====================================input======================================
<div>
{{array.[true]}}
{{[dot.]}}
{{array.2.[@#].[1]}}
{{array.2.[a b]}}
{{this.test}}
{{#if @foo}}
{{/if}}
<!-- legacy path format, which glimmer does not parse - we don't want to wrap with brackets -->
{{foo/bar/baz ab=cd}}
</div>

=====================================output=====================================
<div>
{{array.[true]}}
{{[dot.]}}
{{array.[2].[@#].[1]}}
{{array.[2].[a b]}}
{{this.test}}
{{#if @foo}}{{/if}}
<!-- legacy path format, which glimmer does not parse - we don't want to wrap with brackets -->
{{foo/bar/baz ab=cd}}
</div>
================================================================================
`;

exports[`numeric-expressions.hbs format 1`] = `
====================================options=====================================
parsers: ["glimmer"]
printWidth: 80
| printWidth
=====================================input======================================
<div>
{{people.[1].name}}
{{people.[1]}}
{{array.[1].barray.[13]}}
{{array.[1234]}}
{{array.[123].[item-class]}}
{{array.[12].item}}
{{people.[1a]}}
</div>

<ul class="people_list">
{{#each people}}
<li>{{array.[2]}}</li>
{{/each}}
</ul>

<div>
{{#if @foo}}
{{bar.[1]}}
{{bar.1.name}}
{{/if}}
</div>

=====================================output=====================================
<div>
{{people.[1].name}}
{{people.[1]}}
{{array.[1].barray.[13]}}
{{array.[1234]}}
{{array.[123].item-class}}
{{array.[12].item}}
{{people.[1a]}}
</div>

<ul class="people_list">
{{#each people}}
<li>{{array.[2]}}</li>
{{/each}}
</ul>

<div>
{{#if @foo}}
{{bar.[1]}}
{{bar.[1].name}}
{{/if}}
</div>
================================================================================
`;
1 change: 1 addition & 0 deletions tests/format/handlebars/path-expressions/jsfmt.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
run_spec(import.meta, ["glimmer"]);
10 changes: 10 additions & 0 deletions tests/format/handlebars/path-expressions/keyword-expressions.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div>
{{true.foo}}
{{foo.[true]}}
{{false.foo}}
{{foo.[false]}}
{{null.foo}}
{{foo.[null]}}
{{undefined.foo}}
{{foo.[undefined]}}
</div>
11 changes: 11 additions & 0 deletions tests/format/handlebars/path-expressions/literal-expressions.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div>
{{array.[true]}}
{{[dot.]}}
{{array.2.[@#].[1]}}
{{array.2.[a b]}}
{{this.test}}
{{#if @foo}}
{{/if}}
<!-- legacy path format, which glimmer does not parse - we don't want to wrap with brackets -->
{{foo/bar/baz ab=cd}}
</div>
22 changes: 22 additions & 0 deletions tests/format/handlebars/path-expressions/numeric-expressions.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<div>
{{people.[1].name}}
{{people.[1]}}
{{array.[1].barray.[13]}}
{{array.[1234]}}
{{array.[123].[item-class]}}
{{array.[12].item}}
{{people.[1a]}}
</div>

<ul class="people_list">
{{#each people}}
<li>{{array.[2]}}</li>
{{/each}}
</ul>

<div>
{{#if @foo}}
{{bar.[1]}}
{{bar.1.name}}
{{/if}}
</div>