Skip to content

Commit

Permalink
(feat) rewrite svelte2tsx (#1237)
Browse files Browse the repository at this point in the history
Goal: Get rid of a tsx-style transformation and simplify transformations along the way
#1077
#1256
#1149

Advantages:

-   no messing with projects who use JSX for some other reason and then have conflicting JSX definitions
-   TS control flow easier to keep flowing
-   overall easier transformations of Svelte specific syntax like await, each etc.
-   better type inference in some cases, for example around svelte:component

This includes:

-   rewriting the html2jsx part of svelte2tsx
-   adjusting the language server to the new transformation
-   adding a toggle to language-server and vs code extension to turn on the new transformation. Default "off" for now, with the plan to switch it to "on" and then removing the old transformation altogether
-   ensuring tests run with both new and old transformation
-   adjusting the TypeScript plugin. It uses the new transformation already now since it's not easily possible to add an option (on/off). Should be low-impact since TS doesn't need to know much about the contents of a Svelte file, only its public API

Look at the PR of this commit to see more explanations with some of the entry points of the new transformation.
  • Loading branch information
dummdidumm authored Jan 27, 2022
1 parent 42f273e commit cad755f
Show file tree
Hide file tree
Showing 416 changed files with 11,684 additions and 3,432 deletions.
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ packages/svelte2tsx/test/**/*
packages/svelte2tsx/index.*
declare module '*.svelte'
packages/svelte2tsx/svelte-shims.d.ts
packages/svelte2tsx/svelte-jsx.d.ts
4 changes: 2 additions & 2 deletions .prettierignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
packages/svelte2tsx/*.d.ts
packages/svelte2tsx/test/*/samples/*/*sx
packages/svelte2tsx/test/*/samples/*/*.svelte
packages/svelte2tsx/repl/*
packages/svelte2tsx/test/*/samples/**/*
packages/svelte2tsx/test/sourcemaps/samples/*
packages/svelte2tsx/test/emitDts/samples/*/expected/**
packages/language-server/test/**/*.svelte
Expand Down
4 changes: 2 additions & 2 deletions packages/language-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"main": "dist/src/index.js",
"typings": "dist/src/index",
"scripts": {
"test": "cross-env TS_NODE_TRANSPILE_ONLY=true mocha --require ts-node/register \"test/**/*.ts\"",
"test": "cross-env TS_NODE_TRANSPILE_ONLY=true mocha --require ts-node/register \"test/**/*.ts\" --exclude \"test/**/*.d.ts\"",
"build": "tsc",
"prepublishOnly": "npm run build",
"watch": "tsc -w"
Expand Down Expand Up @@ -57,7 +57,7 @@
"source-map": "^0.7.3",
"svelte": "^3.46.1",
"svelte-preprocess": "~4.10.1",
"svelte2tsx": "~0.4.0",
"svelte2tsx": "~0.5.0",
"typescript": "*",
"vscode-css-languageservice": "~5.1.0",
"vscode-emmet-helper": "~2.6.0",
Expand Down
13 changes: 13 additions & 0 deletions packages/language-server/src/lib/documents/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,19 @@ export function getNodeIfIsInStartTag(html: HTMLDocument, offset: number): Node
}
}

/**
* Returns `true` if `offset` is a html tag and within the name of the start tag or end tag
*/
export function isInHTMLTagRange(html: HTMLDocument, offset: number): boolean {
const node = html.findNodeAt(offset);
return (
!!node.tag &&
node.tag[0] === node.tag[0].toLowerCase() &&
(node.start + node.tag.length + 1 >= offset ||
(!!node.endTagStart && node.endTagStart <= offset))
);
}

