From 2caebd1d48a0f7f2d72dda888324bd4b0d496140 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Fri, 27 Oct 2023 13:44:04 -0400 Subject: [PATCH] Remove duplicate `variant` + `value` pairs from completions (#874) * Refactor * Support using multiple fixtures in a single test file * Add test * Remove duplicate `variant` + `value` pairs from completions * Update changelog --- .../tests/common.js | 41 ++--- .../tests/completions/completions.test.js | 32 ++++ .../overrides-variants/tailwind.config.js | 8 + .../src/completionProvider.ts | 150 ++++++++++-------- packages/vscode-tailwindcss/CHANGELOG.md | 1 + 5 files changed, 133 insertions(+), 99 deletions(-) create mode 100644 packages/tailwindcss-language-server/tests/fixtures/overrides-variants/tailwind.config.js diff --git a/packages/tailwindcss-language-server/tests/common.js b/packages/tailwindcss-language-server/tests/common.js index 7202e73..abdc8cc 100644 --- a/packages/tailwindcss-language-server/tests/common.js +++ b/packages/tailwindcss-language-server/tests/common.js @@ -3,13 +3,11 @@ import * as cp from 'node:child_process' import * as rpc from 'vscode-jsonrpc' import { beforeAll } from 'vitest' -let settings = {} -let initPromise -let childProcess -let docSettings = new Map() - async function init(fixture) { - childProcess = cp.fork('./bin/tailwindcss-language-server', { silent: true }) + let settings = {} + let docSettings = new Map() + + let childProcess = cp.fork('./bin/tailwindcss-language-server', { silent: true }) const capabilities = { textDocument: { @@ -116,7 +114,7 @@ async function init(fixture) { }) }) - initPromise = new Promise((resolve) => { + let initPromise = new Promise((resolve) => { connection.onRequest(new rpc.RequestType('client/registerCapability'), ({ registrations }) => { if (registrations.findIndex((r) => r.method === 'textDocument/completion') > -1) { resolve() @@ -177,33 +175,18 @@ async function init(fixture) { } export function withFixture(fixture, callback) { - let c + let c = {} beforeAll(async () => { - c = await init(fixture) + // Using the connection object as the prototype lets us access the connection + // without defining getters for all the methods and also lets us add helpers + // to the connection object without having to resort to using a Proxy + Object.setPrototypeOf(c, await init(fixture)) + return () => c.connection.end() }) - callback({ - get connection() { - return c.connection - }, - get sendRequest() { - return c.sendRequest - }, - get onNotification() { - return c.onNotification - }, - get openDocument() { - return c.openDocument - }, - get updateSettings() { - return c.updateSettings - }, - get updateFile() { - return c.updateFile - }, - }) + callback(c) } // let counter = 0 diff --git a/packages/tailwindcss-language-server/tests/completions/completions.test.js b/packages/tailwindcss-language-server/tests/completions/completions.test.js index df64d38..996518a 100644 --- a/packages/tailwindcss-language-server/tests/completions/completions.test.js +++ b/packages/tailwindcss-language-server/tests/completions/completions.test.js @@ -119,3 +119,35 @@ withFixture('basic', (c) => { }) }) }) + +withFixture('overrides-variants', (c) => { + async function completion({ + lang, + text, + position, + context = { + triggerKind: 1, + }, + settings, + }) { + let textDocument = await c.openDocument({ text, lang, settings }) + + return c.sendRequest('textDocument/completion', { + textDocument, + position, + context, + }) + } + + test.concurrent( + 'duplicate variant + value pairs do not produce multiple completions', + async () => { + let result = await completion({ + text: '
', + position: { line: 0, character: 23 }, + }) + + expect(result.items.filter((item) => item.label.endsWith('custom-hover:')).length).toBe(1) + } + ) +}) diff --git a/packages/tailwindcss-language-server/tests/fixtures/overrides-variants/tailwind.config.js b/packages/tailwindcss-language-server/tests/fixtures/overrides-variants/tailwind.config.js new file mode 100644 index 0000000..22e28cd --- /dev/null +++ b/packages/tailwindcss-language-server/tests/fixtures/overrides-variants/tailwind.config.js @@ -0,0 +1,8 @@ +module.exports = { + plugins: [ + function ({ addVariant, matchVariant }) { + matchVariant('custom', (value) => `.custom:${value} &`, { values: { hover: 'hover' } }) + addVariant('custom-hover', `.custom:hover &:hover`) + }, + ], +} diff --git a/packages/tailwindcss-language-service/src/completionProvider.ts b/packages/tailwindcss-language-service/src/completionProvider.ts index 62ab9e5..6468b93 100644 --- a/packages/tailwindcss-language-service/src/completionProvider.ts +++ b/packages/tailwindcss-language-service/src/completionProvider.ts @@ -138,6 +138,7 @@ export function completionsFromClassList( } let items: CompletionItem[] = [] + let seenVariants = new Set() if (!important) { let variantOrder = 0 @@ -163,85 +164,94 @@ export function completionsFromClassList( } } - items.push( - ...state.variants.flatMap((variant) => { - let items: CompletionItem[] = [] + for (let variant of state.variants) { + if (existingVariants.includes(variant.name)) { + continue + } - if (variant.isArbitrary) { - items.push( - variantItem({ - label: `${variant.name}${variant.hasDash ? '-' : ''}[]${sep}`, - insertTextFormat: 2, - textEditText: `${variant.name}${variant.hasDash ? '-' : ''}[\${1}]${sep}\${0}`, - // command: { - // title: '', - // command: 'tailwindCSS.onInsertArbitraryVariantSnippet', - // arguments: [variant.name, replacementRange], - // }, - }) + if (seenVariants.has(variant.name)) { + continue + } + + seenVariants.add(variant.name) + + if (variant.isArbitrary) { + items.push( + variantItem({ + label: `${variant.name}${variant.hasDash ? '-' : ''}[]${sep}`, + insertTextFormat: 2, + textEditText: `${variant.name}${variant.hasDash ? '-' : ''}[\${1}]${sep}\${0}`, + // command: { + // title: '', + // command: 'tailwindCSS.onInsertArbitraryVariantSnippet', + // arguments: [variant.name, replacementRange], + // }, + }) + ) + } else { + let shouldSortVariants = !semver.gte(state.version, '2.99.0') + let resultingVariants = [...existingVariants, variant.name] + + if (shouldSortVariants) { + let allVariants = state.variants.map(({ name }) => name) + resultingVariants = resultingVariants.sort( + (a, b) => allVariants.indexOf(b) - allVariants.indexOf(a) ) - } else if (!existingVariants.includes(variant.name)) { - let shouldSortVariants = !semver.gte(state.version, '2.99.0') - let resultingVariants = [...existingVariants, variant.name] + } - if (shouldSortVariants) { - let allVariants = state.variants.map(({ name }) => name) - resultingVariants = resultingVariants.sort( - (a, b) => allVariants.indexOf(b) - allVariants.indexOf(a) - ) - } - - items.push( - variantItem({ - label: `${variant.name}${sep}`, - detail: variant - .selectors() - .map((selector) => addPixelEquivalentsToMediaQuery(selector, rootFontSize)) - .join(', '), - textEditText: resultingVariants[resultingVariants.length - 1] + sep, - additionalTextEdits: - shouldSortVariants && resultingVariants.length > 1 - ? [ - { - newText: - resultingVariants.slice(0, resultingVariants.length - 1).join(sep) + - sep, - range: { - start: { - ...classListRange.start, - character: classListRange.end.character - partialClassName.length, - }, - end: { - ...replacementRange.start, - character: replacementRange.start.character, - }, + items.push( + variantItem({ + label: `${variant.name}${sep}`, + detail: variant + .selectors() + .map((selector) => addPixelEquivalentsToMediaQuery(selector, rootFontSize)) + .join(', '), + textEditText: resultingVariants[resultingVariants.length - 1] + sep, + additionalTextEdits: + shouldSortVariants && resultingVariants.length > 1 + ? [ + { + newText: + resultingVariants.slice(0, resultingVariants.length - 1).join(sep) + sep, + range: { + start: { + ...classListRange.start, + character: classListRange.end.character - partialClassName.length, + }, + end: { + ...replacementRange.start, + character: replacementRange.start.character, }, }, - ] - : [], - }) - ) + }, + ] + : [], + }) + ) + } + + for (let value of variant.values ?? []) { + if (existingVariants.includes(`${variant.name}-${value}`)) { + continue } - if (variant.values.length) { - items.push( - ...variant.values - .filter((value) => !existingVariants.includes(`${variant.name}-${value}`)) - .map((value) => - variantItem({ - label: - value === 'DEFAULT' - ? `${variant.name}${sep}` - : `${variant.name}${variant.hasDash ? '-' : ''}${value}${sep}`, - detail: variant.selectors({ value }).join(', '), - }) - ) - ) + if (seenVariants.has(`${variant.name}-${value}`)) { + continue } - return items - }) - ) + seenVariants.add(`${variant.name}-${value}`) + + items.push( + variantItem({ + label: + value === 'DEFAULT' + ? `${variant.name}${sep}` + : `${variant.name}${variant.hasDash ? '-' : ''}${value}${sep}`, + detail: variant.selectors({ value }).join(', '), + }) + ) + } + } } if (state.classList) { diff --git a/packages/vscode-tailwindcss/CHANGELOG.md b/packages/vscode-tailwindcss/CHANGELOG.md index 176d09b..b5a9adb 100644 --- a/packages/vscode-tailwindcss/CHANGELOG.md +++ b/packages/vscode-tailwindcss/CHANGELOG.md @@ -3,6 +3,7 @@ ## 0.11.x (Pre-Release) - Add support for Glimmer (#867) +- Ignore duplicate variant + value pairs (#874) ## 0.10.1