Skip to content

Commit 2caebd1

Browse files
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
1 parent a13708b commit 2caebd1

File tree

5 files changed

+134
-100
lines changed

5 files changed

+134
-100
lines changed

packages/tailwindcss-language-server/tests/common.js

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ import * as cp from 'node:child_process'
33
import * as rpc from 'vscode-jsonrpc'
44
import { beforeAll } from 'vitest'
55

6-
let settings = {}
7-
let initPromise
8-
let childProcess
9-
let docSettings = new Map()
10-
116
async function init(fixture) {
12-
childProcess = cp.fork('./bin/tailwindcss-language-server', { silent: true })
7+
let settings = {}
8+
let docSettings = new Map()
9+
10+
let childProcess = cp.fork('./bin/tailwindcss-language-server', { silent: true })
1311

1412
const capabilities = {
1513
textDocument: {
@@ -116,7 +114,7 @@ async function init(fixture) {
116114
})
117115
})
118116

119-
initPromise = new Promise((resolve) => {
117+
let initPromise = new Promise((resolve) => {
120118
connection.onRequest(new rpc.RequestType('client/registerCapability'), ({ registrations }) => {
121119
if (registrations.findIndex((r) => r.method === 'textDocument/completion') > -1) {
122120
resolve()
@@ -177,33 +175,18 @@ async function init(fixture) {
177175
}
178176

179177
export function withFixture(fixture, callback) {
180-
let c
178+
let c = {}
181179

182180
beforeAll(async () => {
183-
c = await init(fixture)
181+
// Using the connection object as the prototype lets us access the connection
182+
// without defining getters for all the methods and also lets us add helpers
183+
// to the connection object without having to resort to using a Proxy
184+
Object.setPrototypeOf(c, await init(fixture))
185+
184186
return () => c.connection.end()
185187
})
186188

187-
callback({
188-
get connection() {
189-
return c.connection
190-
},
191-
get sendRequest() {
192-
return c.sendRequest
193-
},
194-
get onNotification() {
195-
return c.onNotification
196-
},
197-
get openDocument() {
198-
return c.openDocument
199-
},
200-
get updateSettings() {
201-
return c.updateSettings
202-
},
203-
get updateFile() {
204-
return c.updateFile
205-
},
206-
})
189+
callback(c)
207190
}
208191

209192
// let counter = 0

packages/tailwindcss-language-server/tests/completions/completions.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,35 @@ withFixture('basic', (c) => {
119119
})
120120
})
121121
})
122+
123+
withFixture('overrides-variants', (c) => {
124+
async function completion({
125+
lang,
126+
text,
127+
position,
128+
context = {
129+
triggerKind: 1,
130+
},
131+
settings,
132+
}) {
133+
let textDocument = await c.openDocument({ text, lang, settings })
134+
135+
return c.sendRequest('textDocument/completion', {
136+
textDocument,
137+
position,
138+
context,
139+
})
140+
}
141+
142+
test.concurrent(
143+
'duplicate variant + value pairs do not produce multiple completions',
144+
async () => {
145+
let result = await completion({
146+
text: '<div class="custom-hover"></div>',
147+
position: { line: 0, character: 23 },
148+
})
149+
150+
expect(result.items.filter((item) => item.label.endsWith('custom-hover:')).length).toBe(1)
151+
}
152+
)
153+
})
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
module.exports = {
2+
plugins: [
3+
function ({ addVariant, matchVariant }) {
4+
matchVariant('custom', (value) => `.custom:${value} &`, { values: { hover: 'hover' } })
5+
addVariant('custom-hover', `.custom:hover &:hover`)
6+
},
7+
],
8+
}

packages/tailwindcss-language-service/src/completionProvider.ts

Lines changed: 81 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export function completionsFromClassList(
138138
}
139139

140140
let items: CompletionItem[] = []
141+
let seenVariants = new Set<string>()
141142

142143
if (!important) {
143144
let variantOrder = 0
@@ -163,85 +164,94 @@ export function completionsFromClassList(
163164
}
164165
}
165166

166-
items.push(
167-
...state.variants.flatMap((variant) => {
168-
let items: CompletionItem[] = []
169-
170-
if (variant.isArbitrary) {
171-
items.push(
172-
variantItem({
173-
label: `${variant.name}${variant.hasDash ? '-' : ''}[]${sep}`,
174-
insertTextFormat: 2,
175-
textEditText: `${variant.name}${variant.hasDash ? '-' : ''}[\${1}]${sep}\${0}`,
176-
// command: {
177-
// title: '',
178-
// command: 'tailwindCSS.onInsertArbitraryVariantSnippet',
179-
// arguments: [variant.name, replacementRange],
180-
// },
181-
})
167+
for (let variant of state.variants) {
168+
if (existingVariants.includes(variant.name)) {
169+
continue
170+
}
171+
172+
if (seenVariants.has(variant.name)) {
173+
continue
174+
}
175+
176+
seenVariants.add(variant.name)
177+
178+
if (variant.isArbitrary) {
179+
items.push(
180+
variantItem({
181+
label: `${variant.name}${variant.hasDash ? '-' : ''}[]${sep}`,
182+
insertTextFormat: 2,
183+
textEditText: `${variant.name}${variant.hasDash ? '-' : ''}[\${1}]${sep}\${0}`,
184+
// command: {
185+
// title: '',
186+
// command: 'tailwindCSS.onInsertArbitraryVariantSnippet',
187+
// arguments: [variant.name, replacementRange],
188+
// },
189+
})
190+
)
191+
} else {
192+
let shouldSortVariants = !semver.gte(state.version, '2.99.0')
193+
let resultingVariants = [...existingVariants, variant.name]
194+
195+
if (shouldSortVariants) {
196+
let allVariants = state.variants.map(({ name }) => name)
197+
resultingVariants = resultingVariants.sort(
198+
(a, b) => allVariants.indexOf(b) - allVariants.indexOf(a)
182199
)
183-
} else if (!existingVariants.includes(variant.name)) {
184-
let shouldSortVariants = !semver.gte(state.version, '2.99.0')
185-
let resultingVariants = [...existingVariants, variant.name]
186-
187-
if (shouldSortVariants) {
188-
let allVariants = state.variants.map(({ name }) => name)
189-
resultingVariants = resultingVariants.sort(
190-
(a, b) => allVariants.indexOf(b) - allVariants.indexOf(a)
191-
)
192-
}
200+
}
193201

194-
items.push(
195-
variantItem({
196-
label: `${variant.name}${sep}`,
197-
detail: variant
198-
.selectors()
199-
.map((selector) => addPixelEquivalentsToMediaQuery(selector, rootFontSize))
200-
.join(', '),
201-
textEditText: resultingVariants[resultingVariants.length - 1] + sep,
202-
additionalTextEdits:
203-
shouldSortVariants && resultingVariants.length > 1
204-
? [
205-
{
206-
newText:
207-
resultingVariants.slice(0, resultingVariants.length - 1).join(sep) +
208-
sep,
209-
range: {
210-
start: {
211-
...classListRange.start,
212-
character: classListRange.end.character - partialClassName.length,
213-
},
214-
end: {
215-
...replacementRange.start,
216-
character: replacementRange.start.character,
217-
},
202+
items.push(
203+
variantItem({
204+
label: `${variant.name}${sep}`,
205+
detail: variant
206+
.selectors()
207+
.map((selector) => addPixelEquivalentsToMediaQuery(selector, rootFontSize))
208+
.join(', '),
209+
textEditText: resultingVariants[resultingVariants.length - 1] + sep,
210+
additionalTextEdits:
211+
shouldSortVariants && resultingVariants.length > 1
212+
? [
213+
{
214+
newText:
215+
resultingVariants.slice(0, resultingVariants.length - 1).join(sep) + sep,
216+
range: {
217+
start: {
218+
...classListRange.start,
219+
character: classListRange.end.character - partialClassName.length,
220+
},
221+
end: {
222+
...replacementRange.start,
223+
character: replacementRange.start.character,
218224
},
219225
},
220-
]
221-
: [],
222-
})
223-
)
226+
},
227+
]
228+
: [],
229+
})
230+
)
231+
}
232+
233+
for (let value of variant.values ?? []) {
234+
if (existingVariants.includes(`${variant.name}-${value}`)) {
235+
continue
224236
}
225237

226-
if (variant.values.length) {
227-
items.push(
228-
...variant.values
229-
.filter((value) => !existingVariants.includes(`${variant.name}-${value}`))
230-
.map((value) =>
231-
variantItem({
232-
label:
233-
value === 'DEFAULT'
234-
? `${variant.name}${sep}`
235-
: `${variant.name}${variant.hasDash ? '-' : ''}${value}${sep}`,
236-
detail: variant.selectors({ value }).join(', '),
237-
})
238-
)
239-
)
238+
if (seenVariants.has(`${variant.name}-${value}`)) {
239+
continue
240240
}
241241

242-
return items
243-
})
244-
)
242+
seenVariants.add(`${variant.name}-${value}`)
243+
244+
items.push(
245+
variantItem({
246+
label:
247+
value === 'DEFAULT'
248+
? `${variant.name}${sep}`
249+
: `${variant.name}${variant.hasDash ? '-' : ''}${value}${sep}`,
250+
detail: variant.selectors({ value }).join(', '),
251+
})
252+
)
253+
}
254+
}
245255
}
246256

247257
if (state.classList) {

packages/vscode-tailwindcss/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## 0.11.x (Pre-Release)
44

55
- Add support for Glimmer (#867)
6+
- Ignore duplicate variant + value pairs (#874)
67

78
## 0.10.1
89

0 commit comments

Comments
 (0)