Upgrade: Ensure underscores in url() and var() are not escaped

[philipp-spiess]

This PR fixes an issue where currently a theme() function call inside an arbitrary value that used a dot in the key path:

let className = "ml-[theme(spacing[1.5])]"

Was causing issues when going though the codemod. The issue is that for candidates, we require _ to be escaped, since otherwise they will be replaced with underscore. When going through the codemods, the above candidate would be translated to the following CSS variable access:

let className = "ml-[var(--spacing-1\_5))"

Because the underscore was escaped, we now have an invalid string inside a JavaScript file (as the \ would escape inside the quoted string.

To resolve this, we decided that this common case (as its used by the Tailwind CSS default theme) should work without escaping. In #14776, we made the changes that CSS variables used via var() no longer unescape underscores. This PR extends that so that the Variant printer (that creates the serialized candidate representation after the codemods make changes) take this new encoding into account.

This will result in the above example being translated into:

let className = "ml-[var(--spacing-1_5))"

With no more escaping. Nice!

Test Plan

I have added test for this to the kitchen-sink upgrade tests. Furthermore, to ensure this really works full-stack, I have updated the kitchen-sink test to actually build the migrated project with Tailwind CSS v4. After doing so, we can assert that we indeed have the right class name in the generated CSS.

[philipp-spiess]

• Upgrade: Ensure underscores in url() and var() are not escaped #14778 👈
• Don't escape underscores for the first parameter of var() #14776
• next

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @philipp-spiess and the rest of your teammates on Graphite

[adamwathan]

Any reason not to inline this if it's only ever called once? Also wonder if it would be helpful to use a more descriptive message like "Unexpected node kind `${node.kind}` is not handled for when escaping underscores" ?

[philipp-spiess]

The idea here is to use the never type so that TypeScript can actually notify us when we add a test (creating an exhaustive switch)

But... I did screw this up, we actually need to assert that node.value has the type never not just the return function here. 😶‍🌫️

This way that symbol is never read, it won't even compile if a new type to the ValueParser is added!

[RobinMalfait]

Another fun solution to this problem is to use

default:
  node satisfies never

Then you don't get a runtime error, but only a compile time one. But I think in either solution (current one and the one I'm showing) we always build regardless of build errors (or at least, it seems to be building locally) 🤔.

[philipp-spiess]

Ah I tried with as and it wasn't working, didn't test satisfies! Yeah might be cleaner.

On the other thing: I think it's fine if it actually builds but at least CI should yell at us if we have a type issue somewhere.