diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c509331d5c..0e4fc04e625c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/integrations/content-resolution/tests/content.test.js b/integrations/content-resolution/tests/content.test.js index a4db09f1b0d3..7dc135635d7f 100644 --- a/integrations/content-resolution/tests/content.test.js +++ b/integrations/content-resolution/tests/content.test.js @@ -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`
Bad
` + ) + + 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`
Bad
` + ) + + 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`
Bad
` + ) + + 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`
Bad
` + ) + + 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`
Bad
` + ) + + 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: { diff --git a/integrations/execute.js b/integrations/execute.js index 6c5f2036327d..a1a0d539077d 100644 --- a/integrations/execute.js +++ b/integrations/execute.js @@ -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() { diff --git a/integrations/io.js b/integrations/io.js index c555ac8ba02c..348c170112e3 100644 --- a/integrations/io.js +++ b/integrations/io.js @@ -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) { diff --git a/src/cli/build/plugin.js b/src/cli/build/plugin.js index 49ffdcb0ccd3..1cb4ef5a9784 100644 --- a/src/cli/build/plugin.js +++ b/src/cli/build/plugin.js @@ -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' } diff --git a/src/lib/content.js b/src/lib/content.js index e814efe4d34e..abb0e5840f13 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -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