JSON.stringify should use JsonSerializable interface if possible

[colinmollenhour]

Awesome work on this extension!

It would great if calling JSON.stringify() on a PHP object would make use of that object's jsonSerialize method if the object implements JsonSerializable. That is, the JSON.stringify() result would be easily controlled by the PHP author and have the same outcome as json_encode().

<?php
class Foo implements JsonSerializable {
    public $bar = 'bar';

    public function jsonSerialize()
    {
        return ['bar' => $this->bar];
    }
}
$v8 = new V8Js();
$v8->foo = new Foo;
$v8->executeString('
print( JSON.stringify(PHP.foo) );
');

Given the above code the current result is:

{"$bar":"bar"}

But the desired result is:

{"bar":"bar"}

[chrisbckr]

It's possible to do something like toString() -> __toString() ?
I have tried to implement this in v8js_named_property_enumerator and v8js_named_property_callback but no luck.

Did some debugging and found that callback_type == V8JS_PROP_GETTER was not been called. So I changed the ret_value when callback_type == V8JS_PROP_QUERY to v8::None and then V8JS_PROP_GETTER worker, but not with the value returned by jsonSerialize function.
I'm missing something, I just need to find out what it is.

[redbullmarky]

@chrisbckr Funnily enough, have been looking back on some of these enhancements too (particularly this and the forEach one, #451 ) to see if I can get somewhere and also to dust off my internals knowledge :) I’ll let you know if I find anything.

[stesie]

@chrisbckr PHP's __toString is already transparently mapped to toString. Hence this already works:

php > class C { function __toString() { return 'blarg'; }}
php > $c = new C();
php > var_dump(  (string) $c );
string(5) "blarg"
php > $v8->c = $c;
php > $v8->executeString(' print( String( PHP.c ) ); ');
blarg

JSON.stringify is calling toJSON to provide an object representation, if available.

However it seems that V8 isn't invoking the proxied method:

php > class A { function toJSON() { return [ 'blaaaa' => 42 ]; }}
php > $a = new A();
php > var_dump($a->toJSON());
array(1) {
  ["blaaaa"]=>
  int(42)
}
php > $v8 = new V8Js();
php > $v8->a = $a;
php > $v8->executeString('  print( JSON.stringify( PHP.a )); ');
{}

if we create a JS object and bind toJSON manually, then it happily invokes it like so:

php > $v8->executeString('b = { toJSON: PHP.a.toJSON.bind(PHP.a) };   print( JSON.stringify( b )); ');
{"blaaaa":42}

I haven't (yet) dug deeper into V8 if there's some reason for this behaviour. It might also be worth a try to actively provide a toJSON property/implementation to V8 instead of relying on the proxying/accessor logic.

[stesie]

from V8's source

MaybeHandle<Object> JsonStringifier::ApplyToJsonFunction(Handle<Object> object,
                                                         Handle<Object> key) {
  HandleScope scope(isolate_);

  // Retrieve toJSON function. The LookupIterator automatically handles
  // the ToObject() equivalent ("GetRoot") if {object} is a BigInt.
  Handle<Object> fun;
  LookupIterator it(isolate_, object, tojson_string_,
                    LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
  ASSIGN_RETURN_ON_EXCEPTION(isolate_, fun, Object::GetProperty(&it), Object);
  if (!fun->IsCallable()) return object;

... I don't know why they skip interceptors there. But they do :-/

Seems like we would have to actively export a toJSON symbol, when exporting the class (if it implements the \JsonSerializable interface).

[chrisbckr]

Hmmm I had a look at this code today but didn't pay attention enough to this detail. I will see how to check for the interface and how to implement this.

Thank you!!!

[stesie]

There already are interface checks for ArrayAccess and Countable, maybe have a look at those.
Active export of functions is done with the V8Js object itself for reasons of legacy. You may check the end of PHP_METHOD(V8Js, __construct) for how to do it.

[chrisbckr]

I'll look at this!
Thank you @stesie !

[chrisbckr]

Something like this?
#510
Don't know if I put on the right place or checked everything that need.
Probably need some improvements, but I think that is the way to do.

[colinmollenhour]

Thank you @chrisbckr and @stesie !! Ya'll are rock stars!