Fix "Cannot read properties of undefined" crash on malformed arbitrary value#18133
Merged
RobinMalfait merged 4 commits intomainfrom May 23, 2025
Merged
Fix "Cannot read properties of undefined" crash on malformed arbitrary value#18133RobinMalfait merged 4 commits intomainfrom
RobinMalfait merged 4 commits intomainfrom
Conversation
- Add missing `path`, which is required according to the types - Remove unused variables warnings
Right now if a candidate looks like `[--btn-border:var(--color-maroon)/90)]` then there is a bad closing `)`. In this case the value parser is parsing `var(--color-maroon)/90)`, and because of the `)` we will hit the "closing function" branch. This branch currently assumes that an opening `(` was used. I am actually not 100% sure what we should do here because I don't think there is a good solution. We could throw an error with more info, but since this kind of class could be returned from Oxide you may not have control over this at all.
thecrypticace
approved these changes
May 23, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a crash when an arbitrary value was malformed and crashed the build.
If you have a utility like
[--btn-border:var(--color-maroon)/90)]which is malformed, it will crash the build. It might not be easy to spot but the easy is the additional)after the90.The reason this crashes is because we parse the value
var(--color-maroon)/90)and when we see)we assume it's the end of a "function" which also assumes it was preceded by a(. This is not the case and we crash.This PR fixes that by not assuming the parsed object is available and uses
?to be safe and only accessnodesif it's available.I'm actually not 100% sure what the best solution is in this scenario because these candidates could (and will) be returned from Oxide so even if we throw a more descriptive error, it will still crash the build and you might not even have control over the candidate.
This candidate will now eventually generate the following CSS:
Which still looks odd, but even Lightning CSS doesn't throw an error in this case (because it's a CSS variable definition), so I think it's the best we can do. If you open your devtools you will see the weird values, so it's still debug-able.
Fixes: #17064
Test plan
Manually tested the candidate that crashed it, and after the change generated the above CSS. Then used it in JSFiddle to proof it's fixed now. https://jsfiddle.net/z850ykew/
Couldn't use Tailwind Play because the candidate will cause a crash there as well 😅