Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. WalkthroughLegacy container padding keys are retained when they match a configured breakpoint or custom container screen. Unmatched keys are skipped. Custom-screen padding is added to its screen rule, and padding is sorted only when there are no custom screen overwrites. Tests cover unmatched Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The reviewed container-padding behavior matches the intended handling of unknown keys and custom screens. No actionable issue remains before merge. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: e3e0f1f9-78b6-4c01-a75e-119fdc245fde
📒 Files selected for processing (3)
CHANGELOG.mdpackages/tailwindcss/src/compat/container-config.test.tspackages/tailwindcss/src/compat/container.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
Summary
buildCustomContainerUtilityRules(used for the legacytheme.containerconfig and by@tailwindcss/upgrade) resolves everypaddingkey withtheme.resolveValue(key, ['--breakpoint'])and sorts the result withcompareBreakpoints. A key that isn't a breakpoint resolves tonull, andcompareBreakpointsthen callsnull.indexOf('('), so the build throwsTypeError: Cannot read properties of null (reading 'indexOf'). The.filter(Boolean)after themaplooks like it was meant to drop those keys, but it runs on the[key, breakpoint, value]tuples, which are always truthy.This happens with a valid v3 config where a padding key is only defined as a
containerscreen:Without custom
screens, a padding key that isn't a breakpoint crashes the same way, or, when it is the only one, makes the wholecontainerutility disappear becausetheme(--breakpoint-xs)can't be resolved.The fix filters on the resolved value instead: keys that are neither a breakpoint nor a custom
containerscreen are skipped, like in v3. With customcontainerscreens, each padding value is added to that screen's own rule, so the sort is only done when there are no custom screens.Test plan
packages/tailwindcss/src/compat/container-config.test.ts. Both fail before the change with theTypeErrorabove and pass after it:padding applies to custom container screensnow has anxscontainer screen withxspadding, and the snapshot gains the@media (min-width: 30rem)rule withpadding-inline: 1rem.allows padding to be defined at custom breakpointsnow has anxspadding key that isn't a breakpoint. The snapshot is unchanged because the key is ignored.vitest run src/compat/container-config.test.ts(inpackages/tailwindcss)vitest run(inpackages/tailwindcss): 42 files, 5015 tests passedprettier --checkon the changed files