Skip to content

Commit

Permalink
Remove layer(utilities) if imports contain @utility (#14738)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
RobinMalfait authored Oct 21, 2024
1 parent 19de557 commit d2865c3
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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

Expand Down
56 changes: 55 additions & 1 deletion integrations/upgrade/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,60 @@ 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';
@import 'tailwindcss/components';
`,
'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',
{
Expand Down Expand Up @@ -737,7 +791,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';
Expand Down
41 changes: 41 additions & 0 deletions packages/@tailwindcss-upgrade/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,47 @@ async function run() {
error(`${e}`)
}

// 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
// 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! })
Expand Down
11 changes: 6 additions & 5 deletions packages/@tailwindcss-upgrade/src/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,20 +159,20 @@ 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

sheet.canMigrate = false

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)}`)
}
}
Expand All @@ -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<Stylesheet>()

for (let sheet of stylesheets) {
Expand Down Expand Up @@ -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,
},
Expand Down
16 changes: 8 additions & 8 deletions packages/@tailwindcss-upgrade/src/stylesheet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]() {
Expand Down

0 comments on commit d2865c3

Please sign in to comment.