From f8d7c20e1e6704f55cfa3a9c56170be2fb938c21 Mon Sep 17 00:00:00 2001 From: Simon H <5968653+dummdidumm@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:09:46 +0200 Subject: [PATCH] feat: support prettier v3 (#2100) --- package.json | 2 +- packages/language-server/package.json | 2 +- .../src/plugins/svelte/SveltePlugin.ts | 80 ++++++++++++---- .../test/plugins/svelte/SveltePlugin.test.ts | 93 ++++++++++++++++++- .../features/diagnostics/index.test.ts | 2 +- .../features/inlayHints/index.test.ts | 2 +- .../test/plugins/typescript/test-utils.ts | 12 +-- pnpm-lock.yaml | 18 ++-- 8 files changed, 173 insertions(+), 38 deletions(-) diff --git a/package.json b/package.json index 10223c91b..b059eacff 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,8 @@ "typescript": "^5.1.3" }, "devDependencies": { - "prettier": "2.8.6", "cross-env": "^7.0.2", + "prettier": "~2.8.8", "ts-node": "^10.0.0" }, "packageManager": "pnpm@8.4.0" diff --git a/packages/language-server/package.json b/packages/language-server/package.json index 24cb0cc19..b835eb035 100644 --- a/packages/language-server/package.json +++ b/packages/language-server/package.json @@ -53,7 +53,7 @@ "estree-walker": "^2.0.1", "fast-glob": "^3.2.7", "lodash": "^4.17.21", - "prettier": "2.8.6", + "prettier": "~2.8.8", "prettier-plugin-svelte": "~2.10.1", "svelte": "^3.57.0", "svelte-preprocess": "~5.0.4", diff --git a/packages/language-server/src/plugins/svelte/SveltePlugin.ts b/packages/language-server/src/plugins/svelte/SveltePlugin.ts index ddeb6df92..3529ec152 100644 --- a/packages/language-server/src/plugins/svelte/SveltePlugin.ts +++ b/packages/language-server/src/plugins/svelte/SveltePlugin.ts @@ -75,16 +75,46 @@ export class SveltePlugin } const filePath = document.getFilePath()!; - const prettier = importPrettier(filePath); - // Try resolving the config through prettier and fall back to possible editor config - const config = this.configManager.getMergedPrettierConfig( - await prettier.resolveConfig(filePath, { editorconfig: true }), - // Be defensive here because IDEs other than VSCode might not have these settings - options && { - tabWidth: options.tabSize, - useTabs: !options.insertSpaces + + /** + * Prettier v2 can't use v3 plugins and vice versa. Therefore, we need to check + * which version of prettier is used in the workspace and import the correct + * version of the Svelte plugin. If user uses Prettier >= 3 and has no Svelte plugin + * then fall back to our built-in versions which are both v2 and compatible with + * each other. + * TODO switch this around at some point to load Prettier v3 by default because it's + * more likely that users have that installed. + */ + const importFittingPrettier = async () => { + const getConfig = async (p: any) => { + // Try resolving the config through prettier and fall back to possible editor config + return this.configManager.getMergedPrettierConfig( + await p.resolveConfig(filePath, { editorconfig: true }), + // Be defensive here because IDEs other than VSCode might not have these settings + options && { + tabWidth: options.tabSize, + useTabs: !options.insertSpaces + } + ); + }; + + const prettier1 = importPrettier(filePath); + const config1 = await getConfig(prettier1); + const pluginLoaded = await hasSveltePluginLoaded(prettier1, config1.plugins); + if (Number(prettier1.version[0]) < 3 || pluginLoaded) { + // plugin loaded, or referenced in user config as a plugin, or same version as our fallback version -> ok + return { prettier: prettier1, config: config1, isFallback: false }; } - ); + + // User either only has Plugin or incompatible Prettier major version installed or none + // -> load our fallback version + const prettier2 = importPrettier(__dirname); + const config2 = await getConfig(prettier2); + return { prettier: prettier2, config: config2, isFallback: true }; + }; + + const { prettier, config, isFallback } = await importFittingPrettier(); + // If user has prettier-plugin-svelte 1.x, then remove `options` from the sort // order or else it will throw a config error (`options` was not present back then). if ( @@ -95,6 +125,16 @@ export class SveltePlugin .replace('-options', '') .replace('options-', ''); } + // If user has prettier-plugin-svelte 3.x, then add `options` from the sort + // order or else it will throw a config error (now required). + if ( + config?.svelteSortOrder && + !config.svelteSortOrder.includes('options') && + config.svelteSortOrder !== 'none' && + getPackageInfo('prettier-plugin-svelte', filePath)?.version.major >= 3 + ) { + config.svelteSortOrder = 'options-' + config.svelteSortOrder; + } // Take .prettierignore into account const fileInfo = await prettier.getFileInfo(filePath, { ignorePath: this.configManager.getPrettierConfig()?.ignorePath ?? '.prettierignore', @@ -106,14 +146,15 @@ export class SveltePlugin return []; } - const formattedCode = prettier.format(document.getText(), { + // Prettier v3 format is async, v2 is not + const formattedCode = await prettier.format(document.getText(), { ...config, plugins: Array.from( new Set([ ...((config.plugins as string[]) ?? []) .map(resolvePlugin) .filter(isNotNullOrUndefined), - ...getSveltePlugin() + ...(await getSveltePlugin(config.plugins)) ]) ), parser: 'svelte' as any @@ -131,15 +172,22 @@ export class SveltePlugin ) ]; - function getSveltePlugin() { + async function getSveltePlugin(plugins: string[] = []) { // Only provide our version of the svelte plugin if the user doesn't have one in // the workspace already. If we did it, Prettier would - for some reason - use // the workspace version for parsing and the extension version for printing, // which could crash if the contract of the parser output changed. - const hasPluginLoadedAlready = prettier - .getSupportInfo() - .languages.some((l) => l.name === 'svelte'); - return hasPluginLoadedAlready ? [] : [require.resolve('prettier-plugin-svelte')]; + return !isFallback && (await hasSveltePluginLoaded(prettier, plugins)) + ? [] + : [require.resolve('prettier-plugin-svelte')]; + } + + async function hasSveltePluginLoaded(p: typeof prettier, plugins: string[] = []) { + if (plugins.some((plugin) => plugin.includes('prettier-plugin-svelte'))) return true; + if (Number(p.version[0]) >= 3) return false; // Prettier version 3 has removed the "search plugins" feature + // Prettier v3 getSupportInfo is async, v2 is not + const info = await p.getSupportInfo(); + return info.languages.some((l) => l.name === 'svelte'); } function resolvePlugin(plugin: string) { diff --git a/packages/language-server/test/plugins/svelte/SveltePlugin.test.ts b/packages/language-server/test/plugins/svelte/SveltePlugin.test.ts index 2eb58bb09..6c6a37c11 100644 --- a/packages/language-server/test/plugins/svelte/SveltePlugin.test.ts +++ b/packages/language-server/test/plugins/svelte/SveltePlugin.test.ts @@ -67,11 +67,12 @@ describe('Svelte Plugin', () => { assert.deepStrictEqual(diagnostics, []); }); - describe('#formatDocument', () => { - function stubPrettier(config: any) { + describe.only('#formatDocument', () => { + function stubPrettierV2(config: any) { const formatStub = sinon.stub().returns('formatted'); sinon.stub(importPackage, 'importPrettier').returns({ + version: '2.8.0', resolveConfig: () => Promise.resolve(config), getFileInfo: () => ({ ignored: false }), format: formatStub, @@ -84,7 +85,8 @@ describe('Svelte Plugin', () => { async function testFormat( config: any, fallbackPrettierConfig: any, - options?: Parameters[2] + options?: Parameters[2], + stubPrettier = stubPrettierV2 ) { const { plugin, document } = setup('unformatted', fallbackPrettierConfig, options); const formatStub = stubPrettier(config); @@ -182,6 +184,91 @@ describe('Svelte Plugin', () => { ...defaultSettings }); }); + + it('should load the user prettier version (version 2)', async () => { + function stubPrettier(config: any) { + const formatStub = sinon.stub().returns('formatted'); + + sinon + .stub(importPackage, 'importPrettier') + .onFirstCall() + .returns({ + version: '2.8.0', + resolveConfig: () => Promise.resolve(config), + getFileInfo: () => ({ ignored: false }), + format: formatStub, + getSupportInfo: () => ({ languages: [{ name: 'svelte' }] }) + }) + .onSecondCall() + .throws(new Error('should not be called')); + + return formatStub; + } + + await testFormat({}, {}, undefined, stubPrettier); + }); + + it('should load the user prettier version (version 3)', async () => { + function stubPrettier(config: any) { + const formatStub = sinon.stub().returns(Promise.resolve('formatted')); + + sinon + .stub(importPackage, 'importPrettier') + .onFirstCall() + .returns({ + version: '3.0.0', + resolveConfig: () => Promise.resolve(config), + getFileInfo: () => ({ ignored: false }), + format: formatStub, + getSupportInfo: () => Promise.resolve({ languages: [] }) + }) + .onSecondCall() + .throws(new Error('should not be called')); + + return formatStub; + } + + await testFormat( + // written like this to not trigger require.resolve which fails here + { plugins: ['./node_modules/prettier-plugin-svelte'] }, + {}, + undefined, + stubPrettier + ); + }); + + it('should fall back to built-in prettier version', async () => { + function stubPrettier(config: any) { + const formatStub = sinon.stub().returns('formatted'); + + sinon + .stub(importPackage, 'importPrettier') + .onFirstCall() + .returns({ + version: '3.0.0', + resolveConfig: () => Promise.resolve(config), + getFileInfo: () => ({ ignored: false }), + format: () => { + throw new Error('should not be called'); + }, + getSupportInfo: () => Promise.resolve({ languages: [] }) + }) + .onSecondCall() + .returns({ + version: '2.8.0', + resolveConfig: () => Promise.resolve(config), + getFileInfo: () => ({ ignored: false }), + format: formatStub, + getSupportInfo: () => ({ languages: [] }) + }) + .onThirdCall() + .throws(new Error('should not be called')); + + return formatStub; + } + + await testFormat({}, {}, undefined, stubPrettier); + }); }); it('can cancel completion before promise resolved', async () => { diff --git a/packages/language-server/test/plugins/typescript/features/diagnostics/index.test.ts b/packages/language-server/test/plugins/typescript/features/diagnostics/index.test.ts index cc60cf8fc..03cc2ba9d 100644 --- a/packages/language-server/test/plugins/typescript/features/diagnostics/index.test.ts +++ b/packages/language-server/test/plugins/typescript/features/diagnostics/index.test.ts @@ -59,7 +59,7 @@ async function executeTest( : defaultExpectedFile; const snapshotFormatter = await createJsonSnapshotFormatter(dir); - updateSnapshotIfFailedOrEmpty({ + await updateSnapshotIfFailedOrEmpty({ assertion() { assert.deepStrictEqual(diagnostics, JSON.parse(readFileSync(expectedFile, 'utf-8'))); }, diff --git a/packages/language-server/test/plugins/typescript/features/inlayHints/index.test.ts b/packages/language-server/test/plugins/typescript/features/inlayHints/index.test.ts index c6a3687ad..295db9a32 100644 --- a/packages/language-server/test/plugins/typescript/features/inlayHints/index.test.ts +++ b/packages/language-server/test/plugins/typescript/features/inlayHints/index.test.ts @@ -71,7 +71,7 @@ async function executeTest( const snapshotFormatter = await createJsonSnapshotFormatter(dir); - updateSnapshotIfFailedOrEmpty({ + await updateSnapshotIfFailedOrEmpty({ assertion() { assert.deepStrictEqual( JSON.parse(JSON.stringify(inlayHints)), diff --git a/packages/language-server/test/plugins/typescript/test-utils.ts b/packages/language-server/test/plugins/typescript/test-utils.ts index b3c3c5bbc..68a956abc 100644 --- a/packages/language-server/test/plugins/typescript/test-utils.ts +++ b/packages/language-server/test/plugins/typescript/test-utils.ts @@ -207,7 +207,7 @@ export function createSnapshotTester< } } -export function updateSnapshotIfFailedOrEmpty({ +export async function updateSnapshotIfFailedOrEmpty({ assertion, expectedFile, rootDir, @@ -216,25 +216,25 @@ export function updateSnapshotIfFailedOrEmpty({ assertion: () => void; expectedFile: string; rootDir: string; - getFileContent: () => string; + getFileContent: () => string | Promise; }) { if (existsSync(expectedFile)) { try { assertion(); } catch (e) { if (process.argv.includes('--auto')) { - writeFile(`Updated ${expectedFile} for`); + await writeFile(`Updated ${expectedFile} for`); } else { throw e; } } } else { - writeFile(`Created ${expectedFile} for`); + await writeFile(`Created ${expectedFile} for`); } - function writeFile(msg: string) { + async function writeFile(msg: string) { console.info(msg, dirname(expectedFile).substring(rootDir.length)); - writeFileSync(expectedFile, getFileContent(), 'utf-8'); + writeFileSync(expectedFile, await getFileContent(), 'utf-8'); } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 750f7eb91..63b680d51 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -16,8 +16,8 @@ importers: specifier: ^7.0.2 version: 7.0.3 prettier: - specifier: 2.8.6 - version: 2.8.6 + specifier: ~2.8.8 + version: 2.8.8 ts-node: specifier: ^10.0.0 version: 10.9.1(@types/node@16.18.32)(typescript@5.1.3) @@ -43,11 +43,11 @@ importers: specifier: ^4.17.21 version: 4.17.21 prettier: - specifier: 2.8.6 - version: 2.8.6 + specifier: ~2.8.8 + version: 2.8.8 prettier-plugin-svelte: specifier: ~2.10.1 - version: 2.10.1(prettier@2.8.6)(svelte@3.57.0) + version: 2.10.1(prettier@2.8.8)(svelte@3.57.0) svelte: specifier: ^3.57.0 version: 3.57.0 @@ -1566,18 +1566,18 @@ packages: resolution: {integrity: sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA==} engines: {node: '>=8.6'} - /prettier-plugin-svelte@2.10.1(prettier@2.8.6)(svelte@3.57.0): + /prettier-plugin-svelte@2.10.1(prettier@2.8.8)(svelte@3.57.0): resolution: {integrity: sha512-Wlq7Z5v2ueCubWo0TZzKc9XHcm7TDxqcuzRuGd0gcENfzfT4JZ9yDlCbEgxWgiPmLHkBjfOtpAWkcT28MCDpUQ==} peerDependencies: prettier: ^1.16.4 || ^2.0.0 svelte: ^3.2.0 || ^4.0.0-next.0 dependencies: - prettier: 2.8.6 + prettier: 2.8.8 svelte: 3.57.0 dev: false - /prettier@2.8.6: - resolution: {integrity: sha512-mtuzdiBbHwPEgl7NxWlqOkithPyp4VN93V7VeHVWBF+ad3I5avc0RVDT4oImXQy9H/AqxA2NSQH8pSxHW6FYbQ==} + /prettier@2.8.8: + resolution: {integrity: sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q==} engines: {node: '>=10.13.0'} hasBin: true