-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Handle future and experimental config keys during upgrade
#19344
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
Conversation
WalkthroughAdds explicit support and validation for Pre-merge checks✅ Passed checks (5 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/@tailwindcss-upgrade/src/codemods/config/migrate-js-config.ts (1)
405-416: Future/experimental validation logic is sound; consider a small naming cleanupThe changes in
canMigrateConfiglook good overall:
- Adding
futureandexperimentaltoknownPropertiesremoves the blanket rejection of any config that happened to use those options.- The new guards that:
- allow
future: 'all'/experimental: 'all', and- require all object keys under
future/experimentalto be from a known allow‑list
are a reasonable, conservative strategy for an auto‑migration: known flags migrate, unknown ones safely fall back to@configwithout guessing.One small refinement to keep things clear:
- In the experimental block, the local variable is still named
knownFutureFlags. Renaming it to something likeknownExperimentalFlagswould avoid confusion when reading this function in isolation.Functionally this is fine as‑is; the rename is just for readability.
Also applies to: 430-452
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)integrations/upgrade/js-config.test.ts(1 hunks)packages/@tailwindcss-upgrade/src/codemods/config/migrate-js-config.ts(2 hunks)packages/tailwindcss/src/compat/config/resolve-config.ts(1 hunks)packages/tailwindcss/src/compat/config/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
integrations/upgrade/js-config.test.ts (1)
integrations/utils.ts (4)
test(80-455)json(523-523)ts(520-520)css(518-518)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Linux / upgrade
- GitHub Check: Linux
🔇 Additional comments (6)
CHANGELOG.md (1)
17-17: Changelog entry is clear and consistentThe new “Upgrade: Handle
futureandexperimentalconfig keys” bullet matches the PR behavior and existing changelog style; nothing to change here.packages/tailwindcss/src/compat/config/resolve-config.ts (1)
33-45: Initializingexperimentalin the minimal resolved config looks rightAdding
experimental: {}tominimalkeepsResolvedConfig’s shape consistent with the types and with the existingfuture: {}handling. No issues here.integrations/upgrade/js-config.test.ts (3)
1931-2006: LGTM! Correctly validates rejection of unknown future keys.The test properly verifies that when an unknown future key is encountered, the migration is aborted and the config file is preserved with an
@configreference. This is the correct defensive behavior.
2008-2083: LGTM! Correctly validates rejection of unknown experimental keys.The test mirrors the unknown future keys test but for experimental configuration keys, ensuring both validation paths work correctly. When an unknown experimental key is encountered, migration is appropriately aborted and the config is preserved.
1845-1929: LGTM! Verification confirms all future/experimental keys are properly recognized.The test correctly verifies that all used configuration keys are explicitly defined in the migration codemod's known flags lists:
- Future flags (
hoverOnlyWhenSupported,respectDefaultRingColorOpacity,disableColorOpacityUtilitiesByDefault,relativeContentPathsByDefault) ✓- Experimental flags (
generalizedModifiers) ✓The migration logic will successfully process these keys without bailing, and the test validates the expected output format with the empty config file and migrated CSS.
packages/tailwindcss/src/compat/config/types.ts (1)
108-116: LGTM! Clean and consistent type definitions for experimental config.The type additions properly extend the configuration surface to support experimental keys, mirroring the existing
futurekey structure. TheUserConfiguses an optional field whileResolvedConfigrequires it (which aligns with initialization to an empty object in resolve-config.ts).
Fixes #19342