Warn when broad glob patterns are used in the content configuration (#14140)
When you use a glob pattern in your `content` configuration that is too broad, it could be that you are accidentally including files that you didn't intend to include. E.g.: all of `node_modules` This has been documented in the [Tailwind CSS documentation](https://tailwindcss.com/docs/content-configuration#pattern-recommendations), but it's still something that a lot of people run into. This PR will try to detect those patterns and show a big warning to let you know if you may have done something wrong. We will show a warning if all of these conditions are true: 1. We detect `**` in the glob pattern 2. _and_ you didn't explicitly use `node_modules` in the glob pattern 3. _and_ we found files that include `node_modules` in the file path 4. _and_ no other globs exist that explicitly match the found file With these rules in place, the DX has nice trade-offs: 1. Very simple projects (that don't even have a `node_modules` folder), can simply use `./**/*` because while resolving actual files we won't see files from `node_modules` and thus won't warn. 2. If you use `./src/**` and you do have a `node_modules`, then we also won't complain (unless you have a `node_modules` folder in the `./src` folder). 3. If you work with a 3rd party library that you want to make changes to. Using an explicit match like `./node_modules/my-package/**/*` is allowed because `node_modules` is explicitly mentioned. Note: this only shows a warning, it does not stop the process entirely. The warning will be show when the very first file in the `node_modules` is detected. <!-- 👋 Hey, thanks for your interest in contributing to Tailwind! **Please ask first before starting work on any significant new features.** It's never a fun experience to have your pull request declined after investing a lot of time and effort into a new feature. To avoid this from happening, we request that contributors create an issue to first discuss any significant new features. This includes things like adding new utilities, creating new at-rules, or adding new component examples to the documentation. https://github.com/tailwindcss/tailwindcss/blob/master/.github/CONTRIBUTING.md --> --------- Co-authored-by: Adam Wathan <adam.wathan@gmail.com>
This commit is contained in:
Robin Malfait
GitHub
parent
1f23c2e842
commit
858696a8bc
6 changed files with 300 additions and 9 deletions
|
|
@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fix minification when using nested CSS ([#14105](https://github.com/tailwindlabs/tailwindcss/pull/14105))
|
||||
- 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
|
||||
|
||||
|
|
|
|||
|
|
@ -1,12 +1,13 @@
|
|||
let fs = require('fs')
|
||||
let path = require('path')
|
||||
let { stripVTControlCharacters } = require('util')
|
||||
let { cwd } = require('./cwd.js')
|
||||
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: '.',
|
||||
})
|
||||
|
|
@ -37,14 +38,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'),
|
||||
}
|
||||
}
|
||||
|
|
@ -160,6 +169,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(stripVTControlCharacters(result.stderr)).toMatchInlineSnapshot(`
|
||||
"
|
||||
warn - Your \`content\` configuration includes a pattern which looks like it's accidentally matching all of \`node_modules\` and can cause serious performance issues.
|
||||
warn - Pattern: \`./**/*.html\`
|
||||
warn - See our documentation for recommendations:
|
||||
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(stripVTControlCharacters(result.stderr)).toMatchInlineSnapshot(`
|
||||
"
|
||||
warn - Your \`content\` configuration includes a pattern which looks like it's accidentally matching all of \`node_modules\` and can cause serious performance issues.
|
||||
warn - Pattern: \`./**/*.html\`
|
||||
warn - See our documentation for recommendations:
|
||||
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: {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
@ -129,7 +129,7 @@ module.exports = function $(command, options = {}) {
|
|||
})
|
||||
})
|
||||
|
||||
runningProcessess.push(runningProcess)
|
||||
runningProcesses.push(runningProcess)
|
||||
|
||||
return Object.assign(runningProcess, {
|
||||
stop() {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import { loadAutoprefixer, loadCssNano, loadPostcss, loadPostcssImport } from '.
|
|||
import { formatNodes, drainStdin, outputFile } from './utils'
|
||||
import { env } from '../../lib/sharedState'
|
||||
import resolveConfig from '../../../resolveConfig.js'
|
||||
import { parseCandidateFiles } from '../../lib/content.js'
|
||||
import { createBroadPatternCheck, parseCandidateFiles } from '../../lib/content.js'
|
||||
import { createWatcher } from './watching.js'
|
||||
import fastGlob from 'fast-glob'
|
||||
import { findAtConfigPath } from '../../lib/findAtConfigPath.js'
|
||||
|
|
@ -184,7 +184,11 @@ let state = {
|
|||
// TODO: When we make the postcss plugin async-capable this can become async
|
||||
let files = fastGlob.sync(this.contentPatterns.all)
|
||||
|
||||
let checkBroadPattern = createBroadPatternCheck(this.contentPatterns.all)
|
||||
|
||||
for (let file of files) {
|
||||
checkBroadPattern(file)
|
||||
|
||||
content.push({
|
||||
content: fs.readFileSync(path.resolve(file), 'utf8'),
|
||||
extension: path.extname(file).slice(1),
|
||||
|
|
@ -318,7 +322,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'
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,8 @@ import fastGlob from 'fast-glob'
|
|||
import normalizePath from 'normalize-path'
|
||||
import { parseGlob } from '../util/parseGlob'
|
||||
import { env } from './sharedState'
|
||||
import log from '../util/log'
|
||||
import micromatch from 'micromatch'
|
||||
|
||||
/** @typedef {import('../../types/config.js').RawFile} RawFile */
|
||||
/** @typedef {import('../../types/config.js').FilePath} FilePath */
|
||||
|
|
@ -181,6 +183,74 @@ export function resolvedChangedContent(context, candidateFiles, fileModifiedMap)
|
|||
return [changedContent, mTimesToCommit]
|
||||
}
|
||||
|
||||
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[]} paths
|
||||
*/
|
||||
export function createBroadPatternCheck(paths) {
|
||||
// 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 = paths.some(
|
||||
(path) => path.includes('**') && !LARGE_DIRECTORIES_REGEX.test(path)
|
||||
)
|
||||
|
||||
// Didn't detect any potentially broad patterns, so we can skip further
|
||||
// checks.
|
||||
if (!maybeBroadPattern) {
|
||||
return () => {}
|
||||
}
|
||||
|
||||
// All globs that explicitly contain any of the known large directories (e.g.:
|
||||
// node_modules).
|
||||
let explicitGlobs = paths.filter((path) => LARGE_DIRECTORIES_REGEX.test(path))
|
||||
|
||||
// Keep track of whether we already warned about the broad pattern issue or
|
||||
// not. The `log.warn` function already does something similar where we only
|
||||
// output the log once. However, with this we can also skip the other checks
|
||||
// when we already warned about the broad pattern.
|
||||
let warned = false
|
||||
|
||||
/**
|
||||
* @param {string} file
|
||||
*/
|
||||
return (file) => {
|
||||
if (warned) return // Already warned about the broad pattern
|
||||
if (micromatch.isMatch(file, explicitGlobs)) return // Explicitly included, so we can skip further checks
|
||||
|
||||
// When a broad pattern is used, we have to double check that the file was
|
||||
// not explicitly included in the globs.
|
||||
let matchingGlob = paths.find((path) => micromatch.isMatch(file, path))
|
||||
if (!matchingGlob) return // This should never happen
|
||||
|
||||
// Create relative paths to make the output a bit more readable.
|
||||
let relativeMatchingGlob = path.relative(process.cwd(), matchingGlob)
|
||||
if (relativeMatchingGlob[0] !== '.') relativeMatchingGlob = `./${relativeMatchingGlob}`
|
||||
|
||||
let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory))
|
||||
if (largeDirectory) {
|
||||
warned = true
|
||||
|
||||
log.warn('broad-content-glob-pattern', [
|
||||
`Your \`content\` configuration includes a pattern which looks like it's accidentally matching all of \`${largeDirectory}\` and can cause serious performance issues.`,
|
||||
`Pattern: \`${relativeMatchingGlob}\``,
|
||||
`See our documentation for recommendations:`,
|
||||
'https://tailwindcss.com/docs/content-configuration#pattern-recommendations',
|
||||
])
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
*
|
||||
* @param {ContentPath[]} candidateFiles
|
||||
|
|
@ -191,10 +261,14 @@ function resolveChangedFiles(candidateFiles, fileModifiedMap) {
|
|||
let paths = candidateFiles.map((contentPath) => contentPath.pattern)
|
||||
let mTimesToCommit = new Map()
|
||||
|
||||
let checkBroadPattern = createBroadPatternCheck(paths)
|
||||
|
||||
let changedFiles = new Set()
|
||||
env.DEBUG && console.time('Finding changed files')
|
||||
let files = fastGlob.sync(paths, { absolute: true })
|
||||
for (let file of files) {
|
||||
checkBroadPattern(file)
|
||||
|
||||
let prevModified = fileModifiedMap.get(file) || -Infinity
|
||||
let modified = fs.statSync(file).mtimeMs
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue