From a4b112cb0b93cbde4d217ebc302b4f448e6d6107 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 23 Jan 2025 16:19:48 -0800 Subject: [PATCH] Do not consume delimeter when not consuming component value (#48841) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/48841 Right now during parsing we can ask for a next component value, with a delimeter, and even if we don't have a component value to consume, we will consume the delimeter. This is kind of awkward since e.g. trailing comma can be consumed, then we think syntax is valid. Let's try changing this. Changelog: [Internal] Reviewed By: lenaic Differential Revision: D68474739 fbshipit-source-id: 47a942681bc8472ca28470eba821d4d95306ae5d --- .../react/renderer/css/CSSSyntaxParser.h | 3 +++ .../css/tests/CSSSyntaxParserTest.cpp | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h b/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h index 5eea737d7fd..22c96834b9f 100644 --- a/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h +++ b/packages/react-native/ReactCommon/react/renderer/css/CSSSyntaxParser.h @@ -233,11 +233,14 @@ struct CSSComponentValueVisitorDispatcher { constexpr ReturnT consumeComponentValue( CSSDelimiter delimiter, const VisitorsT&... visitors) { + auto originalParser = parser; if (!consumeDelimiter(delimiter)) { + parser = originalParser; return {}; } if (parser.peek().type() == parser.terminator_) { + parser = originalParser; return {}; } diff --git a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp index 9dc8aa6721a..86336b104de 100644 --- a/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/css/tests/CSSSyntaxParserTest.cpp @@ -579,4 +579,31 @@ TEST(CSSSyntaxParser, solidus_or_whitespace) { EXPECT_FALSE(delimValue1); } +TEST(CSSSyntaxParser, delimeter_not_consumed_when_no_component_value) { + CSSSyntaxParser parser{"foo ,"}; + + auto identValue = parser.consumeComponentValue( + [](const CSSPreservedToken& token) { + EXPECT_EQ(token.type(), CSSTokenType::Ident); + EXPECT_EQ(token.stringValue(), "foo"); + return token.stringValue(); + }); + + EXPECT_EQ(identValue, "foo"); + + auto identValue2 = parser.consumeComponentValue( + CSSDelimiter::Comma, + [](const CSSPreservedToken& /*token*/) { return true; }); + + EXPECT_FALSE(identValue2); + + auto hasComma = parser.consumeComponentValue( + CSSDelimiter::Whitespace, [](const CSSPreservedToken& token) { + EXPECT_EQ(token.type(), CSSTokenType::Comma); + return true; + }); + + EXPECT_TRUE(hasComma); +} + } // namespace facebook::react