[Webkit-unassigned] [Bug 160410] [ES2016] Implement Object.values

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=160410

--- Comment #14 from Yusuke Suzuki <utatane.tea at gmail.com> ---
Comment on attachment 285496
  --> https://bugs.webkit.org/attachment.cgi?id=285496
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=285496&action=review

Super nice!

> Source/JavaScriptCore/builtins/ObjectConstructor.js:34
> +        let nextKey =  ownKeys[i];

Let's check `If Type(key) is String, then` in the spec 4-a since we can get Symbol here.
(And adding a test for that is very nice thing)

> Source/JavaScriptCore/builtins/ObjectConstructor.js:38
> +                properties.push(object[nextKey]);

Array.prototype.push can be replaced by users.
So, let's use `properties. at push(object[nextKey])`. (And adding the test for replacing Array.prototype.push is good).

> Source/JavaScriptCore/builtins/ObjectConstructor.js:46
> +function values(o)

Right, the spec says this argument as O. But `object` is more readable I think :)

> Source/JavaScriptCore/builtins/ObjectConstructor.js:54
> +    if (o === null)
> +        throw new @TypeError("null is not an object (evaluating 'Object.values(value)')");
> +
> +    if (typeof o === "undefined")
> +        throw new @TypeError("undefined is not an object (evaluating 'Object.values(value)')");

Use `if (object == null)` to unify the above 2 errors. See the other builtin JS for the error message cases. (e.g. ArrayPrototype.js).

> Source/JavaScriptCore/builtins/ObjectConstructor.js:56
> +    return @enumerableOwnProperties(o, 'value');

How about using builtin intrinsic constant (for example, @IterationKindValue) instead of the string "value"?
We can easily expose C++ enums to builtin JS world by using builtin intrinsic constant. For example, in GeneratorPrototype.js, we use @GeneratorResumeModeNormal etc.
And unifying XXXIterationKind to one enum IterationKind is good cleaning up (For example, currently, there are ArrayIterationKind, MapIterationKind, and SetIterationKind in C++ world).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160807/ffeec015/attachment.html>