From 14d9443ce74bf579d360273c204de6704a411096 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 31 Jul 2024 13:46:11 +0200 Subject: [PATCH 1/3] feat: replace svelte-preprocess with barebones TS preprocessor Removes the need for a dependency on svelte-preprocess in svelte-language-server, since that was all we used it for. closes: - #1683 and #1259, because the situation no longer arises (because we no longer have svelte-preprocess inside the VS Code extension) - #2391 --- packages/language-server/package.json | 1 - packages/language-server/src/importPackage.ts | 20 ++++-- .../src/lib/documents/configLoader.ts | 62 +++++++++++++------ .../src/plugins/svelte/SvelteDocument.ts | 9 ++- .../plugins/svelte/features/getDiagnostics.ts | 49 +++++++++++++-- .../svelte/features/getDiagnostics.test.ts | 27 ++++++-- pnpm-lock.yaml | 3 - 7 files changed, 128 insertions(+), 43 deletions(-) diff --git a/packages/language-server/package.json b/packages/language-server/package.json index 84ec3ed9b..680e21419 100644 --- a/packages/language-server/package.json +++ b/packages/language-server/package.json @@ -55,7 +55,6 @@ "prettier": "~3.2.5", "prettier-plugin-svelte": "^3.2.2", "svelte": "^3.57.0", - "svelte-preprocess": "^5.1.3", "svelte2tsx": "workspace:~", "typescript": "^5.5.2", "typescript-auto-import-cache": "^0.3.3", diff --git a/packages/language-server/src/importPackage.ts b/packages/language-server/src/importPackage.ts index 1c9ea6dcd..41e040762 100644 --- a/packages/language-server/src/importPackage.ts +++ b/packages/language-server/src/importPackage.ts @@ -1,7 +1,6 @@ import { dirname, resolve } from 'path'; import * as prettier from 'prettier'; import * as svelte from 'svelte/compiler'; -import sveltePreprocess from 'svelte-preprocess'; import { Logger } from './logger'; /** @@ -25,11 +24,15 @@ function dynamicRequire(dynamicFileToRequire: string): any { return require(dynamicFileToRequire); } -export function getPackageInfo(packageName: string, fromPath: string) { - const paths = [__dirname]; +export function getPackageInfo(packageName: string, fromPath: string, use_fallback = true) { + const paths: string[] = []; if (isTrusted) { - paths.unshift(fromPath); + paths.push(fromPath); } + if (use_fallback) { + paths.push(__dirname); + } + const packageJSONPath = require.resolve(`${packageName}/package.json`, { paths }); @@ -73,8 +76,13 @@ export function importSvelte(fromPath: string): typeof svelte { } } -export function importSveltePreprocess(fromPath: string): typeof sveltePreprocess { - const pkg = getPackageInfo('svelte-preprocess', fromPath); +/** Can throw because no fallback guaranteed */ +export function importSveltePreprocess(fromPath: string): any { + const pkg = getPackageInfo( + 'svelte-preprocess', + fromPath, + false // svelte-language-server doesn't have a dependency on svelte-preprocess so we can't provide a fallback + ); const main = resolve(pkg.path); Logger.debug('Using svelte-preprocess v' + pkg.version.full, 'from', main); return dynamicRequire(main); diff --git a/packages/language-server/src/lib/documents/configLoader.ts b/packages/language-server/src/lib/documents/configLoader.ts index 2cea65b7e..5c311a96f 100644 --- a/packages/language-server/src/lib/documents/configLoader.ts +++ b/packages/language-server/src/lib/documents/configLoader.ts @@ -9,6 +9,7 @@ import _path from 'path'; import _fs from 'fs'; import { pathToFileURL, URL } from 'url'; import { FileMap } from './fileCollection'; +import ts from 'typescript'; export type InternalPreprocessorGroup = PreprocessorGroup & { /** @@ -249,24 +250,49 @@ export class ConfigLoader { } private useFallbackPreprocessor(path: string, foundConfig: boolean): SvelteConfig { - Logger.log( - (foundConfig - ? 'Found svelte.config.js but there was an error loading it. ' - : 'No svelte.config.js found. ') + - 'Using https://github.com/sveltejs/svelte-preprocess as fallback' - ); - const sveltePreprocess = importSveltePreprocess(path); - return { - preprocess: sveltePreprocess({ - // 4.x does not have transpileOnly anymore, but if the user has version 3.x - // in his repo, that one is loaded instead, for which we still need this. - typescript: { - transpileOnly: true, - compilerOptions: { sourceMap: true, inlineSourceMap: false } - } - }), - isFallbackConfig: true - }; + try { + const sveltePreprocess = importSveltePreprocess(path); + Logger.log( + (foundConfig + ? 'Found svelte.config.js but there was an error loading it. ' + : 'No svelte.config.js found. ') + + 'Using https://github.com/sveltejs/svelte-preprocess as fallback' + ); + return { + preprocess: sveltePreprocess({ + // 4.x does not have transpileOnly anymore, but if the user has version 3.x + // in his repo, that one is loaded instead, for which we still need this. + typescript: { + transpileOnly: true, + compilerOptions: { sourceMap: true, inlineSourceMap: false } + } + }), + isFallbackConfig: true + }; + } catch (e) { + // User doesn't have svelte-preprocess installed, provide a barebones TS preprocessor + return { + preprocess: { + // @ts-expect-error name property exists in Svelte 4 onwards + name: 'svelte-language-tools-ts-fallback-preprocessor', + script: ({ content, attributes, filename }) => { + if (attributes.lang !== 'ts') return; + + const { outputText, sourceMapText } = ts.transpileModule(content, { + fileName: filename, + compilerOptions: { + module: ts.ModuleKind.ESNext, + target: ts.ScriptTarget.ESNext, + sourceMap: true, + verbatimModuleSyntax: true + } + }); + return { code: outputText, map: sourceMapText }; + } + }, + isFallbackConfig: true + }; + } } } diff --git a/packages/language-server/src/plugins/svelte/SvelteDocument.ts b/packages/language-server/src/plugins/svelte/SvelteDocument.ts index 0f132b502..3213eff66 100644 --- a/packages/language-server/src/plugins/svelte/SvelteDocument.ts +++ b/packages/language-server/src/plugins/svelte/SvelteDocument.ts @@ -37,6 +37,7 @@ type PositionMapper = Pick { if (!this.transpiledDoc) { - const { - version: { major, minor } - } = getPackageInfo('svelte', this.getFilePath()); + if (!this.svelteVersion) { + const { major, minor } = getPackageInfo('svelte', this.getFilePath()).version; + this.svelteVersion = [major, minor]; + } + const [major, minor] = this.svelteVersion; if (major > 3 || (major === 3 && minor >= 32)) { this.transpiledDoc = await TranspiledSvelteDocument.create( diff --git a/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts b/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts index 4c1f91d7a..7b7267c6e 100644 --- a/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts +++ b/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts @@ -1,5 +1,3 @@ -// @ts-ignore -import { Warning } from 'svelte/types/compiler/interfaces'; import { CancellationToken, Diagnostic, @@ -63,6 +61,17 @@ async function tryGetDiagnostics( if (cancellationToken?.isCancellationRequested) { return []; } + + let ignoreScriptWarnings = false; + let ingoreStyleWarnings = false; + let ignoreTemplateWarnings = false; + if (!document.config?.preprocess || !!document.config.isFallbackConfig) { + ignoreTemplateWarnings = !!document.getLanguageAttribute('template'); + ingoreStyleWarnings = !!document.getLanguageAttribute('style'); + const scriptAttr = document.getLanguageAttribute('script'); + ignoreScriptWarnings = !!scriptAttr && scriptAttr !== 'ts'; + } + return (res.warnings || []) .filter((warning) => settings[warning.code] !== 'ignore') .map((warning) => { @@ -81,7 +90,15 @@ async function tryGetDiagnostics( }) .map((diag) => mapObjWithRangeToOriginal(transpiled, diag)) .map((diag) => adjustMappings(diag, document)) - .filter((diag) => isNoFalsePositive(diag, document)); + .filter((diag) => + isNoFalsePositive( + diag, + document, + ignoreScriptWarnings, + ingoreStyleWarnings, + ignoreTemplateWarnings + ) + ); } catch (err) { return createParserErrorDiagnostic(err, document) .map((diag) => mapObjWithRangeToOriginal(transpiled, diag)) @@ -290,8 +307,28 @@ function getErrorMessage(error: any, source: string, hint = '') { ); } -function isNoFalsePositive(diag: Diagnostic, doc: Document): boolean { - if (diag.code !== 'unused-export-let') { +function isNoFalsePositive( + diag: Diagnostic, + doc: Document, + ignoreScriptWarnings: boolean, + ingoreStyleWarnings: boolean, + ignoreTemplateWarnings: boolean +): boolean { + if ( + (ignoreTemplateWarnings || ignoreScriptWarnings) && + (typeof diag.code !== 'string' || !diag.code.startsWith('a11y')) + ) { + return false; + } + + if ( + ingoreStyleWarnings && + (diag.code === 'css-unused-selector' || diag.code === 'css_unused_selector') + ) { + return false; + } + + if (diag.code !== 'unused-export-let' && diag.code !== 'export_let_unused') { return true; } @@ -328,7 +365,7 @@ function adjustMappings(diag: Diagnostic, doc: Document): Diagnostic { diag.range = moveRangeStartToEndIfNecessary(diag.range); if ( - diag.code === 'css-unused-selector' && + (diag.code === 'css-unused-selector' || diag.code === 'css_unused_selector') && doc.styleInfo && !isInTag(diag.range.start, doc.styleInfo) ) { diff --git a/packages/language-server/test/plugins/svelte/features/getDiagnostics.test.ts b/packages/language-server/test/plugins/svelte/features/getDiagnostics.test.ts index 0915cc35a..0c3856a6b 100644 --- a/packages/language-server/test/plugins/svelte/features/getDiagnostics.test.ts +++ b/packages/language-server/test/plugins/svelte/features/getDiagnostics.test.ts @@ -31,7 +31,12 @@ describe('SveltePlugin#getDiagnostics', () => { docText?: string; }) { const document = new Document('', docText); - const svelteDoc: SvelteDocument = { getTranspiled, getCompiled, config }; + const svelteDoc: SvelteDocument = { + getTranspiled, + getCompiled, + config, + getSvelteVersion: () => [4, 0] + }; const result = await getDiagnostics(document, svelteDoc, settings); return { toEqual: (expected: Diagnostic[]) => assert.deepStrictEqual(result, expected) @@ -298,7 +303,9 @@ describe('SveltePlugin#getDiagnostics', () => { } ] }), - config: {} + config: { + preprocess: [] + } }) ).toEqual([ { @@ -343,7 +350,9 @@ describe('SveltePlugin#getDiagnostics', () => { ] } }), - config: {} + config: { + preprocess: [] + } }) ).toEqual([]); }); @@ -372,7 +381,9 @@ describe('SveltePlugin#getDiagnostics', () => { ] } }), - config: {} + config: { + preprocess: [] + } }) ).toEqual([]); }); @@ -399,7 +410,9 @@ describe('SveltePlugin#getDiagnostics', () => { ] } }), - config: {}, + config: { + preprocess: [] + }, settings: { 123: 'ignore' } }) ).toEqual([]); @@ -425,7 +438,9 @@ describe('SveltePlugin#getDiagnostics', () => { } ] }), - config: {}, + config: { + preprocess: [] + }, settings: { 123: 'error' } }) ).toEqual([ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 7411c3b4d..a1e587871 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -51,9 +51,6 @@ importers: svelte: specifier: ^3.57.0 version: 3.57.0 - svelte-preprocess: - specifier: ^5.1.3 - version: 5.1.3(svelte@3.57.0)(typescript@5.5.2) svelte2tsx: specifier: workspace:~ version: link:../svelte2tsx From d8a8fd9481e1889268a721c60de4fb247112706e Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 31 Jul 2024 13:53:04 +0200 Subject: [PATCH 2/3] use ts-ignore instead because of Svelte 5 --- packages/language-server/src/lib/documents/configLoader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/language-server/src/lib/documents/configLoader.ts b/packages/language-server/src/lib/documents/configLoader.ts index 5c311a96f..4d7e583c6 100644 --- a/packages/language-server/src/lib/documents/configLoader.ts +++ b/packages/language-server/src/lib/documents/configLoader.ts @@ -273,7 +273,7 @@ export class ConfigLoader { // User doesn't have svelte-preprocess installed, provide a barebones TS preprocessor return { preprocess: { - // @ts-expect-error name property exists in Svelte 4 onwards + // @ts-ignore name property exists in Svelte 4 onwards name: 'svelte-language-tools-ts-fallback-preprocessor', script: ({ content, attributes, filename }) => { if (attributes.lang !== 'ts') return; From 530abfd9bb505fb112e598fb8a60511ea6148536 Mon Sep 17 00:00:00 2001 From: "Lyu, Wei Da" Date: Thu, 1 Aug 2024 10:13:21 +0800 Subject: [PATCH 3/3] typo --- .../src/plugins/svelte/features/getDiagnostics.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts b/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts index 7b7267c6e..969760178 100644 --- a/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts +++ b/packages/language-server/src/plugins/svelte/features/getDiagnostics.ts @@ -63,11 +63,11 @@ async function tryGetDiagnostics( } let ignoreScriptWarnings = false; - let ingoreStyleWarnings = false; + let ignoreStyleWarnings = false; let ignoreTemplateWarnings = false; if (!document.config?.preprocess || !!document.config.isFallbackConfig) { ignoreTemplateWarnings = !!document.getLanguageAttribute('template'); - ingoreStyleWarnings = !!document.getLanguageAttribute('style'); + ignoreStyleWarnings = !!document.getLanguageAttribute('style'); const scriptAttr = document.getLanguageAttribute('script'); ignoreScriptWarnings = !!scriptAttr && scriptAttr !== 'ts'; } @@ -95,7 +95,7 @@ async function tryGetDiagnostics( diag, document, ignoreScriptWarnings, - ingoreStyleWarnings, + ignoreStyleWarnings, ignoreTemplateWarnings ) ); @@ -311,7 +311,7 @@ function isNoFalsePositive( diag: Diagnostic, doc: Document, ignoreScriptWarnings: boolean, - ingoreStyleWarnings: boolean, + ignoreStyleWarnings: boolean, ignoreTemplateWarnings: boolean ): boolean { if ( @@ -322,7 +322,7 @@ function isNoFalsePositive( } if ( - ingoreStyleWarnings && + ignoreStyleWarnings && (diag.code === 'css-unused-selector' || diag.code === 'css_unused_selector') ) { return false;