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 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Don't emit utilities containing invalid theme keys
  • Loading branch information
thecrypticace committed Sep 14, 2022
commit 5ccc8854be350278515373f4393edacd7eccffce
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ isolate*.log

# Generated files
/src/corePluginList.js

# Generated files during tests
/tests/evaluate-tailwind-functions.test.html
23 changes: 22 additions & 1 deletion src/lib/evaluateTailwindFunctions.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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'])
})
})