Skip to content

Commit c2de02b

Browse files
leaysgurleaysgur
committed
fix(formatter): decide embedded template layout from AST, not source shape (#27218)
Closes #21908 The layout of an embedded template (`` css`...` ``, etc) as a sole call argument or an arrow body was decided by its source shape. Since the content is rewritten, the same input could get different layouts depending on how it was formatted, and some cases were not idempotent. Now: - `embeddedLanguageFormatting: off`: verbatim template rule, as before - whitespace-only embedded templates are also now kept as-is, like Prettier - Otherwise, an embed target is detected from the AST (`embed_hug()`), regardless of whether its content formats - Sole argument: last-arg expansion (hug if the first line fits, otherwise break the args) - Arrow body: hug (except whitespace-sensitive HTML) NOTE: `openclaw` repo in ecosystem-ci report a lot of diffs like: ```ts // main (entry) => html`<li> ... </li>`, // This PR (entry) => html`<li> ... </li>`, ``` But this is the same behavior with Prettier 3.9.9.
1 parent c9e1224 commit c2de02b

23 files changed

Lines changed: 633 additions & 214 deletions

‎apps/oxfmt/DIVERGENCES.md‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,68 @@ const TomatoButton = Button.extend`
5454

5555
`Xxx.extend` / `Xxx.extend.attr(...)` (styled-components v3, removed in v4) is not recognized as a css-in-js tag,
5656
so its template stays verbatim; Prettier still formats it. Deprecated API, not worth extending the tag heuristic.
57+
58+
## embedded-template-short-argument
59+
60+
- Why: invariant
61+
- Pin: `conformance/fixtures/edge-cases/gql-in-js/embedded-template-short-argument.js`
62+
63+
```js
64+
/* input */
65+
const schema = graphql(`query{users{x}}`);
66+
67+
/* ours */
68+
const schema = graphql(`
69+
query {
70+
users {
71+
x
72+
}
73+
}
74+
`);
75+
76+
/* prettier */
77+
const schema =
78+
graphql(`
79+
query {
80+
users {
81+
x
82+
}
83+
}
84+
`);
85+
```
86+
87+
To decide whether an assignment breaks after `=`, Prettier checks if a sole template argument is short by its source text.
88+
An embedded template is rewritten, so that text says nothing about the output:
89+
the first pass breaks after `=`, and the second pass hugs, which is not a fixpoint.
90+
We treat an embedded template as never short, which gives Prettier's second-pass output (its fixpoint).
91+
92+
## embedded-template-invalid-content
93+
94+
- Why: uniform-rule (same construct, same output: the same template with valid content)
95+
- Pin: `conformance/fixtures/edge-cases/gql-in-js/embedded-template-invalid-content.js`
96+
97+
```js
98+
/* input */
99+
foo(
100+
gql`
101+
query {{{
102+
`
103+
);
104+
105+
/* ours */
106+
foo(gql`
107+
query {{{
108+
`);
109+
110+
/* prettier */
111+
foo(
112+
gql`
113+
query {{{
114+
`,
115+
);
116+
```
117+
118+
The layout of an embedded template as a sole argument or an arrow body is decided from the AST, whether its content formats or not.
119+
Prettier falls back to the source shape when the content fails to format, while it hugs the same template with valid content.
120+
The verbatim content cannot be re-indented, so its source indentation stays and may look misaligned once hugged (as with any verbatim template Prettier hugs).
121+
Under `embeddedLanguageFormatting: off`, the source shape still decides, like Prettier.

‎apps/oxfmt/conformance/fixtures/edge-cases/css-in-js/sole-argument-layout.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
foo(css`color: red;`);
2+
foo(css`
3+
color: red;
4+
`);
5+
foo(
6+
css`
7+
color: red;
8+
`
9+
);
10+
this.foo.bar(css`color: red;`);
11+
const a = () => css`color: red;`;
12+
const xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = foo(css`color: red;`);
13+
foo(css // comment
14+
`color: red;`);

‎apps/oxfmt/conformance/fixtures/edge-cases/gql-in-js/embedded-template-invalid-content.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
foo(
2+
gql`
3+
query {{{
4+
`
5+
);

‎apps/oxfmt/conformance/fixtures/edge-cases/gql-in-js/embedded-template-short-argument.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
const schema = graphql(`query{users{x}}`);

‎apps/oxfmt/conformance/fixtures/edge-cases/gql-in-js/sole-argument-layout.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
foo(gql`query Q { x }`);
2+
foo(gql`
3+
query Q { x }
4+
`);
5+
graphql(`query Q { x }`);
6+
graphql(
7+
`
8+
query Q { x }
9+
`
10+
);
11+
this.foo.bar(gql`query Q { x }`);
12+
const a = () => gql`query Q { x }`;
13+
const xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = graphql(
14+
/* GraphQL */ `
15+
query Q {
16+
x
17+
}
18+
`
19+
);

‎apps/oxfmt/conformance/fixtures/edge-cases/html-in-js/sole-argument-layout.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
foo(html` <div>a</div> `);
2+
foo(html`<div class="aaaaaaaaaaaaaaaa"><span>bbbbbbbbbbbbbbbbbbbbbbbb</span><span>cccccccccccccccccc</span></div> `);
3+
foo(html`<div class="aaaaaaaaaaaaaaaa"><span>bbbbbbbbbbbbbbbbbbbbbbbb</span><span>cccccccccccccccccc</span></div>
4+
`);
5+
foo(
6+
html`
7+
<div>a</div>
8+
`
9+
);
10+
const a = () => html` <div>a</div> `;
11+
const xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = foo(html` <div>a</div> `);

‎apps/oxfmt/conformance/fixtures/edge-cases/md-in-js/sole-argument-layout.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
foo(md`
2+
# hi
3+
`);
4+
foo(
5+
md`
6+
# hi
7+
`
8+
);
9+
const xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = foo(markdown`
10+
# hi
11+
`);

‎apps/oxfmt/conformance/fixtures/edge-cases/xxx-in-js-comment/sole-argument-layout.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
foo(/* GraphQL */ `query Q { x }`);
2+
foo(
3+
/* GraphQL */ `
4+
query Q { x }
5+
`
6+
);
7+
foo(/* HTML */ ` <div>a</div> `);
8+
const a = () => /* GraphQL */ `query Q { x }`;
9+
const xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx = foo(/* GraphQL */ `query Q { x }`);

‎apps/oxfmt/conformance/snapshots/conformance.snap.md‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,31 @@
2626

2727
## gql-in-js
2828

29-
### Option 1: 12/12 (100.00%)
29+
### Option 1: 13/15 (86.67%)
3030

3131
```json
3232
{"printWidth":80}
3333
```
3434

35-
### Option 2: 12/12 (100.00%)
35+
- [edge-cases/gql-in-js/embedded-template-invalid-content.js](diffs/gql-in-js/edge-cases__gql-in-js__embedded-template-invalid-content.js.md)
36+
- apps/oxfmt/DIVERGENCES.md#embedded-template-invalid-content
37+
- [edge-cases/gql-in-js/embedded-template-short-argument.js](diffs/gql-in-js/edge-cases__gql-in-js__embedded-template-short-argument.js.md)
38+
- apps/oxfmt/DIVERGENCES.md#embedded-template-short-argument
39+
40+
### Option 2: 13/15 (86.67%)
3641

3742
```json
3843
{"printWidth":100}
3944
```
4045

46+
- [edge-cases/gql-in-js/embedded-template-invalid-content.js](diffs/gql-in-js/edge-cases__gql-in-js__embedded-template-invalid-content.js.md)
47+
- apps/oxfmt/DIVERGENCES.md#embedded-template-invalid-content
48+
- [edge-cases/gql-in-js/embedded-template-short-argument.js](diffs/gql-in-js/edge-cases__gql-in-js__embedded-template-short-argument.js.md)
49+
- apps/oxfmt/DIVERGENCES.md#embedded-template-short-argument
50+
4151
## css-in-js
4252

43-
### Option 1: 19/21 (90.48%)
53+
### Option 1: 20/22 (90.91%)
4454

4555
```json
4656
{"printWidth":80}
@@ -51,7 +61,7 @@
5161
- [externals/prettier/js/multiparser-css/styled-components.js](diffs/css-in-js/externals__prettier__js__multiparser-css__styled-components.js.md)
5262
- apps/oxfmt/DIVERGENCES.md#styled-extend-tag
5363

54-
### Option 2: 19/21 (90.48%)
64+
### Option 2: 20/22 (90.91%)
5565

5666
```json
5767
{"printWidth":100}
@@ -64,7 +74,7 @@
6474

6575
## html-in-js
6676

67-
### Option 1: 173/197 (87.82%)
77+
### Option 1: 174/198 (87.88%)
6878

6979
```json
7080
{"printWidth":80}
@@ -119,7 +129,7 @@
119129
- [externals/webawesome/textarea/textarea.ts](diffs/html-in-js/externals__webawesome__textarea__textarea.ts.md)
120130
- crates/oxc_formatter/DIVERGENCES.md#union-annotation-flat-retry
121131

122-
### Option 2: 187/197 (94.92%)
132+
### Option 2: 188/198 (94.95%)
123133

124134
```json
125135
{"printWidth":100,"htmlWhitespaceSensitivity":"ignore"}
@@ -162,27 +172,27 @@
162172

163173
## md-in-js
164174

165-
### Option 1: 8/8 (100.00%)
175+
### Option 1: 9/9 (100.00%)
166176

167177
```json
168178
{"printWidth":80}
169179
```
170180

171-
### Option 2: 8/8 (100.00%)
181+
### Option 2: 9/9 (100.00%)
172182

173183
```json
174184
{"printWidth":100,"proseWrap":"always"}
175185
```
176186

177187
## xxx-in-js-comment
178188

179-
### Option 1: 6/6 (100.00%)
189+
### Option 1: 7/7 (100.00%)
180190

181191
```json
182192
{"printWidth":80}
183193
```
184194

185-
### Option 2: 6/6 (100.00%)
195+
### Option 2: 7/7 (100.00%)
186196

187197
```json
188198
{"printWidth":100}

‎apps/oxfmt/conformance/snapshots/diffs/gql-in-js/edge-cases__gql-in-js__embedded-template-invalid-content.js.md‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
# edge-cases/gql-in-js/embedded-template-invalid-content.js
2+
3+
## Option 1
4+
5+
`````json
6+
{"printWidth":80}
7+
`````
8+
9+
### Diff
10+
11+
`````diff
12+
===================================================================
13+
--- prettier
14+
+++ oxfmt
15+
@@ -1,5 +1,3 @@
16+
-foo(
17+
- gql`
18+
+foo(gql`
19+
query {{{
20+
- `,
21+
-);
22+
+ `);
23+
24+
`````
25+
26+
### Actual (oxfmt)
27+
28+
`````js
29+
foo(gql`
30+
query {{{
31+
`);
32+
33+
`````
34+
35+
### Expected (prettier)
36+
37+
`````js
38+
foo(
39+
gql`
40+
query {{{
41+
`,
42+
);
43+
44+
`````
45+
46+
## Option 2
47+
48+
`````json
49+
{"printWidth":100}
50+
`````
51+
52+
### Diff
53+
54+
`````diff
55+
===================================================================
56+
--- prettier
57+
+++ oxfmt
58+
@@ -1,5 +1,3 @@
59+
-foo(
60+
- gql`
61+
+foo(gql`
62+
query {{{
63+
- `,
64+
-);
65+
+ `);
66+
67+
`````
68+
69+
### Actual (oxfmt)
70+
71+
`````js
72+
foo(gql`
73+
query {{{
74+
`);
75+
76+
`````
77+
78+
### Expected (prettier)
79+
80+
`````js
81+
foo(
82+
gql`
83+
query {{{
84+
`,
85+
);
86+
87+
`````

0 commit comments

Comments
 (0)