Skip to content

Commit

Permalink
Properly validate theme(…) function paths in v4 (#1074)
Browse files Browse the repository at this point in the history
In v4 currently using the theme function in your CSS will show
diagnostics about the theme config keys not existing regardless of the
theme key.

We've done two things here:
- We compile a candidate with the theme function to see if the theme key
exists. While this isn't an optimal solution it works without needing to
introduce new APIs into v4 — also prevents us from having to expose the
legacy resolved config anywhere in v4.
- We look at all currently registered theme keys to offer suggestions in
case you type a theme key incorrectly _as long as it's a CSS variable_.
In v4 suggestions for legacy theme config values is not implemented.

---------

Co-authored-by: Robin Malfait <[email protected]>
  • Loading branch information
thecrypticace and RobinMalfait authored Nov 8, 2024
1 parent 8a39555 commit 5798007
Show file tree
Hide file tree
Showing 8 changed files with 343 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,23 @@ withFixture('v4/basic', (c) => {
})
}

function testInline(fixture, { code, expected, language = 'html' }) {
test(fixture, async () => {
let promise = new Promise((resolve) => {
c.onNotification('textDocument/publishDiagnostics', ({ diagnostics }) => {
resolve(diagnostics)
})
})

let doc = await c.openDocument({ text: code, lang: language })
let diagnostics = await promise

expected = JSON.parse(JSON.stringify(expected).replaceAll('{{URI}}', doc.uri))

expect(diagnostics).toEqual(expected)
})
}

testFixture('css-conflict/simple')
testFixture('css-conflict/variants-negative')
testFixture('css-conflict/variants-positive')
Expand All @@ -69,5 +86,171 @@ withFixture('v4/basic', (c) => {
// testFixture('css-conflict/css-multi-rule')
// testFixture('css-conflict/css-multi-prop')
// testFixture('invalid-screen/simple')
// testFixture('invalid-theme/simple')

testInline('simple typos in theme keys (in key)', {
code: '.test { color: theme(--color-red-901) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 36 } },
severity: 1,
message: "'--color-red-901' does not exist in your theme. Did you mean '--color-red-900'?",
suggestions: ['--color-red-900'],
},
],
})

testInline('simple typos in theme keys (in namespace)', {
code: '.test { color: theme(--colors-red-901) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 37 } },
severity: 1,
message: "'--colors-red-901' does not exist in your theme. Did you mean '--color-red-900'?",
suggestions: ['--color-red-900'],
},
],
})

testInline('No similar theme key exists', {
code: '.test { color: theme(--font-obliqueness-90) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 42 } },
severity: 1,
message: "'--font-obliqueness-90' does not exist in your theme.",
suggestions: [],
},
],
})

testInline('valid theme keys dont issue diagnostics', {
code: '.test { color: theme(--color-red-900) }',
language: 'css',
expected: [],
})

testInline('types in legacy theme config paths', {
code: '.test { color: theme(colors.red.901) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 35 } },
severity: 1,
message: "'colors.red.901' does not exist in your theme config.",
suggestions: [],
},
],
})

testInline('valid legacy theme config paths', {
code: '.test { color: theme(colors.red.900) }',
language: 'css',
expected: [],
})
})

withFixture('v4/with-prefix', (c) => {
function testInline(fixture, { code, expected, language = 'html' }) {
test(fixture, async () => {
let promise = new Promise((resolve) => {
c.onNotification('textDocument/publishDiagnostics', ({ diagnostics }) => {
resolve(diagnostics)
})
})

let doc = await c.openDocument({ text: code, lang: language })
let diagnostics = await promise

expected = JSON.parse(JSON.stringify(expected).replaceAll('{{URI}}', doc.uri))

expect(diagnostics).toEqual(expected)
})
}

// testFixture('css-conflict/simple')
// testFixture('css-conflict/variants-negative')
// testFixture('css-conflict/variants-positive')
// testFixture('css-conflict/jsx-concat-negative')
// testFixture('css-conflict/jsx-concat-positive')
// testFixture('css-conflict/vue-style-lang-sass')

// testFixture('css-conflict/css')
// testFixture('css-conflict/css-multi-rule')
// testFixture('css-conflict/css-multi-prop')
// testFixture('invalid-screen/simple')

testInline('simple typos in theme keys (in key)', {
code: '.test { color: theme(--color-red-901) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 36 } },
severity: 1,
message: "'--color-red-901' does not exist in your theme. Did you mean '--color-red-900'?",
suggestions: ['--color-red-900'],
},
],
})

testInline('simple typos in theme keys (in namespace)', {
code: '.test { color: theme(--colors-red-901) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 37 } },
severity: 1,
message: "'--colors-red-901' does not exist in your theme. Did you mean '--color-red-900'?",
suggestions: ['--color-red-900'],
},
],
})

testInline('No similar theme key exists', {
code: '.test { color: theme(--font-obliqueness-90) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 42 } },
severity: 1,
message: "'--font-obliqueness-90' does not exist in your theme.",
suggestions: [],
},
],
})

testInline('valid theme keys dont issue diagnostics', {
code: '.test { color: theme(--color-red-900) }',
language: 'css',
expected: [],
})

testInline('types in legacy theme config paths', {
code: '.test { color: theme(colors.red.901) }',
language: 'css',
expected: [
{
code: 'invalidConfigPath',
range: { start: { line: 0, character: 21 }, end: { line: 0, character: 35 } },
severity: 1,
message: "'colors.red.901' does not exist in your theme config.",
suggestions: [],
},
],
})

