lodash flatMap internals are affecting the cascade in Tailwind's output

[AlexVipond]

Discovered this after digging into #1013

TL;DR Tailwind probably needs to avoid _.flatMap to avoid unexpected results in the cascade

What's the problem?

Right now, .border comes at the bottom of the border cascade, so it overrides .border-2 etc.

https://github.com/tailwindcss/tailwindcss/blob/beeaa65e4f1d046823766ca8f2410e14e12a205f/__tests__/fixtures/tailwind-output.css#L3145-L3167

This is inconsistent with the behavior of borderRadius and boxShadow, two other plugins that can use the default keyword to produce classes like .rounded and .shadow.

https://github.com/tailwindcss/tailwindcss/blob/beeaa65e4f1d046823766ca8f2410e14e12a205f/__tests__/fixtures/tailwind-output.css#L5815-L5825

https://github.com/tailwindcss/tailwindcss/blob/beeaa65e4f1d046823766ca8f2410e14e12a205f/__tests__/fixtures/tailwind-output.css#L2941-L2951

Why is this happening?

Tailwind's borderWidth plugin uses lodash's flatMap function, which uses lodash's identity utility internally.

https://github.com/tailwindcss/tailwindcss/blob/beeaa65e4f1d046823766ca8f2410e14e12a205f/src/plugins/borderWidth.js#L18-L20

I found out that when you use lodash identity on an object that has any keys that can be coerced to numbers (e.g. '1'), it will coerce those keys to numbers and sort them ascending:

const borderWidth = {
  default: '1px',
  '0': '0',
  '2': '2px',
  '4': '4px',
  '8': '8px',
}

console.log(_.identity(borderWidth))
// { 0: '0', 2: '2px', 4: '4px', 8: '8px', default: '1px' }

Therefore, when Tailwind iterates through the config object to generate CSS classes, default is the last key, and that translates to the .border class showing up last as seen above.

This same bug affects the margin and padding plugins, which reference the spacing key. According to the default config's spacing key, .m-px and .p-px should show up first in the CSS:

https://github.com/tailwindcss/tailwindcss/blob/beeaa65e4f1d046823766ca8f2410e14e12a205f/stubs/defaultConfig.stub.js#L129-L131

But, since margin and padding also use _.flatMap and in turn _.identity, .m-px and .p-px are at the bottom of the cascade:

https://github.com/tailwindcss/tailwindcss/blob/beeaa65e4f1d046823766ca8f2410e14e12a205f/__tests__/fixtures/tailwind-output.css#L3847-L3861

Which plugins are affected?

• borderWidth
• borderRadius
• container
• inset
• margin
• padding

[adamwathan]

This is actually just JavaScript, numeric keys are always automatically sorted first when an object is created:

In general Tailwind makes no guarantees about what should happen when you try to use two utility classes that affect the same property, you should really just only ever apply one class instead of hoping one overrides another in exactly the way that would be convenient for whatever situation you are in.

Nothing to really fix here IMO, if anything we could re-order the keys in the default config file to match the order JavaScript is going to coerce the items into so it's at least less surprising.

[AlexVipond]

I think the most bulletproof solution would be to allow users to pass Map-like arrays in the config file if they want to enforce a certain cascade:

// tailwind.config.js
module.exports = {
  borderWidth: {
    ["default", "1px"],
    ["0", "0"],
    ["2", "2px"],
    ["4", "4px"],
    ["8", "8px],
  }
}

From there, we'd have to add some logic to the resolveConfig util to handle maps in addition to closures and objects.

This feels pretty nifty, but it raises the question of what the theme function would return.

One solution is for theme to resolve closures and just return the result, i.e. a map-like array or an object. whichever was passed by the user in their config. The downside is that every plugin, both core and custom, would have to add logic to deal with each data format.

Another solution is for theme to always return map-like arrays or even actual constructed Maps. Would make it simpler to write plugins, but much more annoying in config files—let's say you don't care about the cascade, and you're just trying to spread theme('myPlugin') into an object. Unexpectedly, you would get a bunch of key/value arrays in your object.

I think the best of both worlds is to allow the theme function to take another argument, e.g. a boolean flag indicating whether or not it should return a map (defaults to false and returns an object, even if the user passed a map-like array). If done carefully I think this solution would not be a breaking change and would give the end user full control over their cascade.

[adamwathan]

I think this would add a lot of unnecessary complexity honestly, and would rather just continue to discourage people from using two classes that target the same property at the same time.

Really appreciate you thinking the problem through though 👍🏻