Skip to content

Commit d993b25

Browse files
authored
(feat) more robust parsing for mustache tags (#786)
In svelte2tsx, run a simple scan before sending the code to the svelte compiler. Right now the compiler errors on mustache tags that are incomplete and are for example ended by an operand like ".". This change allows the errored syntax to be emitted untouched, allowing better DX. The behavior is toggleable and off by default so that `svelte-check` will still emit these errors. #538
1 parent 6ef9b67 commit d993b25

File tree

20 files changed

+269
-45
lines changed

20 files changed

+269
-45
lines changed

packages/language-server/src/plugins/typescript/DocumentSnapshot.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ export interface SnapshotFragment extends DocumentMapper {
7373
*/
7474
export interface SvelteSnapshotOptions {
7575
strictMode: boolean;
76+
transformOnTemplateError: boolean;
7677
}
7778

7879
export namespace DocumentSnapshot {
@@ -142,7 +143,8 @@ function preprocessSvelteFile(document: Document, options: SvelteSnapshotOptions
142143
const tsx = svelte2tsx(text, {
143144
strictMode: options.strictMode,
144145
filename: document.getFilePath() ?? undefined,
145-
isTsFile: scriptKind === ts.ScriptKind.TSX
146+
isTsFile: scriptKind === ts.ScriptKind.TSX,
147+
emitOnTemplateError: options.transformOnTemplateError
146148
});
147149
text = tsx.code;
148150
tsxMap = tsx.map;

packages/language-server/src/plugins/typescript/LSAndTSDocResolver.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,17 @@ import {
77
getLanguageServiceForDocument,
88
getLanguageServiceForPath,
99
getService,
10-
LanguageServiceContainer
10+
LanguageServiceContainer,
11+
LanguageServiceDocumentContext
1112
} from './service';
1213
import { SnapshotManager } from './SnapshotManager';
1314

1415
export class LSAndTSDocResolver {
1516
constructor(
1617
private readonly docManager: DocumentManager,
1718
private readonly workspaceUris: string[],
18-
private readonly configManager: LSConfigManager
19+
private readonly configManager: LSConfigManager,
20+
private readonly transformOnTemplateError = true
1921
) {
2022
docManager.on(
2123
'documentChange',
@@ -43,8 +45,15 @@ export class LSAndTSDocResolver {
4345
return document;
4446
};
4547

48+
private get lsDocumentContext(): LanguageServiceDocumentContext {
49+
return {
50+
createDocument: this.createDocument,
51+
transformOnTemplateError: this.transformOnTemplateError
52+
};
53+
}
54+
4655
getLSForPath(path: string) {
47-
return getLanguageServiceForPath(path, this.workspaceUris, this.createDocument);
56+
return getLanguageServiceForPath(path, this.workspaceUris, this.lsDocumentContext);
4857
}
4958

5059
getLSAndTSDoc(
@@ -57,7 +66,7 @@ export class LSAndTSDocResolver {
5766
const lang = getLanguageServiceForDocument(
5867
document,
5968
this.workspaceUris,
60-
this.createDocument
69+
this.lsDocumentContext
6170
);
6271
const filePath = document.getFilePath()!;
6372
const tsDoc = this.getSnapshot(filePath, document);
@@ -74,7 +83,10 @@ export class LSAndTSDocResolver {
7483

7584
let tsDoc = snapshotManager.get(filePath);
7685
if (!tsDoc) {
77-
const options = { strictMode: !!tsService.compilerOptions.strict };
86+
const options = {
87+
strictMode: !!tsService.compilerOptions.strict,
88+
transformOnTemplateError: this.transformOnTemplateError
89+
};
7890
tsDoc = document
7991
? DocumentSnapshot.fromDocument(document, options)
8092
: DocumentSnapshot.fromFilePath(filePath, options);
@@ -99,7 +111,7 @@ export class LSAndTSDocResolver {
99111
}
100112

101113
private getTSService(filePath: string): LanguageServiceContainer {
102-
return getService(filePath, this.workspaceUris, this.createDocument);
114+
return getService(filePath, this.workspaceUris, this.lsDocumentContext);
103115
}
104116

105117
private getUserPreferences(scriptKind: ts.ScriptKind): ts.UserPreferences {

packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,16 @@ export class TypeScriptPlugin
100100
constructor(
101101
docManager: DocumentManager,
102102
configManager: LSConfigManager,
103-
workspaceUris: string[]
103+
workspaceUris: string[],
104+
isEditor = true
104105
) {
105106
this.configManager = configManager;
106-
this.lsAndTsDocResolver = new LSAndTSDocResolver(docManager, workspaceUris, configManager);
107+
this.lsAndTsDocResolver = new LSAndTSDocResolver(
108+
docManager,
109+
workspaceUris,
110+
configManager,
111+
/**transformOnTemplateError */ isEditor
112+
);
107113
this.completionProvider = new CompletionsProviderImpl(this.lsAndTsDocResolver);
108114
this.codeActionsProvider = new CodeActionsProviderImpl(
109115
this.lsAndTsDocResolver,
@@ -381,7 +387,10 @@ export class TypeScriptPlugin
381387

382388
// Since the options parameter only applies to svelte snapshots, and this is not
383389
// a svelte file, we can just set it to false without having any effect.
384-
snapshotManager.updateByFileName(fileName, { strictMode: false });
390+
snapshotManager.updateByFileName(fileName, {
391+
strictMode: false,
392+
transformOnTemplateError: false
393+
});
385394
}
386395
}
387396

packages/language-server/src/plugins/typescript/service.ts

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,35 +19,42 @@ export interface LanguageServiceContainer {
1919

2020
const services = new Map<string, LanguageServiceContainer>();
2121

22-
export type CreateDocument = (fileName: string, content: string) => Document;
22+
export interface LanguageServiceDocumentContext {
23+
transformOnTemplateError: boolean;
24+
createDocument: (fileName: string, content: string) => Document;
25+
}
2326

2427
export function getLanguageServiceForPath(
2528
path: string,
2629
workspaceUris: string[],
27-
createDocument: CreateDocument
30+
docContext: LanguageServiceDocumentContext
2831
): ts.LanguageService {
29-
return getService(path, workspaceUris, createDocument).getService();
32+
return getService(path, workspaceUris, docContext).getService();
3033
}
3134

3235
export function getLanguageServiceForDocument(
3336
document: Document,
3437
workspaceUris: string[],
35-
createDocument: CreateDocument
38+
docContext: LanguageServiceDocumentContext
3639
): ts.LanguageService {
37-
return getService(document.getFilePath() || '', workspaceUris, createDocument).updateDocument(
40+
return getService(document.getFilePath() || '', workspaceUris, docContext).updateDocument(
3841
document
3942
);
4043
}
4144

42-
export function getService(path: string, workspaceUris: string[], createDocument: CreateDocument) {
45+
export function getService(
46+
path: string,
47+
workspaceUris: string[],
48+
docContext: LanguageServiceDocumentContext
49+
) {
4350
const tsconfigPath = findTsConfigPath(path, workspaceUris);
4451

4552
let service: LanguageServiceContainer;
4653
if (services.has(tsconfigPath)) {
4754
service = services.get(tsconfigPath)!;
4855
} else {
4956
Logger.log('Initialize new ts service at ', tsconfigPath);
50-
service = createLanguageService(tsconfigPath, createDocument);
57+
service = createLanguageService(tsconfigPath, docContext);
5158
services.set(tsconfigPath, service);
5259
}
5360

@@ -56,7 +63,7 @@ export function getService(path: string, workspaceUris: string[], createDocument
5663

5764
export function createLanguageService(
5865
tsconfigPath: string,
59-
createDocument: CreateDocument
66+
docContext: LanguageServiceDocumentContext
6067
): LanguageServiceContainer {
6168
const workspacePath = tsconfigPath ? dirname(tsconfigPath) : '';
6269

@@ -105,6 +112,10 @@ export function createLanguageService(
105112
getScriptKind: (fileName: string) => getSnapshot(fileName).scriptKind
106113
};
107114
let languageService = ts.createLanguageService(host);
115+
const transformationConfig = {
116+
strictMode: !!compilerOptions.strict,
117+
transformOnTemplateError: docContext.transformOnTemplateError
118+
};
108119

109120
return {
110121
tsconfigPath,
@@ -128,9 +139,7 @@ export function createLanguageService(
128139
return languageService;
129140
}
130141

131-
const newSnapshot = DocumentSnapshot.fromDocument(document, {
132-
strictMode: !!compilerOptions.strict
133-
});
142+
const newSnapshot = DocumentSnapshot.fromDocument(document, transformationConfig);
134143
if (preSnapshot && preSnapshot.scriptKind !== newSnapshot.scriptKind) {
135144
// Restart language service as it doesn't handle script kind changes.
136145
languageService.dispose();
@@ -151,11 +160,12 @@ export function createLanguageService(
151160

152161
if (isSvelteFilePath(fileName)) {
153162
const file = ts.sys.readFile(fileName) || '';
154-
doc = DocumentSnapshot.fromDocument(createDocument(fileName, file), {
155-
strictMode: !!compilerOptions.strict
156-
});
163+
doc = DocumentSnapshot.fromDocument(
164+
docContext.createDocument(fileName, file),
165+
transformationConfig
166+
);
157167
} else {
158-
doc = DocumentSnapshot.fromFilePath(fileName, { strictMode: !!compilerOptions.strict });
168+
doc = DocumentSnapshot.fromFilePath(fileName, transformationConfig);
159169
}
160170

161171
snapshotManager.set(fileName, doc);

packages/language-server/src/svelte-check.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,12 @@ export class SvelteCheck {
4343
}
4444
if (shouldRegister('js')) {
4545
this.pluginHost.register(
46-
new TypeScriptPlugin(this.docManager, this.configManager, [
47-
pathToUrl(workspacePath)
48-
])
46+
new TypeScriptPlugin(
47+
this.docManager,
48+
this.configManager,
49+
[pathToUrl(workspacePath)],
50+
/**isEditor */ false
51+
)
4952
);
5053
}
5154

packages/language-server/test/plugins/typescript/features/CompletionProvider.test.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ describe('CompletionProviderImpl', () => {
6767
);
6868
assert.ok(completions!.items.length > 0, 'Expected completions to have length');
6969

70-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
71-
const { data, ...withoutData } = completions!.items[0];
70+
const first = completions!.items[0];
71+
delete first.data;
7272

73-
assert.deepStrictEqual(withoutData, <CompletionItem>{
73+
assert.deepStrictEqual(first, <CompletionItem>{
7474
label: 'b',
7575
insertText: undefined,
7676
kind: CompletionItemKind.Method,
@@ -80,6 +80,31 @@ describe('CompletionProviderImpl', () => {
8080
});
8181
});
8282

83+
it('provides completions on simple property access in mustache', async () => {
84+
const { completionProvider, document } = setup('mustache.svelte');
85+
86+
const completions = await completionProvider.getCompletions(
87+
document,
88+
Position.create(5, 3),
89+
{
90+
triggerKind: CompletionTriggerKind.TriggerCharacter,
91+
triggerCharacter: '.'
92+
}
93+
);
94+
95+
const first = completions!.items[0];
96+
delete first.data;
97+
98+
assert.deepStrictEqual(first, <CompletionItem>{
99+
label: 'b',
100+
insertText: undefined,
101+
kind: CompletionItemKind.Field,
102+
sortText: '1',
103+
commitCharacters: ['.', ',', '('],
104+
preselect: undefined
105+
});
106+
});
107+
83108
it('provides event completions', async () => {
84109
const { completionProvider, document } = setup('component-events-completion.svelte');
85110

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<script>
2+
let a = 1;
3+
let b = { b: 1 };
4+
</script>
5+
{a?.}
6+
{b.}

packages/svelte2tsx/index.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,9 @@ export default function svelte2tsx(
3232
* `svelte-preprocess`.
3333
*/
3434
isTsFile?: boolean;
35+
/**
36+
* Whether to try emitting result when there's a syntax error in the template
37+
*/
38+
emitOnTemplateError?: boolean;
3539
}
3640
): SvelteCompiledToTsx

packages/svelte2tsx/src/htmlxtojsx/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ export function convertHtmlxToJsx(
138138
/**
139139
* @internal For testing only
140140
*/
141-
export function htmlx2jsx(htmlx: string) {
142-
const ast = parseHtmlx(htmlx);
141+
export function htmlx2jsx(htmlx: string, options?: { emitOnTemplateError?: boolean }) {
142+
const ast = parseHtmlx(htmlx, options);
143143
const str = new MagicString(htmlx);
144144

145145
convertHtmlxToJsx(str, ast);

packages/svelte2tsx/src/svelte2tsx/index.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,11 @@ type TemplateProcessResult = {
6666
*/
6767
const COMPONENT_SUFFIX = '__SvelteComponent_';
6868

69-
function processSvelteTemplate(str: MagicString): TemplateProcessResult {
70-
const htmlxAst = parseHtmlx(str.original);
69+
function processSvelteTemplate(
70+
str: MagicString,
71+
options?: { emitOnTemplateError?: boolean }
72+
): TemplateProcessResult {
73+
const htmlxAst = parseHtmlx(str.original, options);
7174

7275
let uses$$props = false;
7376
let uses$$restProps = false;
@@ -407,7 +410,12 @@ function createRenderFunction({
407410

408411
export function svelte2tsx(
409412
svelte: string,
410-
options?: { filename?: string; strictMode?: boolean; isTsFile?: boolean }
413+
options?: {
414+
filename?: string;
415+
strictMode?: boolean;
416+
isTsFile?: boolean;
417+
emitOnTemplateError?: boolean;
418+
}
411419
) {
412420
const str = new MagicString(svelte);
413421
// process the htmlx as a svelte template
@@ -420,7 +428,7 @@ export function svelte2tsx(
420428
uses$$restProps,
421429
events,
422430
componentDocumentation
423-
} = processSvelteTemplate(str);
431+
} = processSvelteTemplate(str, options);
424432

425433
/* Rearrange the script tags so that module is first, and instance second followed finally by the template
426434
* This is a bit convoluted due to some trouble I had with magic string. A simple str.move(start,end,0) for each script wasn't enough

0 commit comments

Comments
 (0)