Do not consume delimeter when not consuming component value (#48841)

NickGerleman · facebook-github-bot · commit a4b112cb0b93 · 2025-01-23T16:19:48.000-08:00
Summary: Pull Request resolved: #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
diff --git 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
@@ -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<std::string_view>(
+      [](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<bool>(
+      CSSDelimiter::Comma,
+      [](const CSSPreservedToken& /*token*/) { return true; });
+
+  EXPECT_FALSE(identValue2);
+
+  auto hasComma = parser.consumeComponentValue<bool>(
+      CSSDelimiter::Whitespace, [](const CSSPreservedToken& token) {
+        EXPECT_EQ(token.type(), CSSTokenType::Comma);
+        return true;
+      });
+
+  EXPECT_TRUE(hasComma);
+}
+
 } // namespace facebook::react

Original file line number	Diff line number	Diff line change
`@@ -233,11 +233,14 @@ struct CSSComponentValueVisitorDispatcher {`
`233`	`233`	`constexpr ReturnT consumeComponentValue(`
`234`	`234`	`CSSDelimiter delimiter,`
`235`	`235`	`const VisitorsT&... visitors) {`
	`236`	`+ auto originalParser = parser;`
`236`	`237`	`if (!consumeDelimiter(delimiter)) {`
	`238`	`+ parser = originalParser;`
`237`	`239`	`return {};`
`238`	`240`	`}`
`239`	`241`
`240`	`242`	`if (parser.peek().type() == parser.terminator_) {`
	`243`	`+ parser = originalParser;`
`241`	`244`	`return {};`
`242`	`245`	`}`
`243`	`246`