Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- _Upgrade (experimental)_: Ensure it's safe to migrate `blur`, `rounded`, or `shadow` ([#14979](https://github.com/tailwindlabs/tailwindcss/pull/14979))
- _Upgrade (experimental)_: Do not rename classes using custom defined theme values ([#14976](https://github.com/tailwindlabs/tailwindcss/pull/14976))
- _Upgrade (experimental)_: Ensure `@config` is injected in nearest common ancestor stylesheet ([#14989](https://github.com/tailwindlabs/tailwindcss/pull/14989))
- _Upgrade (experimental)_: Add missing `layer(…)` to imports above Tailwind directives ([#14982](https://github.com/tailwindlabs/tailwindcss/pull/14982))

## [4.0.0-alpha.33] - 2024-11-11

Expand Down
130 changes: 128 additions & 2 deletions integrations/upgrade/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,132 @@ test(
},
)

test(
'migrate imports with `layer(…)`',
{
fs: {
'package.json': json`
{
"dependencies": {
"tailwindcss": "workspace:^",
"@tailwindcss/upgrade": "workspace:^"
}
}
`,
'tailwind.config.js': js`module.exports = {}`,
'src/index.css': css`
@import './base.css';
@import './components.css';
@import './utilities.css';
@import './mix.css';

@tailwind base;
@tailwind components;
@tailwind utilities;
`,
'src/base.css': css`
html {
color: red;
}
`,
'src/components.css': css`
@layer components {
.foo {
color: red;
}
}
`,
'src/utilities.css': css`
@layer utilities {
.bar {
color: red;
}
}
`,
'src/mix.css': css`
html {
color: blue;
}

@layer components {
.foo-mix {
color: red;
}
}

@layer utilities {
.bar-mix {
color: red;
}
}
`,
},
},
async ({ fs, exec }) => {
await exec('npx @tailwindcss/upgrade')

expect(await fs.dumpFiles('./src/**/*.css')).toMatchInlineSnapshot(`
"
--- ./src/index.css ---
@import './base.css' layer(base);
@import './components.css';
@import './utilities.css';
@import './mix.css' layer(base);
@import './mix.utilities.css';

@import 'tailwindcss';

/*
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);
}
}

--- ./src/base.css ---
html {
color: red;
}

--- ./src/components.css ---
@utility foo {
color: red;
}

--- ./src/mix.css ---
html {
color: blue;
}

--- ./src/mix.utilities.css ---
@utility foo-mix {
color: red;
}

@utility bar-mix {
color: red;
}

--- ./src/utilities.css ---
@utility bar {
color: red;
}
"
`)
},
)

test(
'migrates a simple postcss setup',
{
Expand Down Expand Up @@ -1571,7 +1697,7 @@ test(
}

--- ./src/components.css ---
@import './typography.css';
@import './typography.css' layer(components);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has layer(components) because this is coming from the components file where nothing sits above it (in that file), but there is a @layer components and @tailwind components below it.

if we want to look at as-if it was flattened, then it exists between base and components. This means that if this code existed in the same file we would've generated layer(base) instead. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also what we get on the Commit template with this PR:
image

Without knowing the rest of the files, it looks correct. But I wonder if layer(base) is more correct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked with @philipp-spiess and @thecrypticace and we came to the conclusion that this is the least surprising outcome where you look at CSS files in isolation.


@utility foo {
color: red;
Expand Down Expand Up @@ -1706,7 +1832,7 @@ test(
}

--- ./src/components.css ---
@import './typography.css';
@import './typography.css' layer(components);

@utility foo {
color: red;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ export function migrateMissingLayers(): Plugin {

// Add layer to `@import` at-rules
if (node.name === 'import') {
if (lastLayer !== '' && !node.params.includes('layer(')) {
node.params += ` layer(${lastLayer})`
node.raws.tailwind_injected_layer = true
}

if (bucket.length > 0) {
buckets.push([lastLayer, bucket.splice(0)])
}

// Create new bucket just for the import. This way every import exists
// in its own layer which allows us to add the `layer(…)` parameter
// later on.
buckets.push([lastLayer, [node]])
return
}
}
Expand All @@ -102,7 +102,6 @@ export function migrateMissingLayers(): Plugin {
bucket.push(node)
})

// Wrap each bucket in an `@layer` at-rule
for (let [layerName, nodes] of buckets) {
let targetLayerName = layerName || firstLayerName || ''
if (targetLayerName === '') {
Expand All @@ -114,6 +113,20 @@ export function migrateMissingLayers(): Plugin {
continue
}

// Add `layer(…)` to `@import` at-rules
if (nodes.every((node) => node.type === 'atrule' && node.name === 'import')) {
for (let node of nodes) {
if (node.type !== 'atrule' || node.name !== 'import') continue

if (!node.params.includes('layer(')) {
node.params += ` layer(${targetLayerName})`
node.raws.tailwind_injected_layer = true
}
}
continue
}

// Wrap each bucket in an `@layer` at-rule
let target = nodes[0]
let layerNode = new AtRule({
name: 'layer',
Expand Down
47 changes: 37 additions & 10 deletions packages/@tailwindcss-upgrade/src/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,21 +402,48 @@ export async function split(stylesheets: Stylesheet[]) {
}

// Keep track of sheets that contain `@utility` rules
let containsUtilities = new Set<Stylesheet>()
let requiresSplit = new Set<Stylesheet>()

for (let sheet of stylesheets) {
let layers = sheet.layers()
let isLayered = layers.has('utilities') || layers.has('components')
if (!isLayered) continue
// Root files don't need to be split
if (sheet.isTailwindRoot) continue

let containsUtility = false
let containsUnsafe = sheet.layers().size > 0

walk(sheet.root, (node) => {
if (node.type !== 'atrule') return
if (node.name !== 'utility') return
if (node.type === 'atrule' && node.name === 'utility') {
containsUtility = true
}

// Safe to keep without splitting
else if (
// An `@import "…" layer(…)` is safe
(node.type === 'atrule' && node.name === 'import' && node.params.includes('layer(')) ||
// @layer blocks are safe
(node.type === 'atrule' && node.name === 'layer') ||
// Comments are safe
node.type === 'comment'
) {
return WalkAction.Skip
}

containsUtilities.add(sheet)
// Everything else is not safe, and requires a split
else {
containsUnsafe = true
}

// We already know we need to split this sheet
if (containsUtility && containsUnsafe) {
return WalkAction.Stop
}

return WalkAction.Stop
return WalkAction.Skip
})

if (containsUtility && containsUnsafe) {
requiresSplit.add(sheet)
}
}

// Split every imported stylesheet into two parts
Expand All @@ -429,8 +456,8 @@ export async function split(stylesheets: Stylesheet[]) {

// Skip stylesheets that don't have utilities
// and don't have any children that have utilities
if (!containsUtilities.has(sheet)) {
if (!Array.from(sheet.descendants()).some((child) => containsUtilities.has(child))) {
if (!requiresSplit.has(sheet)) {
if (!Array.from(sheet.descendants()).some((child) => requiresSplit.has(child))) {
continue
}
}
Expand Down