mirrors/ladybird
0
0
Fork 0
mirror of https://github.com/LadybirdBrowser/ladybird.git synced 2025-08-01 05:39:11 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

LibWeb/Bindings: Do put OptionalNone as member in returned dictionaries

Browse source 
Our handling of 'optional' return values was previously not correct
in that we would always call 'create_data_property' for every
single member of the returned dictionary, even if that property did
not have a value (by falling back to JS::js_null).

This was resulting in a massive number of test failures for URL
pattern which was expecting 'undefined' as the member value, instead
of 'null'.
This commit is contained in:
Shannon Booth 2025-03-23 20:35:39 +13:00 • committed by Tim Flynn
commit f64d5451df
Notes: github-actions[bot] 2025-04-06 12:26:30 +00:00
2 changed files with 193 additions and 175 deletions
Download patch file Download diff file Expand all files Collapse all files

28
Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp
View file

@ -2018,9 +2018,6 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
auto wrapped_value_name = ByteString::formatted("wrapped_{}", member_value_js_name);
dictionary_generator.set("wrapped_value_name", wrapped_value_name);
dictionary_generator.append(R"~~~(
JS::Value @wrapped_value_name@;
)~~~");
// NOTE: This has similar semantics as 'required' in WebIDL. However, the spec does not put 'required' on
// _returned_ dictionary members since with the way the spec is worded it has no normative effect to
// do so. We could implement this without the 'GenerateAsRequired' extended attribute, but it would require
@ -2028,11 +2025,27 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
// determine whether the type is present or not (e.g through a has_value() on an Optional<T>, or a null
// check on a GC::Ptr<T>). So to save some complexity in the generator, give ourselves a hint of what to do.
bool is_optional = !member.extended_attributes.contains("GenerateAsRequired") && !member.default_value.has_value();
if (is_optional) {
dictionary_generator.append(R"~~~(
Optional<JS::Value> @wrapped_value_name@;
)~~~");
} else {
dictionary_generator.append(R"~~~(
JS::Value @wrapped_value_name@;
)~~~");
}
generate_wrap_statement(dictionary_generator, ByteString::formatted("{}{}{}", value, type.is_nullable() ? "->" : ".", member.name.to_snakecase()), member.type, interface, ByteString::formatted("{} =", wrapped_value_name), WrappingReference::No, recursion_depth + 1, is_optional);
dictionary_generator.append(R"~~~(
if (is_optional) {
dictionary_generator.append(R"~~~(
if (@wrapped_value_name@.has_value())
MUST(dictionary_object@recursion_depth@->create_data_property("@member_key@"_fly_string, @wrapped_value_name@.release_value()));
)~~~");
} else {
dictionary_generator.append(R"~~~(
MUST(dictionary_object@recursion_depth@->create_data_property("@member_key@"_fly_string, @wrapped_value_name@));
)~~~");
}
}
if (current_dictionary->parent_name.is_empty())
@ -2061,11 +2074,16 @@ static void generate_wrap_statement(SourceGenerator& generator, ByteString const
}
}
if ((type.is_nullable() || is_optional) && !is<UnionType>(type)) {
if (type.is_nullable() && !is<UnionType>(type)) {
scoped_generator.append(R"~~~(
} else {
@result_expression@ JS::js_null();
}
)~~~");
} else if (is_optional) {
// Optional return values should not be assigned any value (including null) if the value is not present.
scoped_generator.append(R"~~~(
}
)~~~");
}
}