src: fix implementation of `PropertySetterCallback`

[targos]

V8 does not allow returning arbitrary values from the interceptor setter callbacks, only a boolean return value is allowed. Since default return value is true, it's not even necessary to set the return value on a successful path.

Refs: https://crbug.com/348660658

This was adapted from https://github.com/v8/node/pull/194

[aduh95]

V8 does not allow returning arbitrary values from the interceptor setter callbacks

I suppose that doesn't apply to the version of V8 we're currently compiling, otherwise I don't understand how we're able to build. Or is it only a warning for now?

[targos]

Maybe @isheludko has the answer?

[isheludko]

The call to args.GetReturnValue().Set(value); wasn't actually necessary since https://github.com/v8/node/pull/180, it was necessary for the old interceptors Api to indicate that the request was intercepted while the actual value was ignored.

This CL is a preparation for V8 CL which actually bans misuses of the ReturnValue: https://crrev.com/c/5647894. See https://chromium-review.googlesource.com/c/v8/v8/+/5647894/13/include/v8-function-callback.h. When v8_imminent_deprecation_warnings gn arg is true then the v8::ReturnValue<void>::Set(Local<S>) methods will trigger a static_assert failure with a descriptive message.

Heads-up: at some point V8 will support returning a boolean value from Setter/Definer callbacks for real : https://crbug.com/348660658.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/60147/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/60192/

[nodejs-github-bot]

Landed in e849dd66322332638a0a14eea3a2ea9ea9e5970c...eaffed65f0bc1dbd82a436d5c4cd14519d9ad632