Skip to content

Commit 1a44f66

Browse files
gitKrystangitKrystan
committed
Fix bug where preprocess caused syntax errors in component classes
1 parent fc4d756 commit 1a44f66

8 files changed

Lines changed: 335 additions & 28 deletions

File tree

‎src/parse.ts‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
isRawGlimmerClassProperty,
3030
} from './types/raw';
3131
import { hasAmbiguousNextLine } from './utils/ambiguity';
32+
import { assert } from './utils/index';
3233

3334
const typescript = babelParsers['babel-ts'] as Parser<BaseNode>;
3435

@@ -53,24 +54,7 @@ const preprocess: Required<Parser<BaseNode>>['preprocess'] = (
5354
relativePath: options.filepath,
5455
}).output;
5556

56-
const placeholderOpen = `[${TEMPLATE_TAG_PLACEHOLDER}`; // intentionally missing ]
57-
const sugaredDefaultExport = new RegExp(`^\\s*\\(?\\s*\\${placeholderOpen}`);
58-
const desugaredDefaultExport = `export default ${placeholderOpen}`;
59-
return preprocessed
60-
.split(/\r?\n/)
61-
.map((line, index, array) => {
62-
const previousLine = findPreviousLine(index, array);
63-
if (
64-
previousLine &&
65-
(previousLine.includes('prettier-ignore') ||
66-
previousLine.trim().endsWith('{'))
67-
) {
68-
return line;
69-
} else {
70-
return line.replace(sugaredDefaultExport, desugaredDefaultExport);
71-
}
72-
})
73-
.join('\r\n');
57+
return desugarDefaultExportTemplates(preprocessed);
7458
};
7559

7660
export const parser: Parser<BaseNode> = {
@@ -164,15 +148,50 @@ export const parser: Parser<BaseNode> = {
164148
},
165149
};
166150