/**
* Gets word range at position.
* Delimiter is by default a whitespace, but can be adjusted.
Expand Down
2 changes: 2 additions & 0 deletions packages/language-server/src/ls-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const defaultLSConfig: LSConfig = {
},
svelte: {
enable: true,
useNewTransformation: false,
compilerWarnings: {},
diagnostics: { enable: true },
rename: { enable: true },
Expand Down Expand Up @@ -176,6 +177,7 @@ export type CompilerWarningsSettings = Record<string, 'ignore' | 'error'>;

export interface LSSvelteConfig {
enable: boolean;
useNewTransformation: boolean;
compilerWarnings: CompilerWarningsSettings;
diagnostics: {
enable: boolean;
Expand Down
60 changes: 48 additions & 12 deletions packages/language-server/src/plugins/PluginHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import {
TextEdit,
WorkspaceEdit
} from 'vscode-languageserver';
import { DocumentManager } from '../lib/documents';
import { DocumentManager, getNodeIfIsInHTMLStartTag } from '../lib/documents';
import { Logger } from '../logger';
import { regexLastIndexOf } from '../utils';
import { isNotNullOrUndefined, regexLastIndexOf } from '../utils';
import {
AppCompletionItem,
FileRename,
Expand Down Expand Up @@ -115,18 +115,54 @@ export class PluginHost implements LSProvider, OnWatchFileChanges {
): Promise<CompletionList> {
const document = this.getDocument(textDocument.uri);

const completions = (
await this.execute<CompletionList>(
'getCompletions',
[document, position, completionContext, cancellationToken],
ExecuteMode.Collect,
'high'
)
).filter((completion) => completion != null);
const completions = await Promise.all(
this.plugins.map(async (plugin) => {
const result = await this.tryExecutePlugin(
plugin,
'getCompletions',
[document, position, completionContext, cancellationToken],
null
);
if (result) {
return { result: result as CompletionList, plugin: plugin.__name };
}
})
).then((completions) => completions.filter(isNotNullOrUndefined));

const html = completions.find((completion) => completion.plugin === 'html');
const ts = completions.find((completion) => completion.plugin === 'ts');
if (html && ts && getNodeIfIsInHTMLStartTag(document.html, document.offsetAt(position))) {
// Completion in a component or html start tag and both html and ts
// suggest something -> filter out all duplicates from TS completions
const htmlCompletions = new Set(html.result.items.map((item) => item.label));
ts.result.items = ts.result.items.filter((item) => {
const label = item.label;
if (htmlCompletions.has(label)) {
return false;
}
if (label[0] === '"' && label[label.length - 1] === '"') {
if (htmlCompletions.has(label.slice(1, -1))) {
// "aria-label" -> aria-label -> exists in html completions
return false;
}
}
if (label.startsWith('on')) {
if (htmlCompletions.has('on:' + label.slice(2))) {
// onclick -> on:click -> exists in html completions
return false;
}
}
// adjust sort text so it does appear after html completions
item.sortText = 'Z' + (item.sortText || '');
return true;
});
}

let flattenedCompletions = flatten(completions.map((completion) => completion.items));
let flattenedCompletions = flatten(
completions.map((completion) => completion.result.items)
);
const isIncomplete = completions.reduce(
(incomplete, completion) => incomplete || completion.isIncomplete,
(incomplete, completion) => incomplete || completion.result.isIncomplete,
false as boolean
);

Expand Down
1 change: 1 addition & 0 deletions packages/language-server/src/plugins/css/CSSPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class CSSPlugin
DocumentSymbolsProvider,
SelectionRangeProvider
{
__name = 'css';
private configManager: LSConfigManager;
private cssDocuments = new WeakMap<Document, CSSDocument>();
private triggerCharacters = ['.', ':', '-', '/'];
Expand Down
1 change: 1 addition & 0 deletions packages/language-server/src/plugins/html/HTMLPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { possiblyComponent } from '../../utils';
export class HTMLPlugin
implements HoverProvider, CompletionsProvider, RenameProvider, LinkedEditingRangesProvider
{
__name = 'html';
private configManager: LSConfigManager;
private lang = getLanguageService({
customDataProviders: [svelteHtmlDataProvider],
Expand Down
2 changes: 1 addition & 1 deletion packages/language-server/src/plugins/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,4 @@ export type Plugin = Partial<
OnWatchFileChanges &
SelectionRangeProvider &
UpdateTsOrJsFile
>;
> & { __name: string };
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export class SveltePlugin
CodeActionsProvider,
SelectionRangeProvider
{
__name = 'svelte';
private docManager = new Map<Document, SvelteDocument>();

constructor(private configManager: LSConfigManager) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export interface SnapshotFragment extends DocumentMapper {
*/
export interface SvelteSnapshotOptions {
transformOnTemplateError: boolean;
useNewTransformation: boolean;
typingsNamespace: string;
}

export namespace DocumentSnapshot {
Expand Down Expand Up @@ -159,13 +161,21 @@ function preprocessSvelteFile(document: Document, options: SvelteSnapshotOptions
getScriptKindFromAttributes(document.scriptInfo?.attributes ?? {}),
getScriptKindFromAttributes(document.moduleScriptInfo?.attributes ?? {})
].includes(ts.ScriptKind.TSX)
? ts.ScriptKind.TSX
? options.useNewTransformation
? ts.ScriptKind.TS
: ts.ScriptKind.TSX
: options.useNewTransformation
? ts.ScriptKind.JS
: ts.ScriptKind.JSX;

try {
const tsx = svelte2tsx(text, {
filename: document.getFilePath() ?? undefined,
isTsFile: scriptKind === ts.ScriptKind.TSX,
isTsFile: options.useNewTransformation
? scriptKind === ts.ScriptKind.TS
: scriptKind === ts.ScriptKind.TSX,
mode: options.useNewTransformation ? 'ts' : 'tsx',
typingsNamespace: options.useNewTransformation ? options.typingsNamespace : undefined,
emitOnTemplateError: options.transformOnTemplateError,
namespace: document.config?.compilerOptions?.namespace,
accessors:
Expand Down Expand Up @@ -388,7 +398,7 @@ export class SvelteSnapshotFragment implements SnapshotFragment {
constructor(
private readonly mapper: DocumentMapper,
public readonly text: string,
private readonly parent: Document,
public readonly parent: Document,
private readonly url: string
) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export class LSAndTSDocResolver {
return {
ambientTypesSource: this.isSvelteCheck ? 'svelte-check' : 'svelte2tsx',
createDocument: this.createDocument,
useNewTransformation: this.configManager.getConfig().svelte.useNewTransformation,
transformOnTemplateError: !this.isSvelteCheck,
globalSnapshotsManager: this.globalSnapshotsManager,
notifyExceedSizeLimit: this.notifyExceedSizeLimit
Expand Down
Loading

0 comments on commit cad755f

Please sign in to comment.