Fix fontSize array upgrade
#17630
Fix fontSize array upgrade
#17630
Conversation
| @@ -130,7 +130,13 @@ export function themeableValues(config: ResolvedConfig['theme']): [string[], unk | |||
| } | |||
|
|
|||
| if (Array.isArray(value) && value.every((v) => typeof v === 'string')) { | |||
| toAdd.push([path, value.join(', ')]) | |||
| if (path[0] === 'fontSize') { | |||
There was a problem hiding this comment.
| if (path[0] === 'fontSize') { | |
| if (path[0] === 'fontSize' && value.length === 2) { |
I'd probably want to make sure this only works for tuples.
There was a problem hiding this comment.
I think if we are to keep maximum compatibility with v3, we shouldn't restrict to 2 values. In v3, it still emits line-height with > 2 values, though I'm not sure how likely this would have occurred in the real world. Though one could argue we shouldn't support > 2 values as this was never truly intended.
Though thinking about this, perhaps we may need to check that there is at least two values for the value[1] reference?
There was a problem hiding this comment.
ah yep let's check for at least 2 entries then
There was a problem hiding this comment.
I've just tested again in v3 and an array value with one element, i.e. 2xl: ['2rem'] actually unsets the line-height. I'll assume we want to mirror this behavior as well in the compat layer. Edit: but only for extends? 🤔
There was a problem hiding this comment.
Perhaps this logic is better placed in mergeTheme() in packages/tailwindcss/src/compat/config/resolve-config.ts? We already do some massaging of screens here, so perhaps it would be best to have special cases be reworked in this function.
There was a problem hiding this comment.
@wongjn Hm that's interesting. Maybe we'll keep this in the back of our minds for now and see if this is actually causing an issue for people?
wongjn
force-pushed
the
fix-fontSize-array-upgrade
branch
from
f1d887e to
798d687
Compare
Fixes #17622.
Adds a specific handling case in
themeableValues()inpackages/tailwindcss/src/compat/apply-config-to-theme.ts. It seems like this has unique handling in v3 for an array value, whereby the second value is treated as aline-height.