From 4f8ca556cf172f721b9ed54f8d59e2f91b48476c Mon Sep 17 00:00:00 2001 From: Robin Malfait Date: Mon, 30 Sep 2024 15:32:30 +0200 Subject: [PATCH] CSS codemod: inject `@import` in a more expected location (#14536) This PR inserts the `@import` in a more sensible location when running codemods. The idea is that we replace `@tailwind base; @tailwind components; @tailwind utilities;` with the much simple `@import "tailwindcss";`. We did this by adding the `@import` to the top of the file. While this is correct, this means that the diff might not be as clear. For example, if you have a situation where you have a license comment: ```css /**! My license comment */ @tailwind base; @tailwind components; @tailwind utilities; ``` This resulted in: ```css @import "tailwindcss"; /**! My license comment */ ``` While it is not wrong, it feels weird that this behaves like this. In this commit we make sure that it is injected in-place (the first `@tailwind` at-rule we find) and fixup the position if we can't inject it in-place. The above example results in this: ```css /**! My license comment */ @import "tailwindcss"; ``` However, there are scenario's where you can't replace the `@tailwind` directives directly. E.g.: ```css /**! My license comment */ html { color: red; } @tailwind base; @tailwind components; @tailwind utilities; ``` If we replace the `@tailwind` directives in-place, it would look like this: ```css /**! My license comment */ html { color: red; } @import "tailwindcss"; ``` But this is invalid CSS, because you can't have CSS above an `@import` at-rule. There are some exceptions like: - `@charset` - `@import` - `@layer foo, bar;` (just the order, without a body) - comments In this scenario, we inject the import in the nearest place where it is allowed to. In this case: ```css /**! My license comment */ @import "tailwindcss"; @layer base { html { color: red; } } ``` Additionally, we will wrap the existing CSS in an `@layer` of the first Tailwind directive we saw. In this case an `@layer base`. This ensures that utilities still win from the default styles. Also note that the (license) comment is allowed to exist before the `@import`, therefore we do not put the `@import` above it. This also means that the diff doesn't touch the license header at all, which makes the diffs cleaner and easier to reason about. --------- Co-authored-by: Philipp Spiess --- CHANGELOG.md | 5 +- .../codemods/migrate-missing-layers.test.ts | 64 +++++++++++ .../src/codemods/migrate-missing-layers.ts | 21 +++- .../migrate-tailwind-directives.test.ts | 64 +++++++++++ .../codemods/migrate-tailwind-directives.ts | 101 +++++++++++++++--- .../@tailwindcss-upgrade/src/index.test.ts | 26 +++++ 6 files changed, 260 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ed57c400..6ae894e4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,16 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add support for prefixes ([#14501](https://github.com/tailwindlabs/tailwindcss/pull/14501)) - _Experimental_: Add template codemods for migrating `bg-gradient-*` utilities to `bg-linear-*` ([#14537](https://github.com/tailwindlabs/tailwindcss/pull/14537])) +- _Experimental_: Migrate `@import "tailwindcss/tailwind.css"` to `@import "tailwindcss"` ([#14514](https://github.com/tailwindlabs/tailwindcss/pull/14514)) ### Fixed - Use the right import base path when using the CLI to reading files from stdin ([#14522](https://github.com/tailwindlabs/tailwindcss/pull/14522)) +- Ensure that `@utility` is top-level and cannot be nested ([#14525](https://github.com/tailwindlabs/tailwindcss/pull/14525)) - _Experimental_: Improve codemod output, keep CSS after last Tailwind directive unlayered ([#14512](https://github.com/tailwindlabs/tailwindcss/pull/14512)) - _Experimental_: Fix incorrect empty `layer()` at the end of `@import` at-rules when running codemods ([#14513](https://github.com/tailwindlabs/tailwindcss/pull/14513)) -- _Experimental_: Migrate `@import "tailwindcss/tailwind.css"` to `@import "tailwindcss"` ([#14514](https://github.com/tailwindlabs/tailwindcss/pull/14514)) - _Experimental_: Do not wrap comment nodes in `@layer` when running codemods ([#14517](https://github.com/tailwindlabs/tailwindcss/pull/14517)) - _Experimental_: Ensure we don't lose selectors when running codemods ([#14518](https://github.com/tailwindlabs/tailwindcss/pull/14518)) -- Ensure that `@utility` is top-level and cannot be nested ([#14525](https://github.com/tailwindlabs/tailwindcss/pull/14525)) +- _Experimental_: inject `@import` in a more expected location when running codemods ([#14536](https://github.com/tailwindlabs/tailwindcss/pull/14536)) ## [4.0.0-alpha.25] - 2024-09-24 diff --git a/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.test.ts b/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.test.ts index a35a50479..055a25269 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.test.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.test.ts @@ -22,6 +22,32 @@ it('should not migrate already migrated `@import` at-rules', async () => { ).toMatchInlineSnapshot(`"@import 'tailwindcss';"`) }) +it('should not migrate anything if no `@tailwind` directives (or imports) are found', async () => { + expect( + await migrate(css` + /* Base */ + html { + color: red; + } + + /* Utilities */ + .foo { + color: blue; + } + `), + ).toMatchInlineSnapshot(` + "/* Base */ + html { + color: red; + } + + /* Utilities */ + .foo { + color: blue; + }" + `) +}) + it('should not wrap comments in a layer, if they are the only nodes', async () => { expect( await migrate(css` @@ -54,6 +80,44 @@ it('should not wrap comments in a layer, if they are the only nodes', async () = `) }) +it('should migrate rules above the `@tailwind base` directive in an `@layer base`', async () => { + expect( + await migrate(css` + @charset "UTF-8"; + @layer foo, bar, baz; + + /**! + * License header + */ + + html { + color: red; + } + + @tailwind base; + @tailwind components; + @tailwind utilities; + `), + ).toMatchInlineSnapshot(` + "@charset "UTF-8"; + @layer foo, bar, baz; + + /**! + * License header + */ + + @layer base { + html { + color: red; + } + } + + @tailwind base; + @tailwind components; + @tailwind utilities;" + `) +}) + it('should migrate rules between tailwind directives', async () => { expect( await migrate(css` diff --git a/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.ts b/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.ts index e63568ee3..4ccfe9b96 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/migrate-missing-layers.ts @@ -5,6 +5,7 @@ export function migrateMissingLayers(): Plugin { let lastLayer = '' let bucket: ChildNode[] = [] let buckets: [layer: string, bucket: typeof bucket][] = [] + let firstLayerName: string | null = null root.each((node) => { if (node.type === 'atrule') { @@ -25,6 +26,7 @@ export function migrateMissingLayers(): Plugin { buckets.push([lastLayer, bucket.splice(0)]) } + firstLayerName ??= 'base' lastLayer = 'base' return } @@ -38,6 +40,7 @@ export function migrateMissingLayers(): Plugin { buckets.push([lastLayer, bucket.splice(0)]) } + firstLayerName ??= 'components' lastLayer = 'components' return } @@ -51,6 +54,7 @@ export function migrateMissingLayers(): Plugin { buckets.push([lastLayer, bucket.splice(0)]) } + firstLayerName ??= 'utilities' lastLayer = 'utilities' return } @@ -76,10 +80,19 @@ export function migrateMissingLayers(): Plugin { } } - // Track the node - if (lastLayer !== '') { - bucket.push(node) + // (License) comments, body-less `@layer` and `@charset` can stay at the + // top, when we haven't found any `@tailwind` at-rules yet. + if ( + lastLayer === '' && + (node.type === 'comment' /* Comment */ || + (node.type === 'atrule' && !node.nodes) || // @layer foo, bar, baz; + (node.type === 'atrule' && node.name === 'charset')) // @charset "UTF-8"; + ) { + return } + + // Track the node + bucket.push(node) }) // Wrap each bucket in an `@layer` at-rule @@ -92,7 +105,7 @@ export function migrateMissingLayers(): Plugin { let target = nodes[0] let layerNode = new AtRule({ name: 'layer', - params: layerName, + params: layerName || firstLayerName || '', nodes: nodes.map((node) => { // Keep the target node as-is, because we will be replacing that one // with the new layer node. diff --git a/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.test.ts b/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.test.ts index 1f4a00b52..1ca17740f 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.test.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.test.ts @@ -58,6 +58,70 @@ it('should migrate the default @tailwind directives as imports to a single impor `) }) +it('should migrate the default @tailwind directives to a single import in a valid location', async () => { + expect( + await migrate(css` + @charset "UTF-8"; + @layer foo, bar, baz; + + /**! + * License header + */ + + html { + color: red; + } + + @tailwind base; + @tailwind components; + @tailwind utilities; + `), + ) + // NOTE: The `html {}` is not wrapped in a `@layer` directive, because that + // is handled by another migration step. See ../index.test.ts for a + // dedicated test. + .toEqual(css` + @charset "UTF-8"; + @layer foo, bar, baz; + + /**! + * License header + */ + + @import 'tailwindcss'; + + html { + color: red; + } + `) +}) + +it('should migrate the default @tailwind directives as imports to a single import in a valid location', async () => { + expect( + await migrate(css` + @charset "UTF-8"; + @layer foo, bar, baz; + + /**! + * License header + */ + + @import 'tailwindcss/base'; + @import 'tailwindcss/components'; + @import 'tailwindcss/utilities'; + `), + ).toEqual(css` + @charset "UTF-8"; + @layer foo, bar, baz; + + /**! + * License header + */ + + @import 'tailwindcss'; + `) +}) + it.each([ [ // The default order diff --git a/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.ts b/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.ts index a3007b77a..58c066758 100644 --- a/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.ts +++ b/packages/@tailwindcss-upgrade/src/codemods/migrate-tailwind-directives.ts @@ -1,16 +1,17 @@ -import { AtRule, type Plugin, type Root } from 'postcss' +import { AtRule, type ChildNode, type Plugin, type Root } from 'postcss' const DEFAULT_LAYER_ORDER = ['theme', 'base', 'components', 'utilities'] export function migrateTailwindDirectives(): Plugin { function migrate(root: Root) { - let baseNode: AtRule | null = null - let utilitiesNode: AtRule | null = null + let baseNode = null as AtRule | null + let utilitiesNode = null as AtRule | null + let orderedNodes: AtRule[] = [] - let defaultImportNode: AtRule | null = null - let utilitiesImportNode: AtRule | null = null - let preflightImportNode: AtRule | null = null - let themeImportNode: AtRule | null = null + let defaultImportNode = null as AtRule | null + let utilitiesImportNode = null as AtRule | null + let preflightImportNode = null as AtRule | null + let themeImportNode = null as AtRule | null let layerOrder: string[] = [] @@ -26,15 +27,15 @@ export function migrateTailwindDirectives(): Plugin { (node.name === 'import' && node.params.match(/^["']tailwindcss\/base["']$/)) ) { layerOrder.push('base') + orderedNodes.push(node) baseNode = node - node.remove() } else if ( (node.name === 'tailwind' && node.params === 'utilities') || (node.name === 'import' && node.params.match(/^["']tailwindcss\/utilities["']$/)) ) { layerOrder.push('utilities') + orderedNodes.push(node) utilitiesNode = node - node.remove() } // Remove directives that are not needed anymore @@ -51,24 +52,34 @@ export function migrateTailwindDirectives(): Plugin { // Insert default import if all directives are present if (baseNode !== null && utilitiesNode !== null) { if (!defaultImportNode) { - root.prepend(new AtRule({ name: 'import', params: "'tailwindcss'" })) + findTargetNode(orderedNodes).before(new AtRule({ name: 'import', params: "'tailwindcss'" })) } + baseNode?.remove() + utilitiesNode?.remove() } // Insert individual imports if not all directives are present else if (utilitiesNode !== null) { if (!utilitiesImportNode) { - root.prepend( + findTargetNode(orderedNodes).before( new AtRule({ name: 'import', params: "'tailwindcss/utilities' layer(utilities)" }), ) } + utilitiesNode?.remove() } else if (baseNode !== null) { - if (!preflightImportNode) { - root.prepend(new AtRule({ name: 'import', params: "'tailwindcss/preflight' layer(base)" })) - } if (!themeImportNode) { - root.prepend(new AtRule({ name: 'import', params: "'tailwindcss/theme' layer(theme)" })) + findTargetNode(orderedNodes).before( + new AtRule({ name: 'import', params: "'tailwindcss/theme' layer(theme)" }), + ) } + + if (!preflightImportNode) { + findTargetNode(orderedNodes).before( + new AtRule({ name: 'import', params: "'tailwindcss/preflight' layer(base)" }), + ) + } + + baseNode?.remove() } // Insert `@layer …;` at the top when the order in the CSS was different @@ -94,3 +105,63 @@ export function migrateTailwindDirectives(): Plugin { OnceExit: migrate, } } + +// Finds the location where we can inject the new `@import` at-rule. This +// guarantees that the `@import` is inserted at the most expected location. +// +// Ideally it's replacing the existing Tailwind directives, but we have to +// ensure that the `@import` is valid in this location or not. If not, we move +// the `@import` up until we find a valid location. +function findTargetNode(nodes: AtRule[]) { + // Start at the `base` or `utilities` node (whichever comes first), and find + // the spot where we can insert the new import. + let target: ChildNode = nodes.at(0)! + + // Only allowed nodes before the `@import` are: + // + // - `@charset` at-rule. + // - `@layer foo, bar, baz;` at-rule to define the order of the layers. + // - `@import` at-rule to import other CSS files. + // - Comments. + // + // Nodes that cannot exist before the `@import` are: + // + // - Any other at-rule. + // - Any rule. + let previous = target.prev() + while (previous) { + // Rules are not allowed before the `@import`, so we have to at least inject + // the `@import` before this rule. + if (previous.type === 'rule') { + target = previous + } + + // Some at-rules are allowed before the `@import`. + if (previous.type === 'atrule') { + // `@charset` and `@import` are allowed before the `@import`. + if (previous.name === 'charset' || previous.name === 'import') { + // Allowed + previous = previous.prev() + continue + } + + // `@layer` without any nodes is allowed before the `@import`. + else if (previous.name === 'layer' && !previous.nodes) { + // Allowed + previous = previous.prev() + continue + } + + // Anything other at-rule (`@media`, `@supports`, etc.) is not allowed + // before the `@import`. + else { + target = previous + } + } + + // Keep checking the previous node. + previous = previous.prev() + } + + return target +} diff --git a/packages/@tailwindcss-upgrade/src/index.test.ts b/packages/@tailwindcss-upgrade/src/index.test.ts index 2f4f7475e..329e03288 100644 --- a/packages/@tailwindcss-upgrade/src/index.test.ts +++ b/packages/@tailwindcss-upgrade/src/index.test.ts @@ -118,3 +118,29 @@ it('should migrate a stylesheet (with imports)', async () => { @import './my-utilities.css' layer(utilities);" `) }) + +it('should migrate a stylesheet (with preceding rules that should be wrapped in an `@layer`)', async () => { + expect( + await migrateContents(css` + @charset "UTF-8"; + @layer foo, bar, baz; + /**! My license comment */ + html { + color: red; + } + @tailwind base; + @tailwind components; + @tailwind utilities; + `), + ).toMatchInlineSnapshot(` + "@charset "UTF-8"; + @layer foo, bar, baz; + /**! My license comment */ + @import 'tailwindcss'; + @layer base { + html { + color: red; + } + }" + `) +})