167-
function findPreviousLine(index: number, array: string[]): string | undefined {
168-
const previousIndex = index - 1;
169-
const previousLine = array[previousIndex];
151+
/**
152+
* Desugar template tag default exports because they parse as
153+
* ExpressionStatement, which has a bunch of irrelevant custom semicolon
154+
* handling in Prettier that is very difficult to undo. We can optionally
155+
* re-sugar on print. See `templateExportDefault` option.
156+
*
157+
* HACK: An attempt was made to do this via babel transforms but it destroyed
158+
* Prettier's newline preservation logic.
159+
*/
160+
function desugarDefaultExportTemplates(preprocessed: string): string {
161+
const placeholderOpen = `[${TEMPLATE_TAG_PLACEHOLDER}`; // intentionally missing ]
162+
163+
// ^\s*(\()?\s*\[__GLIMMER_TEMPLATE
164+
const sugaredDefaultExport = new RegExp(
165+
`^\\s*(\\()?\\s*\\${placeholderOpen}`
166+
);
167+
const desugaredDefaultExport = `export default $1${placeholderOpen}`;
168+
169+
const lines = preprocessed.split(/\r?\n/);
170+
const desugaredLines: string[] = [];
171+
let previousLine: string | null = null;
172+
let blockLevel = 0;
173+
174+
for (let line of lines) {
175+
if (line.includes('{')) {
176+
blockLevel++;
177+
}
170178

171-
if (previousLine === undefined) {
172-
return undefined;
173-
} else if (previousLine.length) {
174-
return previousLine;
175-
} else {
176-
return findPreviousLine(previousIndex, array);
179+
if (line.includes('}')) {
180+
blockLevel--;
181+
}
182+
183+
assert('expected non-negative blockLevel', blockLevel > -1);
184+
185+
if (!previousLine?.includes('prettier-ignore') && blockLevel === 0) {
186+
line = line.replace(sugaredDefaultExport, desugaredDefaultExport);
187+
}
188+
189+
desugaredLines.push(line);
190+
191+
if (line.trim().length) {
192+
previousLine = line;
193+
}
177194
}
195+
196+
return desugaredLines.join('\r\n');
178197
}

‎src/print/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ export function definePrinter(options: Options): void {
8181
(doc) => typeof doc !== 'string' || doc !== ';'
8282
);
8383

84-
// FIXME: Make configurable
8584
if (
8685
!options.templateExportDefault &&
8786
docMatchesString(adjusted[0], 'export') &&

‎tests/cases/gjs/component-class-with-content-before-template.gjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import Component from '@glimmer/component';
2+
3+
/** It's a component */
4+
class MyComponent
5+
extends Component {
6+
get whatever() {}
7+
8+
<template>
9+
10+
11+
<h1> Class top level template. Class top level template. Class top level template. Class top level template. Class top level template. </h1>
12+
</template>
13+
/*AMBIGUOUS*/
14+
}

‎tests/unit-tests/__snapshots__/format.test.ts.snap‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,23 @@ class MyComponent extends Component {
1515
"
1616
`;
1717

18+
exports[`format > config > default > it formats ../cases/gjs/component-class-with-content-before-template.gjs 1`] = `
19+
"import Component from \\"@glimmer/component\\";
20+
21+
/** It's a component */
22+
class MyComponent extends Component {
23+
get whatever() {}
24+
25+
<template>
26+
<h1>
27+
Class top level template. Class top level template. Class top level
28+
template. Class top level template. Class top level template.
29+
</h1>
30+
</template>
31+
}
32+
"
33+
`;
34+
1835
exports[`format > config > default > it formats ../cases/gjs/component-class-with-template-literal.gjs 1`] = `
1936
"import Component from \\"@glimmer/component\\";
2037

‎tests/unit-tests/ambiguous/__snapshots__/index.test.ts.snap‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1176,6 +1176,24 @@ class MyComponent extends Component {
11761176
"
11771177
`;
11781178
1179+
exports[`ambiguous > config > default > <template>oops</template> > with semi > it formats ../cases/gjs/component-class-with-content-before-template.gjs 1`] = `
1180+
"import Component from \\"@glimmer/component\\";
1181+
1182+
/** It's a component */
1183+
class MyComponent extends Component {
1184+
get whatever() {}
1185+
1186+
<template>
1187+
<h1>
1188+
Class top level template. Class top level template. Class top level
1189+
template. Class top level template. Class top level template.
1190+
</h1>
1191+
</template>
1192+
<template>oops</template>
1193+
}
1194+
"
1195+
`;
1196+
11791197
exports[`ambiguous > config > default > <template>oops</template> > with semi > it formats ../cases/gjs/default-export.gjs 1`] = `
11801198
"<template>
11811199
Explicit default export module top level component. Explicit default export
@@ -1451,6 +1469,40 @@ export interface Signature {
14511469
"
14521470
`;
14531471
1472+
exports[`ambiguous > config > default > <template>oops</template> > without semi > it formats ../cases/gjs/component-class.gjs 1`] = `
1473+
"import Component from \\"@glimmer/component\\";
1474+
1475+
/** It's a component */
1476+
class MyComponent extends Component {
1477+
<template>
1478+
<h1>
1479+
Class top level template. Class top level template. Class top level
1480+
template. Class top level template. Class top level template.
1481+
</h1>
1482+
</template>
1483+
<template>oops</template>
1484+
}
1485+
"
1486+
`;
1487+
1488+
exports[`ambiguous > config > default > <template>oops</template> > without semi > it formats ../cases/gjs/component-class-with-content-before-template.gjs 1`] = `
1489+
"import Component from \\"@glimmer/component\\";
1490+
1491+
/** It's a component */
1492+
class MyComponent extends Component {
1493+
get whatever() {}
1494+
1495+
<template>
1496+
<h1>
1497+
Class top level template. Class top level template. Class top level
1498+
template. Class top level template. Class top level template.
1499+
</h1>
1500+
</template>
1501+
<template>oops</template>
1502+
}
1503+
"
1504+
`;
1505+
14541506
exports[`ambiguous > config > default > <template>oops</template> > without semi > it formats ../cases/gjs/default-export.gjs 1`] = `
14551507
"<template>
14561508
Explicit default export module top level component. Explicit default export
@@ -1519,6 +1571,30 @@ exports[`ambiguous > config > default > <template>oops</template> > without semi
15191571
"
15201572
`;
15211573
1574+
exports[`ambiguous > config > default > <template>oops</template> > without semi > it formats ../cases/gts/component-class.gts 1`] = `
1575+
"import Component from \\"@glimmer/component\\";
1576+
1577+
export interface Signature {
1578+
Element: HTMLElement;
1579+
Args: {
1580+
myArg: string;
1581+
};
1582+
Yields: [];
1583+
}
1584+
1585+
/** It's a component */
1586+
class MyComponent extends Component<Signature> {
1587+
<template>
1588+
<h1>
1589+
Class top level template. Class top level template. Class top level
1590+
template. Class top level template. Class top level template.
1591+
</h1>
1592+
</template>
1593+
<template>oops</template>
1594+
}
1595+
"
1596+
`;
1597+
15221598
exports[`ambiguous > config > default > <template>oops</template> > without semi > it formats ../cases/gts/default-export.gts 1`] = `
15231599
"import type { TemplateOnlyComponent } from \\"@ember/component/template-only\\";
15241600
@@ -1720,6 +1796,24 @@ class MyComponent extends Component {
17201796
"
17211797
`;
17221798
1799+
exports[`ambiguous > config > default > ["oops"] > with semi > it formats ../cases/gjs/component-class-with-content-before-template.gjs 1`] = `
1800+
"import Component from \\"@glimmer/component\\";
1801+
1802+
/** It's a component */
1803+
class MyComponent extends Component {
1804+
get whatever() {}
1805+
1806+
<template>
1807+
<h1>
1808+
Class top level template. Class top level template. Class top level
1809+
template. Class top level template. Class top level template.
1810+
</h1>
1811+
</template>
1812+
[\\"oops\\"];
1813+
}
1814+
"
1815+
`;
1816+
17231817
exports[`ambiguous > config > default > ["oops"] > with semi > it formats ../cases/gjs/default-export.gjs 1`] = `
17241818
"<template>
17251819
Explicit default export module top level component. Explicit default export
@@ -2013,6 +2107,24 @@ class MyComponent extends Component {
20132107
"
20142108
`;
20152109
2110+
exports[`ambiguous > config > default > ["oops"] > without semi > it formats ../cases/gjs/component-class-with-content-before-template.gjs 1`] = `
2111+
"import Component from \\"@glimmer/component\\";
2112+
2113+
/** It's a component */
2114+
class MyComponent extends Component {
2115+
get whatever() {}
2116+
2117+
<template>
2118+
<h1>
2119+
Class top level template. Class top level template. Class top level
2120+
template. Class top level template. Class top level template.
2121+
</h1>
2122+
</template>
2123+
[\\"oops\\"];
2124+
}
2125+
"
2126+
`;
2127+
20162128
exports[`ambiguous > config > default > ["oops"] > without semi > it formats ../cases/gjs/js-only.gjs 1`] = `
20172129
"const num = (1)[\\"oops\\"];
20182130
"

0 commit comments

Comments
 (0)