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

Warn when broad glob patterns are used in the content configuration #14140

Merged
merged 12 commits into from
Aug 7, 2024
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

- Fix minification when using nested CSS ([#14105](https://github.com/tailwindlabs/tailwindcss/pull/14105))
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
### Fixed

- Warn when broad glob patterns are used in the content configuration ([#14140](https://github.com/tailwindlabs/tailwindcss/pull/14140))

## [3.4.7] - 2024-07-25

Expand Down
214 changes: 211 additions & 3 deletions integrations/content-resolution/tests/content.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ let { writeConfigs, destroyConfigs } = require('./config.js')
let $ = require('../../execute')
let { css } = require('../../syntax')

let { readOutputFile } = require('../../io')({
let { writeInputFile, readOutputFile } = require('../../io')({
output: 'dist',
input: '.',
})
Expand Down Expand Up @@ -37,14 +37,22 @@ async function build({ cwd: cwdPath } = {}) {

await cwd.switch(cwdPath)

// Hide console.log and console.error output
let consoleLogMock = jest.spyOn(console, 'log').mockImplementation(() => {})
let consoleErrorMock = jest.spyOn(console, 'error').mockImplementation(() => {})

// Note that ./tailwind.config.js is hardcoded on purpose here
// It represents a config but one that could be in different places
await $(`postcss ${inputPath} -o ${outputPath}`, {
env: { NODE_ENV: 'production' },
let result = await $(`postcss ${inputPath} -o ${outputPath}`, {
env: { NODE_ENV: 'production', JEST_WORKER_ID: undefined },
cwd: cwdPath,
})

consoleLogMock.mockRestore()
consoleErrorMock.mockRestore()

return {
...result,
css: await readOutputFile('main.css'),
}
}
Expand Down Expand Up @@ -160,6 +168,206 @@ it('it handles ignored globs correctly when not relative to the config', async (
expect(result.css).toMatchCss(``)
})

it('warns when globs are too broad and match node_modules', async () => {
await writeConfigs({
both: {
content: {
files: ['./**/*.html'],
},
},
})

let result = await build({ cwd: path.resolve(__dirname, '..') })

// No issues yet, because we don't have a file that resolves inside `node_modules`
expect(result.stderr).toEqual('')

// We didn't scan any node_modules files yet
expect(result.css).not.toIncludeCss(
css`
.content-\[\'node\\_modules\/bad\.html\'\] {
--tw-content: 'node_modules/bad.html';
content: var(--tw-content);
}
`
)

// Write a file that resolves inside `node_modules`
await writeInputFile(
'node_modules/bad.html',
String.raw`<div class="content-['node\_modules/bad.html']">Bad</div>`
)

result = await build({ cwd: path.resolve(__dirname, '..') })

// We still expect the node_modules file to be processed
expect(result.css).toIncludeCss(
css`
.content-\[\'node\\_modules\/bad\.html\'\] {
--tw-content: 'node_modules/bad.html';
content: var(--tw-content);
}
`
)

// We didn't list `node_modules` in the glob explicitly, so we should see a
// warning.
expect(result.stderr).toMatchInlineSnapshot(`
"
warn - You are using a glob pattern that includes \`node_modules\` without explicitly specifying \`node_modules\` in the glob.
warn - This can lead to performance issues and is not recommended.
warn - Please consider using a more specific pattern.
warn - https://tailwindcss.com/docs/content-configuration#pattern-recommendations
"
`)
})

it('should not warn when glob contains node_modules explicitly', async () => {
await writeConfigs({
both: {
content: {
files: ['./node_modules/**/*.html'],
},
},
})

let result = await build({ cwd: path.resolve(__dirname, '..') })

// Write a file that resolves inside `node_modules`
await writeInputFile(
'node_modules/bad.html',
String.raw`<div class="content-['node\_modules/bad.html']">Bad</div>`
)

result = await build({ cwd: path.resolve(__dirname, '..') })

// We still expect the node_modules file to be processed
expect(result.css).toIncludeCss(
css`
.content-\[\'node\\_modules\/bad\.html\'\] {
--tw-content: 'node_modules/bad.html';
content: var(--tw-content);
}
`
)

// We explicitly listed `node_modules` in the glob, so we shouldn't see a
// warning.
expect(result.stderr).toEqual('')
})

it('should not warn when globs are too broad if other glob match node_modules explicitly', async () => {
await writeConfigs({
both: {
content: {
files: ['./**/*.html', './node_modules/bad.html'],
},
},
})

let result = await build({ cwd: path.resolve(__dirname, '..') })

// No issues yet, because we don't have a file that resolves inside `node_modules`
expect(result.stderr).toEqual('')

// We didn't scan any node_modules files yet
expect(result.css).not.toIncludeCss(
css`
.content-\[\'node\\_modules\/bad\.html\'\] {
--tw-content: 'node_modules/bad.html';
content: var(--tw-content);
}
`
)

// Write a file that resolves inside `node_modules`
await writeInputFile(
'node_modules/bad.html',
String.raw`<div class="content-['node\_modules/bad.html']">Bad</div>`
)

result = await build({ cwd: path.resolve(__dirname, '..') })

// We still expect the node_modules file to be processed
expect(result.css).toIncludeCss(
css`
.content-\[\'node\\_modules\/bad\.html\'\] {
--tw-content: 'node_modules/bad.html';
content: var(--tw-content);
}
`
)

// We explicitly listed `node_modules` in the glob, so we shouldn't see a
// warning.
expect(result.stderr).toEqual('')

// Write a file that resolves inside `node_modules` but is not covered by the
// explicit glob patterns.
await writeInputFile(
'node_modules/very-very-bad.html',
String.raw`<div class="content-['node\_modules/very-very-bad.html']">Bad</div>`
)

result = await build({ cwd: path.resolve(__dirname, '..') })

// We still expect the node_modules file to be processed
expect(result.css).toIncludeCss(
css`
.content-\[\'node\\_modules\/very-very-bad\.html\'\] {
--tw-content: 'node_modules/very-very-bad.html';
content: var(--tw-content);
}
`
)

// The very-very-bad.html file is not covered by the explicit glob patterns,
// so we should see a warning.
expect(result.stderr).toMatchInlineSnapshot(`
"
warn - You are using a glob pattern that includes \`node_modules\` without explicitly specifying \`node_modules\` in the glob.
warn - This can lead to performance issues and is not recommended.
warn - Please consider using a more specific pattern.
warn - https://tailwindcss.com/docs/content-configuration#pattern-recommendations
"
`)
})

it('should not warn when a negative glob is used', async () => {
await writeConfigs({
both: {
content: {
files: ['./**/*.html', '!./node_modules/**/*.html'],
},
},
})

// Write a file that resolves inside `node_modules`
await writeInputFile(
'node_modules/bad.html',
String.raw`<div class="content-['node\_modules/bad.html']">Bad</div>`
)

let result = await build({ cwd: path.resolve(__dirname, '..') })

// The initial glob resolving shouldn't use the node_modules file
// in the first place.

// We still expect the node_modules file to be processed
expect(result.css).not.toIncludeCss(
css`
.content-\[\'node\\_modules\/bad\.html\'\] {
--tw-content: 'node_modules/bad.html';
content: var(--tw-content);
}
`
)

// The node_modules file shouldn't have been processed at all because it was
// ignored by the negative glob.
expect(result.stderr).toEqual('')
})

it('it handles ignored globs correctly when relative to the config', async () => {
await writeConfigs({
both: {
Expand Down
6 changes: 3 additions & 3 deletions integrations/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ let resolveToolRoot = require('./resolve-tool-root')

let SHOW_OUTPUT = false

let runningProcessess = []
let runningProcesses = []

afterEach(() => {
runningProcessess.splice(0).forEach((runningProcess) => runningProcess.stop())
runningProcesses.splice(0).forEach((runningProcess) => runningProcess.stop())
})

function debounce(fn, ms) {
Expand Down Expand Up @@ -129,7 +129,7 @@ module.exports = function $(command, options = {}) {
})
})

runningProcessess.push(runningProcess)
runningProcesses.push(runningProcess)

return Object.assign(runningProcess, {
stop() {
Expand Down
3 changes: 2 additions & 1 deletion integrations/io.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ module.exports = function ({
}
}

return fs.writeFile(path.resolve(absoluteInputFolder, file), contents, 'utf8')
await fs.mkdir(path.dirname(filePath), { recursive: true })
return fs.writeFile(filePath, contents, 'utf8')
},
async waitForOutputFileCreation(file) {
if (file instanceof RegExp) {
Expand Down
43 changes: 42 additions & 1 deletion src/cli/build/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import path from 'path'
import fs from 'fs'
import postcssrc from 'postcss-load-config'
import { lilconfig } from 'lilconfig'
import micromatch from 'micromatch'
import loadPlugins from 'postcss-load-config/src/plugins' // Little bit scary, looking at private/internal API
import loadOptions from 'postcss-load-config/src/options' // Little bit scary, looking at private/internal API

Expand All @@ -20,6 +21,17 @@ import log from '../../util/log'
import { loadConfig } from '../../lib/load-config'
import getModuleDependencies from '../../lib/getModuleDependencies'

const LARGE_DIRECTORIES = [
'node_modules', // Node
'vendor', // PHP
]

// Ensures that `node_modules` has to match as-is, otherwise `mynode_modules`
// would match as well, but that is not a known large directory.
const LARGE_DIRECTORIES_REGEX = new RegExp(
`(${LARGE_DIRECTORIES.map((dir) => String.raw`\b${dir}\b`).join('|')})`
)

/**
*
* @param {string} [customPostCssPath ]
Expand Down Expand Up @@ -184,7 +196,36 @@ let state = {
// TODO: When we make the postcss plugin async-capable this can become async
let files = fastGlob.sync(this.contentPatterns.all)

// Detect whether a glob pattern might be too broad. This means that it:
// - Includes `**`
// - Does not include any of the known large directories (e.g.: node_modules)
let maybeBroadPattern = this.contentPatterns.all.some(
(path) => path.includes('**') && !LARGE_DIRECTORIES_REGEX.test(path)
)

// All globs that explicitly contain any of the known large directories (e.g.:
// node_modules)
let explicitGlobs = this.contentPatterns.all.filter((path) =>
LARGE_DIRECTORIES_REGEX.test(path)
)

for (let file of files) {
if (
maybeBroadPattern &&
// When a broad pattern is used, we have to double check that the file was
// not explicitly included in the globs.
!micromatch.isMatch(file, explicitGlobs)
) {
let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory))
if (largeDirectory) {
log.warn('broad-content-glob-pattern', [
`You are using a glob pattern that includes \`${largeDirectory}\` without explicitly specifying \`${largeDirectory}\` in the glob.`,
'This can lead to performance issues and is not recommended.',
'Please consider using a more specific pattern.',
'https://tailwindcss.com/docs/content-configuration#pattern-recommendations',
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
])
}
}
content.push({
content: fs.readFileSync(path.resolve(file), 'utf8'),
extension: path.extname(file).slice(1),
Expand Down Expand Up @@ -318,7 +359,7 @@ export async function createProcessor(args, cliConfigPath) {
return fs.promises.readFile(path.resolve(input), 'utf8')
}

// No input file provided, fallback to default atrules
// No input file provided, fallback to default at-rules
return '@tailwind base; @tailwind components; @tailwind utilities'
}

Expand Down
Loading
Loading