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

Feat: Adding a warning in the upgrade CLI when a config file contains non-migratable values. #16113

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

baptisthecht
Copy link

I encountered an issue in several of my projects during migration. I was unable to migrate correctly because my config file contained the now-deprecated "safelist" key. I had to reverse-engineer the problem to identify what was blocking me. To help others facing the same issue, I added this warning to inform them that they need to edit their config file for a smooth automatic migration.

@baptisthecht baptisthecht requested a review from a team as a code owner January 31, 2025 11:39
@baptisthecht baptisthecht changed the title Feat: add warning for non-migratable keys in upgrade-cli Feat: Adding a warning in the upgrade CLI when a config file contains non-migratable values. Jan 31, 2025
@philipp-spiess
Copy link
Member

@baptisthecht Thanks! It would be good if we can add some warnings to the other cases where we opt-out of migrating to a CSS file as well and add a quick test to ensure this works. Would you be interested to take a stab at this?

@baptisthecht
Copy link
Author

@baptisthecht Thanks! It would be good if we can add some warnings to the other cases where we opt-out of migrating to a CSS file as well and add a quick test to ensure this works. Would you be interested to take a stab at this?

Sure !

@baptisthecht
Copy link
Author

baptisthecht commented Feb 11, 2025

Should we keep the non-migratable Tailwind properties by linking the old tailwind.config.xxs file and warning that they are deprecated, or should we ignore the deprecated properties and proceed with the migration anyway?

I think we could migrate by skipping extra properties like the corePlugins property:

if ('corePlugins' in unresolvedConfig) {
  info(
    `The \`corePlugins\` option is no longer supported as of Tailwind CSS v4.0, so it has been removed from your configuration.`
  );
}

So, we may exclude the knownProperties part of the canMigrateConfig function and just skip non-migratable properties.

Then, we can add warnings for other non-migratable cases, such as static plugins or presets.

@philipp-spiess
Copy link
Member

I thin it's fine to skip corePlugins like you said but I'm unsure of skipping all unknown properties without an allow list. Could lead to some confusion since plugins in v3 you can read the whole config object so there might be some plugin specific settings: https://play.tailwindcss.com/1ZICA06UpK?file=config

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.

2 participants