From 36eb79d2ffd2b2551ca99d580448140c5168e0b8 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:21:27 +0200 Subject: [PATCH 1/7] remove `layer(utilities)` if imports contain `@utility` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have a migration that adds the `layer(…)` next to the `@import` depending on the order of original values. For example: ```css @import "tailwindcss/utilities": @import "./foo.css": @import "tailwindcss/components": ``` Will be turned into: ```css @import "tailwindcss": @import "./foo.css" layer(utilities): ``` Because it used to exist between `utilities` and `components`. Without this it would be _after_ `components`. This results in an issue if an import has (deeply) nested `@utility` at-rules after migrations. This is because if this is generated: ```css /* ./src/index.css */ @import "tailwindcss"; @import "./foo.css" layer(utilities); /* ./src/foo.css */ @utility foo { color: red; } ``` Once we interpret this (and thus flatten it), the final CSS would look like: ```css @layer utilities { @utility foo { color: red; } } ``` This means that `@utility` is not top-level and an error would occur. This fixes that by removing the `layer(…)` from the import if the imported file (or any of its children) contains an `@utility`. This is to ensure that once everything is imported and flattened, that all `@utility` at-rules are top-level. --- packages/@tailwindcss-upgrade/src/index.ts | 36 ++++++++++++++++++++ packages/@tailwindcss-upgrade/src/migrate.ts | 1 + 2 files changed, 37 insertions(+) diff --git a/packages/@tailwindcss-upgrade/src/index.ts b/packages/@tailwindcss-upgrade/src/index.ts index f82717413306..3d9ac3238070 100644 --- a/packages/@tailwindcss-upgrade/src/index.ts +++ b/packages/@tailwindcss-upgrade/src/index.ts @@ -149,6 +149,42 @@ async function run() { error(`${e}`) } + // Cleanup `@import "…" layer(utilities)` + for (let sheet of stylesheets) { + let hasAtUtility = false + + // Only remove the `layer(…)` next to the import, if any of the children + // contains an `@utility`. Otherwise the `@utility` will not be top-level. + { + sheet.root.walkAtRules('utility', () => { + hasAtUtility = true + return false + }) + + if (!hasAtUtility) { + for (let child of sheet.descendants()) { + child.root.walkAtRules('utility', () => { + hasAtUtility = true + return false + }) + + if (hasAtUtility) { + break + } + } + } + } + + // No `@utility` found, we can keep the `layer(…)` next to the import + if (!hasAtUtility) continue + + for (let importNode of sheet.importRules) { + if (importNode.raws.tailwind_injected_layer) { + importNode.params = importNode.params.replace(/ layer\([^)]+\)/, '').trim() + } + } + } + // Format nodes for (let sheet of stylesheets) { await postcss([formatNodes()]).process(sheet.root!, { from: sheet.file! }) diff --git a/packages/@tailwindcss-upgrade/src/migrate.ts b/packages/@tailwindcss-upgrade/src/migrate.ts index b6524d1b0451..b97af22c7945 100644 --- a/packages/@tailwindcss-upgrade/src/migrate.ts +++ b/packages/@tailwindcss-upgrade/src/migrate.ts @@ -324,6 +324,7 @@ export async function split(stylesheets: Stylesheet[]) { params: `${quote}${newFile}${quote}`, raws: { after: '\n\n', + tailwind_injected_layer: node.raws.tailwind_injected_layer, tailwind_original_params: `${quote}${id}${quote}`, tailwind_destination_sheet_id: utilityDestination.id, }, From 3daabed3fc7c0eb4b09b8cf36842c5fac6b15b31 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:25:57 +0200 Subject: [PATCH 2/7] add test --- integrations/upgrade/index.test.ts | 53 ++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/integrations/upgrade/index.test.ts b/integrations/upgrade/index.test.ts index 948c0a9e9fd6..1545a338698c 100644 --- a/integrations/upgrade/index.test.ts +++ b/integrations/upgrade/index.test.ts @@ -638,6 +638,59 @@ test( }, ) +test( + 'migrate utilities in an imported file and keep @utility top-level', + { + fs: { + 'package.json': json` + { + "dependencies": { + "tailwindcss": "workspace:^", + "@tailwindcss/upgrade": "workspace:^" + } + } + `, + 'tailwind.config.js': js`module.exports = {}`, + 'src/index.css': css` + @import 'tailwindcss/utilities'; + @import './utilities.css'; + `, + 'src/utilities.css': css` + @layer utilities { + .no-scrollbar::-webkit-scrollbar { + display: none; + } + + .no-scrollbar { + -ms-overflow-style: none; + scrollbar-width: none; + } + } + `, + }, + }, + async ({ fs, exec }) => { + await exec('npx @tailwindcss/upgrade --force') + + expect(await fs.dumpFiles('./src/**/*.css')).toMatchInlineSnapshot(` + " + --- ./src/index.css --- + @import 'tailwindcss/utilities' layer(utilities); + @import './utilities.css'; + + --- ./src/utilities.css --- + @utility no-scrollbar { + &::-webkit-scrollbar { + display: none; + } + -ms-overflow-style: none; + scrollbar-width: none; + } + " + `) + }, +) + test( 'migrate utilities in deep import trees', { From ce9a9cb2f0ea1bfcd42b95dcaaf1be63401311d4 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:26:23 +0200 Subject: [PATCH 3/7] fix existing test with new logic --- integrations/upgrade/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integrations/upgrade/index.test.ts b/integrations/upgrade/index.test.ts index 1545a338698c..f72b2fc8254b 100644 --- a/integrations/upgrade/index.test.ts +++ b/integrations/upgrade/index.test.ts @@ -790,7 +790,7 @@ test( @import './a.1.css' layer(utilities); @import './a.1.utilities.1.css'; @import './b.1.css'; - @import './c.1.css' layer(utilities); + @import './c.1.css'; @import './c.1.utilities.css'; @import './d.1.css'; From c5f0bca593f2167e7caa40b43f2f28257b5cba72 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:26:35 +0200 Subject: [PATCH 4/7] fix typos I thought that `convertable` was a valid word since you are "able to convert" something. Turns out that this is spelled `convertible`. --- packages/@tailwindcss-upgrade/src/migrate.ts | 10 +++++----- packages/@tailwindcss-upgrade/src/stylesheet.ts | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/@tailwindcss-upgrade/src/migrate.ts b/packages/@tailwindcss-upgrade/src/migrate.ts index b97af22c7945..1b72afc6f780 100644 --- a/packages/@tailwindcss-upgrade/src/migrate.ts +++ b/packages/@tailwindcss-upgrade/src/migrate.ts @@ -159,8 +159,8 @@ export async function analyze(stylesheets: Stylesheet[]) { for (let sheet of stylesheets) { if (!sheet.file) continue - let { convertablePaths, nonConvertablePaths } = sheet.analyzeImportPaths() - let isAmbiguous = convertablePaths.length > 0 && nonConvertablePaths.length > 0 + let { convertiblePaths, nonConvertiblePaths } = sheet.analyzeImportPaths() + let isAmbiguous = convertiblePaths.length > 0 && nonConvertiblePaths.length > 0 if (!isAmbiguous) continue @@ -168,11 +168,11 @@ export async function analyze(stylesheets: Stylesheet[]) { let filePath = sheet.file.replace(commonPath, '') - for (let path of convertablePaths) { + for (let path of convertiblePaths) { lines.push(`- ${filePath} <- ${pathToString(path)}`) } - for (let path of nonConvertablePaths) { + for (let path of nonConvertiblePaths) { lines.push(`- ${filePath} <- ${pathToString(path)}`) } } @@ -197,7 +197,7 @@ export async function split(stylesheets: Stylesheet[]) { } } - // Keep track of sheets that contain `@utillity` rules + // Keep track of sheets that contain `@utility` rules let containsUtilities = new Set() for (let sheet of stylesheets) { diff --git a/packages/@tailwindcss-upgrade/src/stylesheet.ts b/packages/@tailwindcss-upgrade/src/stylesheet.ts index c3a91fe4ba88..1b8fa841e7b5 100644 --- a/packages/@tailwindcss-upgrade/src/stylesheet.ts +++ b/packages/@tailwindcss-upgrade/src/stylesheet.ts @@ -197,26 +197,26 @@ export class Stylesheet { * adjusting imports which is a non-trivial task. */ analyzeImportPaths() { - let convertablePaths: StylesheetConnection[][] = [] - let nonConvertablePaths: StylesheetConnection[][] = [] + let convertiblePaths: StylesheetConnection[][] = [] + let nonConvertiblePaths: StylesheetConnection[][] = [] for (let path of this.pathsToRoot()) { - let isConvertable = false + let isConvertible = false for (let { meta } of path) { for (let layer of meta.layers) { - isConvertable ||= layer === 'utilities' || layer === 'components' + isConvertible ||= layer === 'utilities' || layer === 'components' } } - if (isConvertable) { - convertablePaths.push(path) + if (isConvertible) { + convertiblePaths.push(path) } else { - nonConvertablePaths.push(path) + nonConvertiblePaths.push(path) } } - return { convertablePaths, nonConvertablePaths } + return { convertiblePaths, nonConvertiblePaths } } [util.inspect.custom]() { From 02272ac5593bed5b445090712a3b368758e62b8f Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:32:57 +0200 Subject: [PATCH 5/7] only look at the CSS tree if an import with `layer()` exists --- packages/@tailwindcss-upgrade/src/index.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/@tailwindcss-upgrade/src/index.ts b/packages/@tailwindcss-upgrade/src/index.ts index 3d9ac3238070..17a1041c8d5b 100644 --- a/packages/@tailwindcss-upgrade/src/index.ts +++ b/packages/@tailwindcss-upgrade/src/index.ts @@ -151,6 +151,11 @@ async function run() { // Cleanup `@import "…" layer(utilities)` for (let sheet of stylesheets) { + // If the `@import` contains an injected `layer(…)` we need to remove it + if (!Array.from(sheet.importRules).some((node) => node.raws.tailwind_injected_layer)) { + continue + } + let hasAtUtility = false // Only remove the `layer(…)` next to the import, if any of the children From dcefc16bd946ae204c61a848859dc5630f47a28b Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:34:38 +0200 Subject: [PATCH 6/7] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7940c36fd811..e8a13a9ded19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - _Upgrade (experimental)_: Minify arbitrary values when printing candidates ([#14720](https://github.com/tailwindlabs/tailwindcss/pull/14720)) - _Upgrade (experimental)_: Ensure legacy theme values ending in `1` (like `theme(spacing.1)`) are correctly migrated to custom properties ([#14724](https://github.com/tailwindlabs/tailwindcss/pull/14724)) - _Upgrade (experimental)_: Migrate arbitrary values to bare values for the `from-*`, `via-*`, and `to-*` utilities ([#14725](https://github.com/tailwindlabs/tailwindcss/pull/14725)) +- _Upgrade (experimental)_: Ensure `layer(utilities)` is removed from `@import` to keep `@utility` top-level ([#14738](https://github.com/tailwindlabs/tailwindcss/pull/14738)) ### Changed From cf436c2b23355f8458f2961da521ea9b0854bbf6 Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 21 Oct 2024 15:37:32 +0200 Subject: [PATCH 7/7] squish import between existing imports --- integrations/upgrade/index.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/integrations/upgrade/index.test.ts b/integrations/upgrade/index.test.ts index f72b2fc8254b..80005c088256 100644 --- a/integrations/upgrade/index.test.ts +++ b/integrations/upgrade/index.test.ts @@ -654,6 +654,7 @@ test( 'src/index.css': css` @import 'tailwindcss/utilities'; @import './utilities.css'; + @import 'tailwindcss/components'; `, 'src/utilities.css': css` @layer utilities {