Skip to content
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

Migrate at-apply utilites with template migrations #14574

Merged

Conversation

philipp-spiess
Copy link
Member

@philipp-spiess philipp-spiess commented Oct 2, 2024

This PR extracts all candidate migrations from the existing template migrations and reuses these in the @apply CSS migration. Seems like this JustWorks✨.

@philipp-spiess philipp-spiess force-pushed the feat/migrate-at-apply-candidates-with-candidate-migrations branch from 8b2c5ea to f306e98 Compare October 2, 2024 10:56
Comment on lines +39 to +43
// If we have a valid designSystem and config setup, we can run all
// candidate migrations on each utility
if (designSystem && userConfig) {
params = params.map((param) => migrateCandidate(designSystem, userConfig, param))
}
Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably consider making a JS config file input required. This way we can simplify this decision here and don't have a case where @apply is migrated differently. Any thoughts on this @RobinMalfait?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but I think a better way is to try to figure out where the config file lives (similar to how Tailwind CSS v3 just finds the config file if it's a known file name + extension in the root). If you have a custom file (aka, we can't resolve one) then the --config option should be required.

Another tricky thing, but will be more related to the stuff I'm working on with @thecrypticace (where we handle stylesheets as graphs to figure out which file imports what), is the fact that in theory you can have multiple roots with different settings. Which means that @apply in one file could require tw: prefixes, and the other file might not require prefixes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that makes sense! Adding a task

@philipp-spiess philipp-spiess force-pushed the feat/migrate-at-apply-candidates-with-candidate-migrations branch from d2bbddd to 9f1419a Compare October 3, 2024 09:23
'src/input.css',
css`
.btn {
@apply tw:rounded-md! tw:px-2 tw:py-1 tw:bg-blue-500 tw:text-white;
Copy link
Member

Choose a reason for hiding this comment

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

It's so beautiful 🥹

@adamwathan adamwathan merged commit 2e87288 into next Oct 3, 2024
1 check passed
@adamwathan adamwathan deleted the feat/migrate-at-apply-candidates-with-candidate-migrations branch October 3, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants