Skip to content

Commit

Permalink
Fix crash during upgrade when content globs escape root of project (#…
Browse files Browse the repository at this point in the history
…14896)

This PR fixes an issue where globs in you `content` configuration escape
the current "root" of the project.

This can happen if you have a folder, and you need to look up in the
tree (e.g.: when looking at another package in a monorepo, or in case of
a Laravel project where you want to look at mail templates).

This applies a similar strategy we already implement on the Rust side.

1. Expand braces in the globs
2. Move static parts of the `pattern` to the `base` of the glob entry
object

---

Given a project setup like this:
```
.
├── admin
│   ├── my-tailwind.config.ts
│   └── src
│       ├── abc.jpg
│       ├── index.html
│       ├── index.js
│       └── styles
│           └── input.css
├── dashboard
│   ├── src
│   │   ├── index.html
│   │   ├── index.js
│   │   ├── input.css
│   │   └── pickaday.css
│   └── tailwind.config.ts
├── package-lock.json
├── package.json
├── postcss.config.js
└── unrelated
    └── index.html

7 directories, 14 files
```


If you then have this config:
```ts
// admin/my-tailwind.config.ts
export default {
  content: {
    relative: true,
    files: ['./src/**/*.html', '../dashboard/src/**/*.html'],
                            //  ^^  this is the important part, which escapes
                            //      the current root of the project.
  },
  theme: {
    extend: {
      colors: {
        primary: 'red',
      },
    },
  },
}
```


Then before this change, running the command looks like this:
<img width="1760" alt="image"
src="https://github.com/user-attachments/assets/60e2dfc7-3751-4432-80e3-8b4b8f1083d4">


After this change, running the command looks like this:
<img width="1452" alt="image"
src="https://github.com/user-attachments/assets/5c47182c-119c-4732-a253-2dace7086049">

---------

Co-authored-by: Philipp Spiess <[email protected]>
  • Loading branch information
