Skip to content

Commit

Permalink
Do not consume delimeter when not consuming component value
Browse files Browse the repository at this point in the history
Summary:
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]

Differential Revision: D68474739
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Jan 22, 2025
1 parent 290ba82 commit a9db672
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,14 @@ struct CSSComponentValueVisitorDispatcher {
constexpr ReturnT consumeComponentValue(
CSSDelimiter delimiter,
const VisitorsT&... visitors) {
auto currentState = parser;
if (!consumeDelimiter(delimiter)) {
parser = currentState;
return {};
}

if (parser.peek().type() == parser.terminator_) {
parser = currentState;
return {};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit a9db672

Please sign in to comment.