Allow deserialization of missing objects properties into Option<>

[hansl]

Currently the TryFromJs derive macro requires that all properties be defined in the object, whether they are declared as Option<> or not. This commit makes it so if the field is not found in the JsObject, we try to deserialize undefined.

---

It changes the following:

• When using the macro to deserialize a struct, if a property descriptor is not found for a field, use JsValue::undefined() to deserialize instead.
• Add unit tests for the above change.

This is more in line with JavaScript's expected behaviour. It also allows structure to have partial defined fields.

[hansl]

I decided to make this PR tentatively as a start for the discussion. I was not sure if the current behaviour was discussed before (I could not find an discussion, issues or PRs that mentioned it).

I think this PR makes sense in the context where people are interfacing rust code with JavaScript calls, as it removes a lot of unnecessary JS when calling into rust synthetic modules.

[jedel1043]

I'm pretty sure the current behaviour is not intentional, but I'm pinging @Razican in case I'm wrong.

[hansl]

@jedel1043 That's what I was hoping for.

Do you know why CI is not happy? I think it detects boa_engine in the dev-dependencies but doesn't realize it should be unused in the crate, only in its tests (and it is used in /core/macros/tests/try_from_js.rs).

[Razican]

This behavior was not defined at the beginning. What should we do when a field doesn't exist? What if the object has more fields?

serde has attributes for these cases. I'm happy to go with whatever serde does :)

[hansl]

I cannot reproduce the CI doc test failures locally. And it seems unrelated to my changes.

[hansl]

@Razican For de-serialization, serde sets Option<> as optional and missing, so I surmise this PR should go in at a first step to fixing this. If you want to fully go down the serde route I would suggest implementing TryFromJs for serde::Deserialize and remove the derive macro entirely. That would be outside of the scope of this PR though.

WDYT?

[jedel1043]

Will take a look to the CI errors.

[jedel1043]

Fixed. The circular dependency between boa_engine and boa_macros was causing some linking problems, that's why in the past we had to extract the boa_macros tests to an external crate.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 47.73%. Comparing base (6ddc2b4) to head (80ad7fb).
Report is 107 commits behind head on main.

Files	Patch %	Lines
core/macros/src/lib.rs	0.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3767      +/-   ##
==========================================
+ Coverage   47.24%   47.73%   +0.48%     
==========================================
  Files         476      454      -22     
  Lines       46892    44747    -2145     
==========================================
- Hits        22154    21358     -796     
+ Misses      24738    23389    -1349     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hansl]

Oh thank you. Somehow I missed that. This is ready for review as is then. Personally I'd rather this goes in and then work separately on serde-ifying TryFromJs.

[HalidOdat]

Nice work! Looks good to me! :)

[jedel1043]

Everything looks good! Just a missing double colon.

[jedel1043]

Thank you for the contribution!