RobinMalfait and philipp-spiess authored Nov 7, 2024
1 parent 462308d commit 75eeed8
Show file tree
Hide file tree
Showing 7 changed files with 239 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Ensure adjacent rules are merged together after handling nesting when generating optimized CSS ([#14873](https://github.com/tailwindlabs/tailwindcss/pull/14873))
- _Upgrade (experimental)_: Install `@tailwindcss/postcss` next to `tailwindcss` ([#14830](https://github.com/tailwindlabs/tailwindcss/pull/14830))
- _Upgrade (experimental)_: Remove whitespace around `,` separator when print arbitrary values ([#14838](https://github.com/tailwindlabs/tailwindcss/pull/14838))
- _Upgrade (experimental)_: Fix crash during upgrade when content globs escape root of project ([#14896](https://github.com/tailwindlabs/tailwindcss/pull/14896))

### Changed

Expand Down
93 changes: 93 additions & 0 deletions integrations/upgrade/js-config.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import path from 'node:path'
import { describe, expect } from 'vitest'
import { css, html, json, test, ts } from '../utils'

Expand Down Expand Up @@ -938,6 +939,98 @@ test(
},
)

test(
'migrate sources when pointing to folders outside the project root',
{
fs: {
'package.json': json`
{
"dependencies": {
"@tailwindcss/upgrade": "workspace:^"
}
}
`,

'frontend/tailwind.config.ts': ts`
export default {
content: {
relative: true,
files: ['./src/**/*.html', '../backend/mails/**/*.blade.php'],
},
theme: {
extend: {
colors: {
primary: 'red',
},
},
},
}
`,
'frontend/src/input.css': css`
@tailwind base;
@tailwind components;
@tailwind utilities;
@config "../tailwind.config.ts";
`,
'frontend/src/index.html': html`<div class="!text-primary"></div>`,

'backend/mails/welcome.blade.php': html`<div class="!text-primary"></div>`,
},
},
async ({ root, exec, fs }) => {
await exec('npx @tailwindcss/upgrade', {
cwd: path.join(root, 'frontend'),
})

expect(await fs.dumpFiles('frontend/**/*.css')).toMatchInlineSnapshot(`
"
--- frontend/src/input.css ---
@import 'tailwindcss';
@source '../../backend/mails/**/*.blade.php';
@theme {
--color-primary: red;
}
/*
The default border color has changed to \`currentColor\` in Tailwind CSS v4,
so we've added these compatibility styles to make sure everything still
looks the same as it did with Tailwind CSS v3.
If we ever want to remove these styles, we need to add an explicit border
color utility to any element that depends on these defaults.
*/
@layer base {
*,
::after,
::before,
::backdrop,
::file-selector-button {
border-color: var(--color-gray-200, currentColor);
}
}
/*
Form elements have a 1px border by default in Tailwind CSS v4, so we've
added these compatibility styles to make sure everything still looks the
same as it did with Tailwind CSS v3.
If we ever want to remove these styles, we need to add \`border-0\` to
any form elements that shouldn't have a border.
*/
@layer base {
input:where(:not([type='button'], [type='reset'], [type='submit'])),
select,
textarea {
border-width: 0;
}
}
"
`)
},
)

describe('border compatibility', () => {
test(
'migrate border compatibility',
Expand Down
2 changes: 2 additions & 0 deletions packages/@tailwindcss-upgrade/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"dependencies": {
"@tailwindcss/node": "workspace:^",
"@tailwindcss/oxide": "workspace:^",
"braces": "^3.0.3",
"dedent": "1.5.3",
"enhanced-resolve": "^5.17.1",
"globby": "^14.0.2",
Expand All @@ -44,6 +45,7 @@
"tree-sitter-typescript": "^0.23.0"
},
"devDependencies": {
"@types/braces": "^3.0.4",
"@types/node": "catalog:",
"@types/postcss-import": "^14.0.3"
}
Expand Down
7 changes: 4 additions & 3 deletions packages/@tailwindcss-upgrade/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { migrate as migrateTemplate } from './template/migrate'
import { prepareConfig } from './template/prepare-config'
import { args, type Arg } from './utils/args'
import { isRepoDirty } from './utils/git'
import { hoistStaticGlobParts } from './utils/hoist-static-glob-parts'
import { pkg } from './utils/packages'
import { eprintln, error, header, highlight, info, success } from './utils/renderer'

Expand Down Expand Up @@ -143,11 +144,11 @@ async function run() {
info('Migrating templates using the provided configuration file.')
for (let config of configBySheet.values()) {
let set = new Set<string>()
for (let { pattern, base } of config.globs) {
let files = await globby([pattern], {
for (let globEntry of config.globs.flatMap((entry) => hoistStaticGlobParts(entry))) {
let files = await globby([globEntry.pattern], {
absolute: true,
gitignore: true,
cwd: base,
cwd: globEntry.base,
})

for (let file of files) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { expect, it } from 'vitest'
import { hoistStaticGlobParts } from './hoist-static-glob-parts'

it.each([
// A basic glob
[
{ base: '/projects/project-a', pattern: './src/**/*.html' },
[{ base: '/projects/project-a/src', pattern: '**/*.html' }],
],

// A glob pointing to a folder should result in `**/*`
[
{ base: '/projects/project-a', pattern: './src' },
[{ base: '/projects/project-a/src', pattern: '**/*' }],
],

// A glob pointing to a file, should result in the file as the pattern
[
{ base: '/projects/project-a', pattern: './src/index.html' },
[{ base: '/projects/project-a/src', pattern: 'index.html' }],
],

// A glob going up a directory, should result in the new directory as the base
[
{ base: '/projects/project-a', pattern: '../project-b/src/**/*.html' },
[{ base: '/projects/project-b/src', pattern: '**/*.html' }],
],

// A glob with curlies, should be expanded to multiple globs
[
{ base: '/projects/project-a', pattern: '../project-{b,c}/src/**/*.html' },
[
{ base: '/projects/project-b/src', pattern: '**/*.html' },
{ base: '/projects/project-c/src', pattern: '**/*.html' },
],
],
[
{ base: '/projects/project-a', pattern: '../project-{b,c}/src/**/*.{js,html}' },
[
{ base: '/projects/project-b/src', pattern: '**/*.js' },
{ base: '/projects/project-b/src', pattern: '**/*.html' },
{ base: '/projects/project-c/src', pattern: '**/*.js' },
{ base: '/projects/project-c/src', pattern: '**/*.html' },
],
],
])('should hoist the static parts of the glob: %s', (input, output) => {
expect(hoistStaticGlobParts(input)).toEqual(output)

Check failure on line 47 in packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.test.ts

View workflow job for this annotation

GitHub Actions / tests (20, windows-latest, true)

src/utils/hoist-static-glob-parts.test.ts > should hoist the static parts of the glob: { base: '/projects/project-a', pattern: './src/**/*.html' }

AssertionError: expected [ { …(2) } ] to deeply equal [ { …(2) } ] - Expected + Received Array [ Object { - "base": "/projects/project-a/src", + "base": "D:\\projects\\project-a\\src", "pattern": "**/*.html", }, ] ❯ src/utils/hoist-static-glob-parts.test.ts:47:39

Check failure on line 47 in packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.test.ts

View workflow job for this annotation

GitHub Actions / tests (20, windows-latest, true)

src/utils/hoist-static-glob-parts.test.ts > should hoist the static parts of the glob: { base: '/projects/project-a', pattern: './src' }

AssertionError: expected [ { …(2) } ] to deeply equal [ { …(2) } ] - Expected + Received Array [ Object { - "base": "/projects/project-a/src", + "base": "D:\\projects\\project-a\\src", "pattern": "**/*", }, ] ❯ src/utils/hoist-static-glob-parts.test.ts:47:39

Check failure on line 47 in packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.test.ts

View workflow job for this annotation

GitHub Actions / tests (20, windows-latest, true)

src/utils/hoist-static-glob-parts.test.ts > should hoist the static parts of the glob: { base: '/projects/project-a', pattern: './src/index.html' }

AssertionError: expected [ { …(2) } ] to deeply equal [ { …(2) } ] - Expected + Received Array [ Object { - "base": "/projects/project-a/src", + "base": "D:\\projects\\project-a\\src", "pattern": "index.html", }, ] ❯ src/utils/hoist-static-glob-parts.test.ts:47:39

Check failure on line 47 in packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.test.ts

View workflow job for this annotation

GitHub Actions / tests (20, windows-latest, true)

src/utils/hoist-static-glob-parts.test.ts > should hoist the static parts of the glob: { base: '/projects/project-a', pattern: '../project-b/src/**/*.html' }

AssertionError: expected [ { …(2) } ] to deeply equal [ { …(2) } ] - Expected + Received Array [ Object { - "base": "/projects/project-b/src", + "base": "D:\\projects\\project-b\\src", "pattern": "**/*.html", }, ] ❯ src/utils/hoist-static-glob-parts.test.ts:47:39

Check failure on line 47 in packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.test.ts

View workflow job for this annotation

GitHub Actions / tests (20, windows-latest, true)

src/utils/hoist-static-glob-parts.test.ts > should hoist the static parts of the glob: { base: '/projects/project-a', pattern: '../project-{b,c}/src/**/*.html' }

AssertionError: expected [ { …(2) }, { …(2) } ] to deeply equal [ { …(2) }, { …(2) } ] - Expected + Received Array [ Object { - "base": "/projects/project-b/src", + "base": "D:\\projects\\project-b\\src", "pattern": "**/*.html", }, Object { - "base": "/projects/project-c/src", + "base": "D:\\projects\\project-c\\src", "pattern": "**/*.html", }, ] ❯ src/utils/hoist-static-glob-parts.test.ts:47:39

Check failure on line 47 in packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.test.ts

View workflow job for this annotation

GitHub Actions / tests (20, windows-latest, true)

src/utils/hoist-static-glob-parts.test.ts > should hoist the static parts of the glob: { base: '/projects/project-a', pattern: '../project-{b,c}/src/**/*.{js,html}' }

AssertionError: expected [ { …(2) }, { …(2) }, { …(2) }, …(1) ] to deeply equal [ { …(2) }, { …(2) }, { …(2) }, …(1) ] - Expected + Received Array [ Object { - "base": "/projects/project-b/src", + "base": "D:\\projects\\project-b\\src", "pattern": "**/*.js", }, Object { - "base": "/projects/project-b/src", + "base": "D:\\projects\\project-b\\src", "pattern": "**/*.html", }, Object { - "base": "/projects/project-c/src", + "base": "D:\\projects\\project-c\\src", "pattern": "**/*.js", }, Object { - "base": "/projects/project-c/src", + "base": "D:\\projects\\project-c\\src", "pattern": "**/*.html", }, ] ❯ src/utils/hoist-static-glob-parts.test.ts:47:39
})
79 changes: 79 additions & 0 deletions packages/@tailwindcss-upgrade/src/utils/hoist-static-glob-parts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
import braces from 'braces'
import path from 'node:path'

interface GlobEntry {
base: string
pattern: string
}

export function hoistStaticGlobParts(entry: GlobEntry): GlobEntry[] {
return braces(entry.pattern, { expand: true }).map((pattern) => {
let clone = { ...entry }
let [staticPart, dynamicPart] = splitPattern(pattern)

// Move static part into the `base`.
if (staticPart !== null) {
clone.base = path.resolve(entry.base, staticPart)
} else {
clone.base = path.resolve(entry.base)
}

// Move dynamic part into the `pattern`.
if (dynamicPart === null) {
clone.pattern = '**/*'
} else {
clone.pattern = dynamicPart
}

// If the pattern looks like a file, move the file name from the `base` to
// the `pattern`.
let file = path.basename(clone.base)
if (file.includes('.')) {
clone.pattern = file
clone.base = path.dirname(clone.base)
}

return clone
})
}

// Split a glob pattern into a `static` and `dynamic` part.
//
// Assumption: we assume that all globs are expanded, which means that the only
// dynamic parts are using `*`.
//
// E.g.:
// Original input: `../project-b/**/*.{html,js}`
// Expanded input: `../project-b/**/*.html` & `../project-b/**/*.js`
// Split on first input: ("../project-b", "**/*.html")
// Split on second input: ("../project-b", "**/*.js")
function splitPattern(pattern: string): [staticPart: string | null, dynamicPart: string | null] {
// No dynamic parts, so we can just return the input as-is.
if (!pattern.includes('*')) {
return [pattern, null]
}

let lastSlashPosition: number | null = null

for (let i = 0; i < pattern.length; i++) {
let c = pattern[i];
if (c === '/') {
lastSlashPosition = i
}

if (c === '*' || c === '!') {
break
}
}

// Very first character is a `*`, therefore there is no static part, only a
// dynamic part.
if (lastSlashPosition === null) {
return [null, pattern]
}

let staticPart = pattern.slice(0, lastSlashPosition).trim()
let dynamicPart = pattern.slice(lastSlashPosition + 1).trim()

return [staticPart || null, dynamicPart || null]
}
13 changes: 12 additions & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 75eeed8

Please sign in to comment.