Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't emit utilities containing invalid theme fn keys #9319

Merged
merged 2 commits into from Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Expand Up @@ -17,3 +17,6 @@ isolate*.log

# Generated files
/src/corePluginList.js

# Generated files during tests
/tests/evaluate-tailwind-functions.test.html
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -32,6 +32,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix ordering of parallel variants ([#9282](https://github.com/tailwindlabs/tailwindcss/pull/9282))
- Handle variants in utility selectors using `:where()` and `:has()` ([#9309](https://github.com/tailwindlabs/tailwindcss/pull/9309))
- Improve data type analyses for arbitrary values ([#9320](https://github.com/tailwindlabs/tailwindcss/pull/9320))
- Don't emit generated utilities with invalid uses of theme functions ([#9319](https://github.com/tailwindlabs/tailwindcss/pull/9319))

## [3.1.8] - 2022-08-05

Expand Down
23 changes: 22 additions & 1 deletion src/lib/evaluateTailwindFunctions.js
Expand Up @@ -7,6 +7,7 @@ import buildMediaQuery from '../util/buildMediaQuery'
import { toPath } from '../util/toPath'
import { withAlphaValue } from '../util/withAlphaVariable'
import { parseColorFormat } from '../util/pluginUtils'
import log from '../util/log'

function isObject(input) {
return typeof input === 'object' && input !== null
Expand Down Expand Up @@ -196,7 +197,9 @@ function resolvePath(config, path, defaultValue) {
return results.find((result) => result.isValid) ?? results[0]
}

export default function ({ tailwindConfig: config }) {
export default function (context) {
let config = context.tailwindConfig

let functions = {
theme: (node, path, ...defaultValue) => {
let { isValid, value, error, alpha } = resolvePath(
Expand All @@ -206,6 +209,24 @@ export default function ({ tailwindConfig: config }) {
)

if (!isValid) {
let parentNode = node.parent
let candidate = parentNode?.raws.tailwind?.candidate

if (parentNode && candidate !== undefined) {
// Remove this utility from any caches
context.markInvalidUtilityNode(parentNode)

// Remove the CSS node from the markup
parentNode.remove()

// Show a warning
log.warn('invalid-theme-key-in-class', [
`The utility \`${candidate}\` contains an invalid theme value and was not generated.`,
])

return
}

throw node.error(error)
}

Expand Down
49 changes: 49 additions & 0 deletions src/lib/setupContextUtils.js
Expand Up @@ -856,6 +856,52 @@ function registerPlugins(plugins, context) {
}
}

/**
* Mark as class as retroactively invalid
*
*
* @param {string} candidate
*/
function markInvalidUtilityCandidate(context, candidate) {
if (!context.classCache.has(candidate)) {
return
}

// Mark this as not being a real utility
context.notClassCache.add(candidate)

// Remove it from any candidate-specific caches
context.classCache.delete(candidate)
context.applyClassCache.delete(candidate)
context.candidateRuleMap.delete(candidate)
context.candidateRuleCache.delete(candidate)

// Ensure the stylesheet gets rebuilt
context.stylesheetCache = null
}

/**
* Mark as class as retroactively invalid
*
* @param {import('postcss').Node} node
*/
function markInvalidUtilityNode(context, node) {
let candidate = node.raws.tailwind.candidate

if (!candidate) {
return
}

for (const entry of context.ruleCache) {
if (entry[1].raws.tailwind.candidate === candidate) {
context.ruleCache.delete(entry)
// context.postCssNodeCache.delete(node)
}
}

markInvalidUtilityCandidate(context, candidate)
}

export function createContext(tailwindConfig, changedContent = [], root = postcss.root()) {
let context = {
disposables: [],
Expand All @@ -870,6 +916,9 @@ export function createContext(tailwindConfig, changedContent = [], root = postcs
changedContent: changedContent,
variantMap: new Map(),
stylesheetCache: null,

markInvalidUtilityCandidate: (candidate) => markInvalidUtilityCandidate(context, candidate),
markInvalidUtilityNode: (node) => markInvalidUtilityNode(context, node),
}

let resolvedPlugins = resolvePlugins(context, root)
Expand Down
77 changes: 70 additions & 7 deletions tests/evaluateTailwindFunctions.test.js
@@ -1,14 +1,18 @@
import fs from 'fs'
import path from 'path'
import postcss from 'postcss'
import plugin from '../src/lib/evaluateTailwindFunctions'
import tailwind from '../src/index'
import { css } from './util/run'
import { run as runFull, css, html } from './util/run'

function run(input, opts = {}) {
return postcss([plugin({ tailwindConfig: opts })]).process(input, { from: undefined })
}

function runFull(input, config) {
return postcss([tailwind(config)]).process(input, { from: undefined })
return postcss([
plugin({
tailwindConfig: opts,
markInvalidUtilityNode() {
// no op
},
}),
]).process(input, { from: undefined })
}

test('it looks up values in the theme using dot notation', () => {
Expand Down Expand Up @@ -1222,3 +1226,62 @@ it('can find values with slashes in the theme key while still allowing for alpha
`)
})
})

describe('context dependent', () => {
let configPath = path.resolve(__dirname, './evaluate-tailwind-functions.tailwind.config.js')
let filePath = path.resolve(__dirname, './evaluate-tailwind-functions.test.html')
let config = {
content: [filePath],
corePlugins: { preflight: false },
}

// Rebuild the config file for each test
beforeEach(() => fs.promises.writeFile(configPath, `module.exports = ${JSON.stringify(config)};`))
afterEach(() => fs.promises.unlink(configPath))

let warn

beforeEach(() => {
warn = jest.spyOn(require('../src/util/log').default, 'warn')
})

afterEach(() => warn.mockClear())

it('should not generate when theme fn doesnt resolve', async () => {
await fs.promises.writeFile(
filePath,
html`
<div class="underline [--box-shadow:theme('boxShadow.doesnotexist')]"></div>
<div class="bg-[theme('boxShadow.doesnotexist')]"></div>
`
)

// TODO: We need a way to reuse the context in our test suite without requiring writing to files
// It should be an explicit thing tho — like we create a context and pass it in or something
let result = await runFull('@tailwind utilities', configPath)

// 1. On first run it should work because it's been removed from the class cache
expect(result.css).toMatchCss(css`
.underline {
text-decoration-line: underline;
}
`)

// 2. But we get a warning in the console
expect(warn).toHaveBeenCalledTimes(1)
expect(warn.mock.calls.map((x) => x[0])).toEqual(['invalid-theme-key-in-class'])

// 3. The second run should work fine because it's been removed from the class cache
result = await runFull('@tailwind utilities', configPath)

expect(result.css).toMatchCss(css`
.underline {
text-decoration-line: underline;
}
`)

// 4. But we've not received any further logs about it
expect(warn).toHaveBeenCalledTimes(1)
expect(warn.mock.calls.map((x) => x[0])).toEqual(['invalid-theme-key-in-class'])
})
})