Ensure strings are consumed as-is when using internal segment()
#13608
Conversation
When encountering strings when using `segment` we didn't really treat them as actual strings. This means that if you used any parens, brackets, or curlies then we wanted them to be properly balanced. This should not be the case, whenever we encounter a string, we want to consume it as-is and don't want to worry about bracket balancing. We will now consume it until the end of the string (and make sure that escaped closing quotes are not seen as real closing quotes).
Already had this test
|
@RobinMalfait @thecrypticace It may be useful to include tests for bracket and quote chars in arbitrary values, specifically, to include tests similar to the original issue: e.g. We don't need to include all of these, but these should all pass with the proper fix, perhaps with some minor alterations: https://play.tailwindcss.com/EAhSfQFfTa |
There was a problem hiding this comment.
Nice! This looks good to me.
Couple things:
- Should we add a test somewhere for the higher level issue that made us notice this, or is there not really a good/obvious place to put it? It's kinda weird to put it in the
supportstests because it applies to other stuff like thedatavariant too. - Did you benchmark this against the previous implementation (even though it had a bug)? Hoping it's basically negligible.
|
@RobinMalfait Thanks for the quick fix here! |
|
Yeah good ideas to add another, more high level test. My thinking is to add a generic test for candidate parsing so that we know that we at least parse it into the correct data structure. If we want, we can add a specific one for Right now we don't really have generic tests for this. Benchmarking wise, it's actually even better when we encounter strings because we can skip a lot of work. The checks to know that we are in a string or a single integer check. Current PR: Next branch: |
CHANGELOG.md
Outdated
| @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
| ### Fixed | |||
|
|
|||
| - Make sure `contain-*` utility variables resolve to a valid value ([#13521](https://github.com/tailwindlabs/tailwindcss/pull/13521)) | |||
| - Ensure strings are consumed as-is when using internal `segment()` ([#13608](https://github.com/tailwindlabs/tailwindcss/pull/13608)) | |||
There was a problem hiding this comment.
Should we change this to represent the user-facing fix? Maybe something more like "Support unbalanced parentheses and braces in quotes in arbitrary values and variants".
| case SINGLE_QUOTE: | ||
| case DOUBLE_QUOTE: | ||
| while ( | ||
| // Ensure we don't go out of bounds. |
There was a problem hiding this comment.
This might look less weird with the comment above the while instead of in the condition.
This PR fixes an issue where parens, bracket and curlies needed to be balanced inside of strings instead of consuming the string until the end.
When encountering strings when using
segmentwe didn't really treat them as actual strings. This means that if you used any parens, brackets, or curlies then we wanted them to be properly balanced.This should not be the case, whenever we encounter a string, we want to consume it as-is and don't want to worry about bracket balancing. We will now consume it until the end of the string (and make sure that escaped closing quotes are not seen as real closing quotes).
Fixes: #13607