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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved
- 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
219 changes: 216 additions & 3 deletions integrations/content-resolution/tests/content.test.js
Original file line number Diff line number Diff line change
@@ -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: '.',
})
Expand Down Expand Up @@ -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'),
}
}
Expand Down Expand Up @@ -160,6 +169,210 @@ 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 uses 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 - Glob: \`./**/*.html\`
warn - File: \`./node_modules/bad.html\`
warn - Please consider using a more specific pattern or use \`node_modules\` explicitly.
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 uses 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 - Glob: \`./**/*.html\`
warn - File: \`./node_modules/very-very-bad.html\`
warn - Please consider using a more specific pattern or use \`node_modules\` explicitly.
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
8 changes: 6 additions & 2 deletions src/cli/build/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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)
RobinMalfait marked this conversation as resolved.
Show resolved Hide resolved

for (let file of files) {
checkBroadPattern(file)

content.push({
content: fs.readFileSync(path.resolve(file), 'utf8'),
extension: path.extname(file).slice(1),
Expand Down Expand Up @@ -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'
}

Expand Down
Loading
Loading