-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove layer(utilities)
if imports contain @utility
#14738
Remove layer(utilities)
if imports contain @utility
#14738
Conversation
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.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @RobinMalfait and the rest of your teammates on Graphite |
} | ||
} | ||
|
||
return { convertablePaths, nonConvertablePaths } | ||
return { convertiblePaths, nonConvertiblePaths } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol what a typo 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be fair, convertable
makes more sense to my non-native-English brain lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean same — it does make more sense. I guess I just wasn't thinking when I wrote it originally haha
if (!Array.from(sheet.importRules).some((node) => node.raws.tailwind_injected_layer)) { | ||
continue | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can also skip stylesheets with parent stylesheets yeah?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wait not if the tailwind stuff is in a non-root stylesheet so maybe not.
We have a migration that adds the
layer(…)
next to the@import
depending on the order of original values. For example:Will be turned into:
Because it used to exist between
utilities
andcomponents
. Without this it would be aftercomponents
.This results in an issue if an import has (deeply) nested
@utility
at-rules after migrations. This is because if this is generated:Once we interpret this (and thus flatten it), the final CSS would look like:
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.