From 5ccc8854be350278515373f4393edacd7eccffce Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Tue, 13 Sep 2022 14:00:47 -0400 Subject: [PATCH 1/2] Don't emit utilities containing invalid theme keys --- .gitignore | 3 + src/lib/evaluateTailwindFunctions.js | 23 +++++++- src/lib/setupContextUtils.js | 49 ++++++++++++++++ tests/evaluateTailwindFunctions.test.js | 77 ++++++++++++++++++++++--- 4 files changed, 144 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index fc3080cf85f2..5d729b88c82f 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,6 @@ isolate*.log # Generated files /src/corePluginList.js + +# Generated files during tests +/tests/evaluate-tailwind-functions.test.html diff --git a/src/lib/evaluateTailwindFunctions.js b/src/lib/evaluateTailwindFunctions.js index 327581493bb2..7e04ef428174 100644 --- a/src/lib/evaluateTailwindFunctions.js +++ b/src/lib/evaluateTailwindFunctions.js @@ -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 @@ -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( @@ -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) } diff --git a/src/lib/setupContextUtils.js b/src/lib/setupContextUtils.js index e8519ad95f9f..cb3bd1736b39 100644 --- a/src/lib/setupContextUtils.js +++ b/src/lib/setupContextUtils.js @@ -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: [], @@ -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) diff --git a/tests/evaluateTailwindFunctions.test.js b/tests/evaluateTailwindFunctions.test.js index 0214ba113383..8fa950b16e45 100644 --- a/tests/evaluateTailwindFunctions.test.js +++ b/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', () => { @@ -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` +
+
+ ` + ) + + // 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']) + }) +}) From 5ab44148d7447dcd37c8117a0d1daedf826762f1 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Wed, 14 Sep 2022 13:21:10 -0400 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1af2180d1abb..95e21db552d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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