Excellent, thank you Igor. Probably the one suggestion I would make then is to expand either the doc comments around the deprecation notice or update the deprecation notice to reflect that `SetAccessorProperty()` might be more appropriate a replacement than `SetNativeDataProperty()`
Will take a look at how the other change may affect us! Thank you very much - James On Tuesday, May 21, 2024 at 1:37:31 AM UTC-7 [email protected] wrote: > Hello James, > > TL;DR;: the SetAccessor() Api creates data properties that behave > differently than expected by JavaScript spec, see > https://crbug.com/336325111 for details. > > Indeed, if your code is relying on triggering a setter callback installed > on the prototype object then the SetNativeDataProperty() does not fit your > needs. The right way to go would be to use SetAccessorProperty() Api > that'll define a proper accessor property in JavaScript sense and for such > a property an attempt to store via a child object would trigger the setter > callback. > > While we are on the V8 Api changing topic, here's another cleanup project > we are working on which might theoretically affect your code: > https://crbug.com/333672197. > > Regards, > -- Igor > > On Sun, May 19, 2024 at 10:14 PM 'James Snell' via v8-dev < > [email protected]> wrote: > >> In Cloudflare Workers we just updated to v8 12.6 and spotted a new >> pending deprecation notice saying to move from using SetAccessor to >> SetNativeDataProperty. Unfortunately, there appears to possibly to be an >> issue with that when using a `v8::FunctionTemplate` and setting the >> accessor property on the `PrototypeTemplate()` >> >> Take for example the following snippet (there are a few utility methods >> in here from the workerd codebase like check() and `js.str(...)` etc that >> are just helpers... >> >> auto tmpl = v8::FunctionTemplate::New(js.v8Isolate, nullptr); >> v8::Local<v8::String> foo = js.str("foo"_kj); >> >> tmpl->PrototypeTemplate()->SetNativeDataProperty(foo, >> [](v8::Local<v8::Name> name, const >> v8::PropertyCallbackInfo<v8::Value>& info) { >> auto& js = Lock::from(info.GetIsolate()); >> >> info.GetReturnValue().Set(v8::Local<v8::Value>(js.str("foo"_kj))); >> }, >> [](v8::Local<v8::Name> name, v8::Local<v8::Value> value, >> const v8::PropertyCallbackInfo<void>& info) { >> KJ_DBG("Setter is called!"); >> info.GetReturnValue().Set(value); >> }, v8::Local<v8::Value>(), v8::PropertyAttribute::None); >> >> v8::Local<v8::Object> obj = >> check(tmpl->InstanceTemplate()->NewInstance(js.v8Context())); >> >> // The setter is not called in this case! >> check(obj->Set(js.v8Context(), foo, js.num(1))); >> >> // The setter IS called in this one >> check(obj->GetPrototype().As<v8::Object>()->Set(js.v8Context(), >> foo, js.num(2))); >> >> In this case, the Setter configured on the obj prototype is not called >> when set via `obj->Set(...)`. >> >> The question is: with the intended move to `SetNativeDataProperty()` as >> communicated via the pending deprecation notice, is this a bug or is the >> behavior here intentional? Should setters provided >> `PrototypeInstance()->SetNativeDataProperty(...)` Just Work? Or am I just >> doing something wrong here? >> >> -- >> -- >> v8-dev mailing list >> [email protected] >> http://groups.google.com/group/v8-dev >> --- >> You received this message because you are subscribed to the Google Groups >> "v8-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/v8-dev/da4aa5ef-d4ae-4b35-ae5c-4946cf8f4d49n%40googlegroups.com >> >> <https://groups.google.com/d/msgid/v8-dev/da4aa5ef-d4ae-4b35-ae5c-4946cf8f4d49n%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> > > > -- > > Igor Sheludko > > Software Engineer > > [email protected] > > > Google Germany GmbH > > Erika-Mann-Straße 33 > > 80636 München > > > Geschäftsführer: Paul Manicle, Liana Sebastian > > Registergericht und -nummer: Hamburg, HRB 86891 > > Sitz der Gesellschaft: Hamburg > > > Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten > haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, > löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, > dass die E-Mail an die falsche Person gesendet wurde. > > > > This e-mail is confidential. If you received this communication by > mistake, please don't forward it to anyone else, please erase all copies > and attachments, and please let me know that it has gone to the wrong > person. > > > > > -- -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/82c96b60-deb4-4448-bf6c-a9b3007cc901n%40googlegroups.com.
