From f19afd91e50e3f9fc71156327ece8a5869132b17 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Wed, 20 Nov 2024 00:20:40 +0100 Subject: [PATCH] Improve root file detection (#15048) This PR fixes an issue where the Tailwind root file detection was wrong. Whenever a CSS file contains any of the `@tailwind` directives or an `@import` to any of the Tailwind files, the file is considered a Tailwind root file. If multiple CSS files are part of the same tree, then we make the nearest common parent the root file. This root file will be the file where we add `@config` and/or inject other changes during the migration process. However, if your folder structure looked like this: ```css /* index.css */ @import "./base.css"; @import "./typography.css"; @import "tailwindcss/components"; /* This makes index.css a root file */ @import "./utilities.css"; /* base.css */ @tailwind base; /* This makes base.css a root file */ /* utilities.css */ @tailwind utilities; /* This makes utilities.css a root file */ ``` Then we computed that `index.css` nad `base.css` were considered root files even though they belong to the same tree (because `base.css` is imported by `index.css`). This PR fixes that behaviour by essentially being less smart, and just checking again if any sheets are part of the same tree. # Test plan: Added an integration test that covers this scenario and fails before the fix. Also ran it on our tailwindcss.com codebase. | Before | After | | --- | --- | | image | image | (Yes, I know the migration still fails, but that's a different issue.) --- CHANGELOG.md | 1 + integrations/upgrade/index.test.ts | 101 +++++++++++++++++++ packages/@tailwindcss-upgrade/src/migrate.ts | 9 +- 3 files changed, 106 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35eb85fd2..9c1cca3bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - _Upgrade (experimental)_: Ignore analyzing imports with external URLs (e.g.: `@import "https://fonts.google.com"`) ([#15040](https://github.com/tailwindlabs/tailwindcss/pull/15040)) - _Upgrade (experimental)_: Ignore analyzing imports with `url(…)` (e.g.: `@import url("https://fonts.google.com")`) ([#15040](https://github.com/tailwindlabs/tailwindcss/pull/15040)) - _Upgrade (experimental)_: Use `resolveJsId` when resolving `tailwindcss/package.json` ([#15041](https://github.com/tailwindlabs/tailwindcss/pull/15041)) +- _Upgrade (experimental)_: Ensure children of Tailwind root file are not considered Tailwind root files ([#15048](https://github.com/tailwindlabs/tailwindcss/pull/15048)) ### Changed diff --git a/integrations/upgrade/index.test.ts b/integrations/upgrade/index.test.ts index 1e39992b9..f4dbb2ad2 100644 --- a/integrations/upgrade/index.test.ts +++ b/integrations/upgrade/index.test.ts @@ -1970,6 +1970,107 @@ test( }, ) +test( + 'finds the correct root Tailwind CSS file', + { + fs: { + 'package.json': json` + { + "dependencies": { + "tailwindcss": "^3", + "@tailwindcss/upgrade": "workspace:^" + } + } + `, + 'tailwind.config.ts': js`export default {}`, + + /* Considered a Tailwind root because of: `@import 'tailwindcss/components'` */ + 'src/index.css': css` + @import 'base.css'; + @import 'other.css'; + @import 'tailwindcss/components'; + @import 'utilities.css'; + `, + + /* Considered a Tailwind root because of: `@tailwind base` */ + 'src/base.css': css` + html { + color: red; + } + @tailwind base; + `, + 'src/other.css': css` + .typography { + color: red; + } + `, + + /* Considered a Tailwind root because of: `@tailwind utilities` */ + 'src/utilities.css': css` + @layer utilities { + .foo { + color: red; + } + } + + @tailwind utilities; + `, + }, + }, + async ({ exec, fs }) => { + await exec('npx @tailwindcss/upgrade --force') + + expect(await fs.dumpFiles('./src/**/*.{html,css}')).toMatchInlineSnapshot(` + " + --- ./src/index.css --- + @import './base.css' layer(components); + @import './other.css' layer(components); + @import './utilities.css'; + + --- ./src/base.css --- + @import 'tailwindcss/theme' layer(theme); + @import 'tailwindcss/preflight' layer(base); + + /* + 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); + } + } + + @layer base { + html { + color: red; + } + } + + --- ./src/other.css --- + .typography { + color: red; + } + + --- ./src/utilities.css --- + @import 'tailwindcss/utilities' layer(utilities); + + @utility foo { + color: red; + } + " + `) + }, +) + test( 'relative imports without a relative path prefix are migrated to include a relative path prefix', { diff --git a/packages/@tailwindcss-upgrade/src/migrate.ts b/packages/@tailwindcss-upgrade/src/migrate.ts index 08aad4c46..1e6af335a 100644 --- a/packages/@tailwindcss-upgrade/src/migrate.ts +++ b/packages/@tailwindcss-upgrade/src/migrate.ts @@ -282,10 +282,10 @@ export async function analyze(stylesheets: Stylesheet[]) { // parent A and parent B will be moved to the parent of parent A and // parent B. Parent A and parent B will be removed. let repeat = true - while (repeat) { + repeat: while (repeat) { repeat = false - outer: for (let [sheetA, childrenA] of commonParents) { + for (let [sheetA, childrenA] of commonParents) { for (let [sheetB, childrenB] of commonParents) { if (sheetA === sheetB) continue @@ -316,12 +316,11 @@ export async function analyze(stylesheets: Stylesheet[]) { commonParents.get(parent).add(child) } - repeat = parent !== sheetA && parent !== sheetB - // Found a common parent between sheet A and sheet B. We can // stop looking for more common parents between A and B, and // continue with the next sheet. - break outer + repeat = true + continue repeat } } }