Ensure layer(…) on @import is only removed when @utility is present (#14783)

This PR fixes an issue where `layer(…)` next to imports were removed
where they shouldn't have been removed.

The issue exists if _any_ of the `@import` nodes in a file contains
`@utility`, if that's the case then we removed the `layer(…)` next to
_all_ `@import` nodes.

Before we were checking if the current sheet contained `@utility` or in
any of its children (sub-`@import` nodes).

This fixes that by looping over the `@import` nodes in the current
sheet, and looking for the `@utility` in the associated/imported file.
This way we update each node individually.

Test plan:
---

Added a dedicated integration test to make sure all codemods together
result in the correct result. Input:

96e8908378/integrations/upgrade/index.test.ts (L2076-L2108)

Output:

96e8908378/integrations/upgrade/index.test.ts (L2116-L2126)
This commit is contained in:
Robin Malfait 2024-10-24 20:33:10 +02:00 committed by GitHub
parent 148d8707b9
commit 430836f651
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 143 additions and 38 deletions

View File

@ -12,10 +12,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Added `not-*` versions of all builtin media query and supports variants ([#14743](https://github.com/tailwindlabs/tailwindcss/pull/14743))
- Improved support for custom variants with `group-*`, `peer-*`, `has-*`, and `not-*` ([#14743](https://github.com/tailwindlabs/tailwindcss/pull/14743))
### Changed
- Don't convert underscores in the first argument to `var()` and `theme()` to spaces ([#14776](https://github.com/tailwindlabs/tailwindcss/pull/14776), [#14781](https://github.com/tailwindlabs/tailwindcss/pull/14781))
### Fixed
- Ensure individual logical property utilities are sorted later than left/right pair utilities ([#14777](https://github.com/tailwindlabs/tailwindcss/pull/14777))
@ -26,6 +22,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- _Upgrade (experimental)_: Ensure `@import` statements for relative CSS files are actually migrated to use relative path syntax ([#14769](https://github.com/tailwindlabs/tailwindcss/pull/14769))
- _Upgrade (experimental)_: Only generate Preflight compatibility styles when Preflight is used ([#14773](https://github.com/tailwindlabs/tailwindcss/pull/14773))
- _Upgrade (experimental)_: Don't escape underscores when printing theme values migrated to CSS variables in arbitrary values (e.g. `m-[var(--spacing-1_5)]` instead of `m-[var(--spacing-1\_5)]`) ([#14778](https://github.com/tailwindlabs/tailwindcss/pull/14778))
- _Upgrade (experimental)_: Ensure `layer(…)` on `@import` is only removed when `@utility` is present ([#14783](https://github.com/tailwindlabs/tailwindcss/pull/14783))
### Changed
- Don't convert underscores in the first argument to `var()` and `theme()` to spaces ([#14776](https://github.com/tailwindlabs/tailwindcss/pull/14776), [#14781](https://github.com/tailwindlabs/tailwindcss/pull/14781))
## [4.0.0-alpha.29] - 2024-10-23

View File

@ -1024,7 +1024,7 @@ test(
@import './a.1.css' layer(utilities);
@import './a.1.utilities.1.css';
@import './b.1.css';
@import './c.1.css';
@import './c.1.css' layer(utilities);
@import './c.1.utilities.css';
@import './d.1.css';
@ -1545,3 +1545,87 @@ test(
`)
},
)
test(
'that it attaches the correct layers to the imported files',
{
fs: {
'package.json': json`
{
"dependencies": {
"tailwindcss": "workspace:^",
"@tailwindcss/upgrade": "workspace:^"
}
}
`,
'tailwind.config.js': js`module.exports = {}`,
'src/index.css': css`
@import 'tailwindcss/utilities';
/* No layer expected */
@import './my-components.css';
/* No layer expected */
@import './my-utilities.css';
/* Expecting a layer */
@import './my-other.css';
@import 'tailwindcss/components';
`,
'src/my-components.css': css`
@layer components {
.foo {
color: red;
}
}
`,
'src/my-utilities.css': css`
@layer utilities {
.css {
color: red;
}
}
`,
'src/my-other.css': css`
/* All my fonts! */
@font-face {
}
`,
},
},
async ({ fs, exec }) => {
await exec('npx @tailwindcss/upgrade --force')
expect(await fs.dumpFiles('./src/**/*.css')).toMatchInlineSnapshot(`
"
--- ./src/index.css ---
@import 'tailwindcss/utilities' layer(utilities);
/* No layer expected */
@import './my-components.css';
/* No layer expected */
@import './my-utilities.css';
/* Expecting a layer */
@import './my-other.css' layer(utilities);
--- ./src/my-components.css ---
@utility foo {
color: red;
}
--- ./src/my-other.css ---
/* All my fonts! */
@font-face {
}
--- ./src/my-utilities.css ---
@utility css {
color: red;
}
"
`)
},
)

View File

@ -22,6 +22,22 @@ it('should not migrate already migrated `@import` at-rules', async () => {
).toMatchInlineSnapshot(`"@import 'tailwindcss';"`)
})
it('should add missing `layer(…)` to imported files', async () => {
expect(
await migrate(css`
@import 'tailwindcss/utilities'; /* Expected no layer */
@import './foo.css'; /* Expected layer(utilities) */
@import './bar.css'; /* Expected layer(utilities) */
@import 'tailwindcss/components'; /* Expected no layer */
`),
).toMatchInlineSnapshot(`
"@import 'tailwindcss/utilities'; /* Expected no layer */
@import './foo.css' layer(utilities); /* Expected layer(utilities) */
@import './bar.css' layer(utilities); /* Expected layer(utilities) */
@import 'tailwindcss/components'; /* Expected no layer */"
`)
})
it('should not migrate anything if no `@tailwind` directives (or imports) are found', async () => {
expect(
await migrate(css`

View File

@ -151,42 +151,23 @@ 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
}
for (let importRule of sheet.importRules) {
if (!importRule.raws.tailwind_injected_layer) continue
let importedSheet = stylesheets.find(
(sheet) => sheet.id === importRule.raws.tailwind_destination_sheet_id,
)
if (!importedSheet) 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
}
}
// Only remove the `layer(…)` next to the import, if any of the children
// contains an `@utility`. Otherwise the `@utility` will not be top-level.
if (
!importedSheet.containsRule((node) => node.type === 'atrule' && node.name === 'utility')
) {
continue
}
}
// 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()
}
// Make sure to remove the `layer(…)` from the `@import` at-rule
importRule.params = importRule.params.replace(/ layer\([^)]+\)/, '').trim()
}
}

View File

@ -219,6 +219,29 @@ export class Stylesheet {
return { convertiblePaths, nonConvertiblePaths }
}
containsRule(cb: (rule: postcss.AnyNode) => boolean) {
let contains = false
this.root.walk((rule) => {
if (cb(rule)) {
contains = true
return false
}
})
if (contains) {
return true
}
for (let child of this.children) {
if (child.item.containsRule(cb)) {
return true
}
}
return false
}
[util.inspect.custom]() {
return {
...this,