From 59723fb0d6f2d7be6204b3e911a35e9435ee7d95 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Fri, 2 Aug 2024 12:08:20 +0200 Subject: [PATCH 01/12] show a warning when detecting broad glob patterns 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. --- .../content-resolution/tests/content.test.js | 214 +++++++++++++++++- integrations/execute.js | 6 +- integrations/io.js | 3 +- src/cli/build/plugin.js | 43 +++- src/lib/content.js | 40 ++++ 5 files changed, 298 insertions(+), 8 deletions(-) diff --git a/integrations/content-resolution/tests/content.test.js b/integrations/content-resolution/tests/content.test.js index a4db09f1b0d3..3fd16864b754 100644 --- a/integrations/content-resolution/tests/content.test.js +++ b/integrations/content-resolution/tests/content.test.js @@ -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: '.', }) @@ -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'), } } @@ -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`
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(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`
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(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`
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..b4d3b2871153 100644 --- a/src/cli/build/plugin.js +++ b/src/cli/build/plugin.js @@ -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 @@ -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 ] @@ -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', + ]) + } + } content.push({ content: fs.readFileSync(path.resolve(file), 'utf8'), extension: path.extname(file).slice(1), @@ -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' } diff --git a/src/lib/content.js b/src/lib/content.js index e814efe4d34e..4194e624d2c1 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,17 @@ 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 {ContentPath[]} candidateFiles @@ -191,10 +204,37 @@ function resolveChangedFiles(candidateFiles, fileModifiedMap) { let paths = candidateFiles.map((contentPath) => contentPath.pattern) let mTimesToCommit = new Map() + // 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) + ) + + // 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)) + let changedFiles = new Set() env.DEBUG && console.time('Finding changed files') let files = fastGlob.sync(paths, { absolute: true }) 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', + ]) + } + } let prevModified = fileModifiedMap.get(file) || -Infinity let modified = fs.statSync(file).mtimeMs From ef547035c01a48a5b70b24d0eddc9e81a46c6dc1 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 11:27:08 +0200 Subject: [PATCH 02/12] update changelog --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23c509331d5c..12f7e9a40294 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) +### 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 From 89a55e3acce82f6a912336468148be85298f90d3 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 11:48:41 +0200 Subject: [PATCH 03/12] strip all ANSI escape codes --- integrations/content-resolution/tests/content.test.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/integrations/content-resolution/tests/content.test.js b/integrations/content-resolution/tests/content.test.js index 3fd16864b754..7774ac0ebd63 100644 --- a/integrations/content-resolution/tests/content.test.js +++ b/integrations/content-resolution/tests/content.test.js @@ -1,5 +1,6 @@ let fs = require('fs') let path = require('path') +let { stripVTControlCharacters } = require('util') let { cwd } = require('./cwd.js') let { writeConfigs, destroyConfigs } = require('./config.js') @@ -212,7 +213,7 @@ it('warns when globs are too broad and match node_modules', async () => { // We didn't list `node_modules` in the glob explicitly, so we should see a // warning. - expect(result.stderr).toMatchInlineSnapshot(` + expect(stripVTControlCharacters(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. @@ -323,7 +324,7 @@ it('should not warn when globs are too broad if other glob match node_modules ex // The very-very-bad.html file is not covered by the explicit glob patterns, // so we should see a warning. - expect(result.stderr).toMatchInlineSnapshot(` + expect(stripVTControlCharacters(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. From 09b69c621994f313b50b6f1664679638d0dd2036 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 11:51:10 +0200 Subject: [PATCH 04/12] fix CHANGELOG Accidentally removed a line --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f7e9a40294..0e4fc04e625c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### 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 From e5eeb5222cbcaf0bee8e4abba1bc84c703c92359 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 12:09:50 +0200 Subject: [PATCH 05/12] re-use broad glob check --- src/cli/build/plugin.js | 45 +++------------------------------ src/lib/content.js | 56 +++++++++++++++++++++++++++-------------- 2 files changed, 41 insertions(+), 60 deletions(-) diff --git a/src/cli/build/plugin.js b/src/cli/build/plugin.js index b4d3b2871153..1cb4ef5a9784 100644 --- a/src/cli/build/plugin.js +++ b/src/cli/build/plugin.js @@ -4,7 +4,6 @@ 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 @@ -13,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' @@ -21,17 +20,6 @@ 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 ] @@ -196,36 +184,11 @@ 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) - ) + let checkBroadPattern = createBroadPatternCheck(this.contentPatterns.all) 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', - ]) - } - } + checkBroadPattern(file) + content.push({ content: fs.readFileSync(path.resolve(file), 'utf8'), extension: path.extname(file).slice(1), diff --git a/src/lib/content.js b/src/lib/content.js index 4194e624d2c1..7d6b06b7edcb 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -195,15 +195,9 @@ const LARGE_DIRECTORIES_REGEX = new RegExp( ) /** - * - * @param {ContentPath[]} candidateFiles - * @param {Map} fileModifiedMap - * @returns {[Set, Map]} + * @param {string[]} paths */ -function resolveChangedFiles(candidateFiles, fileModifiedMap) { - let paths = candidateFiles.map((contentPath) => contentPath.pattern) - let mTimesToCommit = new Map() - +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) @@ -211,20 +205,23 @@ function resolveChangedFiles(candidateFiles, fileModifiedMap) { (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) + // node_modules). let explicitGlobs = paths.filter((path) => LARGE_DIRECTORIES_REGEX.test(path)) - let changedFiles = new Set() - env.DEBUG && console.time('Finding changed files') - let files = fastGlob.sync(paths, { absolute: true }) - 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) - ) { + /** + * @param {string} file + */ + return (file) => { + // When a broad pattern is used, we have to double check that the file was + // not explicitly included in the globs. + if (!micromatch.isMatch(file, explicitGlobs)) { let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory)) if (largeDirectory) { log.warn('broad-content-glob-pattern', [ @@ -235,6 +232,27 @@ function resolveChangedFiles(candidateFiles, fileModifiedMap) { ]) } } + } +} + +/** + * + * @param {ContentPath[]} candidateFiles + * @param {Map} fileModifiedMap + * @returns {[Set, Map]} + */ +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 From 28d911e620cf8533884dc04884bd06a9969527b1 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 12:20:10 +0200 Subject: [PATCH 06/12] perf: prevent checking all files, if we already warned --- src/lib/content.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/lib/content.js b/src/lib/content.js index 7d6b06b7edcb..e0e45812c38f 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -215,15 +215,25 @@ export function createBroadPatternCheck(paths) { // 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 + // When a broad pattern is used, we have to double check that the file was // not explicitly included in the globs. if (!micromatch.isMatch(file, explicitGlobs)) { let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory)) if (largeDirectory) { + warned = true + 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.', From 8be24836124d9cfef9fae6e83d97dd4b7add6962 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 12:42:40 +0200 Subject: [PATCH 07/12] re-word warning + use the actual glob/file that is causing issues --- src/lib/content.js | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/lib/content.js b/src/lib/content.js index e0e45812c38f..8f98fc96f4f1 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -230,14 +230,26 @@ export function createBroadPatternCheck(paths) { // When a broad pattern is used, we have to double check that the file was // not explicitly included in the globs. if (!micromatch.isMatch(file, explicitGlobs)) { + 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 relativeFile = path.relative(process.cwd(), file) + if (relativeFile[0] !== '.') relativeFile = `./${relativeFile}` + let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory)) if (largeDirectory) { warned = true log.warn('broad-content-glob-pattern', [ - `You are using a glob pattern that includes \`${largeDirectory}\` without explicitly specifying \`${largeDirectory}\` in the glob.`, + `Your \`content\` configuration uses 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.', + `Glob: \`${relativeMatchingGlob}\``, + `File: \`${relativeFile}\``, + `Please consider using a more specific pattern or use \`${largeDirectory}\` explicitly.`, 'https://tailwindcss.com/docs/content-configuration#pattern-recommendations', ]) } From 2d7e4aa8520f2d78ae61fc970c2d615de37c4917 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 12:44:09 +0200 Subject: [PATCH 08/12] update snapshot tests --- .../content-resolution/tests/content.test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/integrations/content-resolution/tests/content.test.js b/integrations/content-resolution/tests/content.test.js index 7774ac0ebd63..33a78dc2c0d8 100644 --- a/integrations/content-resolution/tests/content.test.js +++ b/integrations/content-resolution/tests/content.test.js @@ -215,9 +215,11 @@ it('warns when globs are too broad and match node_modules', async () => { // warning. expect(stripVTControlCharacters(result.stderr)).toMatchInlineSnapshot(` " - warn - You are using a glob pattern that includes \`node_modules\` without explicitly specifying \`node_modules\` in the glob. + 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 - Please consider using a more specific pattern. + 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 " `) @@ -326,9 +328,11 @@ it('should not warn when globs are too broad if other glob match node_modules ex // so we should see a warning. expect(stripVTControlCharacters(result.stderr)).toMatchInlineSnapshot(` " - warn - You are using a glob pattern that includes \`node_modules\` without explicitly specifying \`node_modules\` in the glob. + 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 - Please consider using a more specific pattern. + 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 " `) From b1865b9d54fe03acb43957cb9f4c5fadd3fa6bd4 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 12:45:44 +0200 Subject: [PATCH 09/12] use early return to dedent code --- src/lib/content.js | 49 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/src/lib/content.js b/src/lib/content.js index 8f98fc96f4f1..0fd948c53f03 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -225,34 +225,33 @@ export function createBroadPatternCheck(paths) { * @param {string} file */ return (file) => { - if (warned) return + 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. - if (!micromatch.isMatch(file, explicitGlobs)) { - 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 relativeFile = path.relative(process.cwd(), file) - if (relativeFile[0] !== '.') relativeFile = `./${relativeFile}` - - let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory)) - if (largeDirectory) { - warned = true - - log.warn('broad-content-glob-pattern', [ - `Your \`content\` configuration uses a glob pattern that includes \`${largeDirectory}\` without explicitly specifying \`${largeDirectory}\` in the glob.`, - 'This can lead to performance issues and is not recommended.', - `Glob: \`${relativeMatchingGlob}\``, - `File: \`${relativeFile}\``, - `Please consider using a more specific pattern or use \`${largeDirectory}\` explicitly.`, - 'https://tailwindcss.com/docs/content-configuration#pattern-recommendations', - ]) - } + 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 relativeFile = path.relative(process.cwd(), file) + if (relativeFile[0] !== '.') relativeFile = `./${relativeFile}` + + let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory)) + if (largeDirectory) { + warned = true + + log.warn('broad-content-glob-pattern', [ + `Your \`content\` configuration uses a glob pattern that includes \`${largeDirectory}\` without explicitly specifying \`${largeDirectory}\` in the glob.`, + 'This can lead to performance issues and is not recommended.', + `Glob: \`${relativeMatchingGlob}\``, + `File: \`${relativeFile}\``, + `Please consider using a more specific pattern or use \`${largeDirectory}\` explicitly.`, + 'https://tailwindcss.com/docs/content-configuration#pattern-recommendations', + ]) } } } From f5c8b516c083364b64fa8b39369002a85d91ef19 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 16:50:29 +0200 Subject: [PATCH 10/12] Update src/lib/content.js Co-authored-by: Adam Wathan --- src/lib/content.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/lib/content.js b/src/lib/content.js index 0fd948c53f03..c71eed09f40e 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -245,11 +245,9 @@ export function createBroadPatternCheck(paths) { warned = true log.warn('broad-content-glob-pattern', [ - `Your \`content\` configuration uses a glob pattern that includes \`${largeDirectory}\` without explicitly specifying \`${largeDirectory}\` in the glob.`, - 'This can lead to performance issues and is not recommended.', - `Glob: \`${relativeMatchingGlob}\``, - `File: \`${relativeFile}\``, - `Please consider using a more specific pattern or use \`${largeDirectory}\` explicitly.`, + `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', ]) } From 9c0006fc111aaf7fe69f9a5f8e5171ac3a1ae61d Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 16:51:14 +0200 Subject: [PATCH 11/12] remove unused `relativeFile` --- src/lib/content.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/lib/content.js b/src/lib/content.js index c71eed09f40e..abb0e5840f13 100644 --- a/src/lib/content.js +++ b/src/lib/content.js @@ -237,9 +237,6 @@ export function createBroadPatternCheck(paths) { let relativeMatchingGlob = path.relative(process.cwd(), matchingGlob) if (relativeMatchingGlob[0] !== '.') relativeMatchingGlob = `./${relativeMatchingGlob}` - let relativeFile = path.relative(process.cwd(), file) - if (relativeFile[0] !== '.') relativeFile = `./${relativeFile}` - let largeDirectory = LARGE_DIRECTORIES.find((directory) => file.includes(directory)) if (largeDirectory) { warned = true From a9202fb59b498acc53953639f9736327f3578981 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 7 Aug 2024 16:52:51 +0200 Subject: [PATCH 12/12] update snapshot tests --- .../content-resolution/tests/content.test.js | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/integrations/content-resolution/tests/content.test.js b/integrations/content-resolution/tests/content.test.js index 33a78dc2c0d8..7dc135635d7f 100644 --- a/integrations/content-resolution/tests/content.test.js +++ b/integrations/content-resolution/tests/content.test.js @@ -215,11 +215,9 @@ it('warns when globs are too broad and match node_modules', async () => { // 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 - 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 " `) @@ -328,11 +326,9 @@ it('should not warn when globs are too broad if other glob match node_modules ex // 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 - 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 " `)