Fix JsonDynamic related object serialisation problem in Jint.

[hyzx86]

fixes: #16290

fixes: #16292 (comment)

this PR shuld merge after #16292

[hyzx86]

Hi @lahma , can you help review this PR? Thanks!

[lahma]

Anything related to SetWrapObjectHandler related to STJ sounds like Jint isn't doing good enough job, so might be worth checking if it could.

[gvkries]

Anything related to SetWrapObjectHandler related to STJ sounds like Jint isn't doing good enough job, so might be worth checking if it could.

I think our problem here is more specific and it is not really a problem of Jint. We have an additional layer to support dynamics with STJ and when doing serialization inside Jint, this layer must be ignored.

I experimented with another approach by replacing the "toJSON" method, but don't know enough about Jint:

var engine = new Engine(options =>
{
    options.Interop.MemberAccessor = (eng, target, member) =>
    {
        if (target is JsonDynamicValue || target is JsonDynamicObject || target is JsonDynamicArray)
        {
            if (member == "toJSON")
            {
                return new ClrFunction(eng, "toJSON", (thisObject, _) =>
                {
                    var wrappedInstance = ((IObjectWrapper)thisObject).Target;

                    return JConvert.SerializeObject(wrappedInstance, JOptions.Default);
                });
            }
        }

        return null;
    };
});

[hyzx86]

Anything related to SetWrapObjectHandler related to STJ sounds like Jint isn't doing good enough job, so might be worth checking if it could.

I think Jint is good enough, it's just that STJ is too rudimentary. So we need to put a lot of effort into adapting it.

I experimented with another approach by replacing the "toJSON" method, but don't know enough about Jint:

This would also solve the problem, my solution would be to have the JsonDynamicXX type converted to an STJ object, as Jint already handles STJ objects by default

[gvkries]

This would also solve the problem, my solution would be to have the JsonDynamicXX type converted to an STJ object, as Jint already handles STJ objects by default

This sounds far more reasonable 👍

[hishamco]

If this rely on #16292, please don't merge until the first PR has been merged

[lahma]

Suggestion, make every new introduced class sealed by default. Maybe even internal if possible.

[hyzx86]

Suggestion, make every new introduced class sealed by default. Maybe even internal if possible.

Thanks , this problem has been solved in another PR, I just added the Jint processing to PR #16292 (comment)

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[hyzx86]

@hishamco This PR is ready to merge 😊