testInline('valid legacy theme config paths', {
code: '.test { color: theme(colors.red.900) }',
language: 'css',
expected: [],
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@import 'tailwindcss';

@theme prefix(tw) {
--color-potato: #907a70;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"tailwindcss": "^4.0.0-alpha.30"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,15 @@ import { type InvalidConfigPathDiagnostic, DiagnosticKind } from './types'
import { findHelperFunctionsInDocument } from '../util/find'
import { stringToPath } from '../util/stringToPath'
import isObject from '../util/isObject'
import { closest } from '../util/closest'
import { closest, distance } from '../util/closest'
import { combinations } from '../util/combinations'
import dlv from 'dlv'
import type { TextDocument } from 'vscode-languageserver-textdocument'
import type { DesignSystem } from '../util/v4'

type ValidationResult =
| { isValid: true; value: any }
| { isValid: false; reason: string; suggestions: string[] }

function pathToString(path: string | string[]): string {
if (typeof path === 'string') return path
Expand All @@ -21,8 +26,13 @@ export function validateConfigPath(
state: State,
path: string | string[],
base: string[] = [],
): { isValid: true; value: any } | { isValid: false; reason: string; suggestions: string[] } {
): ValidationResult {
let keys = Array.isArray(path) ? path : stringToPath(path)

if (state.v4) {
return validateV4ThemePath(state, pathToString(keys))
}

let fullPath = [...base, ...keys]
let value = dlv(state.config, fullPath)
let suggestions: string[] = []
Expand Down Expand Up @@ -195,3 +205,113 @@ export function getInvalidConfigPathDiagnostics(

return diagnostics
}

function resolveThemeValue(design: DesignSystem, path: string) {
let prefix = design.theme.prefix ?? null
let candidate = prefix ? `${prefix}:[--custom:theme(${path})]` : `[--custom:theme(${path})]`

// Compile a dummy candidate that uses the theme function with the given path.
//
// We'll get a rule with a declaration from which we read the value. No rule
// will be generated and the root will be empty if the path is invalid.
//
// Non-CSS representable values are not a concern here because the validation
// only happens for calls in a CSS context.
let [root] = design.compile([candidate])

let value: string | null = null

root.walkDecls((decl) => {
value = decl.value
})

return value
}

function resolveKnownThemeKeys(design: DesignSystem): string[] {
let validThemeKeys = Array.from(design.theme.entries(), ([key]) => key)

let prefixLength = design.theme.prefix?.length ?? 0

return prefixLength > 0
? // Strip the configured prefix from the list of valid theme keys
validThemeKeys.map((key) => `--${key.slice(prefixLength + 3)}`)
: validThemeKeys
}

function validateV4ThemePath(state: State, path: string): ValidationResult {
let prefix = state.designSystem.theme.prefix ?? null

let value = resolveThemeValue(state.designSystem, path)

if (value !== null && value !== undefined) {
return { isValid: true, value }
}

let reason = path.startsWith('--')
? `'${path}' does not exist in your theme.`
: `'${path}' does not exist in your theme config.`

let suggestions = suggestAlternativeThemeKeys(state, path)

if (suggestions.length > 0) {
reason += ` Did you mean '${suggestions[0]}'?`
}

return {
isValid: false,
reason,
suggestions,
}
}

function suggestAlternativeThemeKeys(state: State, path: string): string[] {
// Non-v4 projects don't contain CSS variable theme keys
if (!state.v4) return []

// v4 only supports suggesting keys currently known by the theme
// it does not support suggesting keys from the config as that is not
// exposed in any v4 API
if (!path.startsWith('--')) return []

let parts = path.slice(2).split('-')
parts[0] = `--${parts[0]}`

let validThemeKeys = resolveKnownThemeKeys(state.designSystem)
let potentialThemeKey: string | null = null

while (parts.length > 1) {
// Slice off the end of the theme key at the `-`
parts.pop()

// Look at all theme keys that start with that
let prefix = parts.join('-')

let possibleKeys = validThemeKeys.filter((key) => key.startsWith(prefix))

// If there are none, slice again and repeat
if (possibleKeys.length === 0) continue

// Find the closest match using the Fast String Distance (SIFT) algorithm
// ensuring `--color-red-901` suggests `--color-red-900` instead of
// `--color-red-950`. We could in theory use the algorithm directly but it
// does not make sense to suggest keys from an unrelated namespace which is
// why we do filtering beforehand.
potentialThemeKey = closest(path, possibleKeys)!

break
}

// If we haven't found a key based on prefix matching, we'll do one more
// search based on the full list of available keys. This is useful if the
// namespace itself has a typo.
potentialThemeKey ??= closest(path, validThemeKeys)!

// This number was chosen arbitrarily. From some light testing it seems like
// it's a decent threshold for determine if a key is a reasonable suggestion.
// This wasn't chosen by rigorous testing so if it needs to be adjusted it can
// be. Chances are it'll need to be increased instead of decreased.
const MAX_DISTANCE = 5

return distance(path, potentialThemeKey) <= MAX_DISTANCE ? [potentialThemeKey] : []
}
4 changes: 4 additions & 0 deletions packages/tailwindcss-language-service/src/util/closest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,7 @@ import sift from 'sift-string'
export function closest(input: string, options: string[]): string | undefined {
return options.concat([]).sort((a, b) => sift(input, a) - sift(input, b))[0]
}

export function distance(a: string, b: string): number {
return sift(a, b)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import postcss from 'postcss'
import type { Rule } from './ast'
import type { NamedVariant } from './candidate'

export interface Theme {}
export interface Theme {
// Prefix didn't exist on
prefix?: string
entries(): [string, any][]
}

export interface ClassMetadata {
modifiers: string[]
Expand Down
Loading

0 comments on commit 5798007

Please sign in to comment.