<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES2016] Implement Object.values"
href="https://bugs.webkit.org/show_bug.cgi?id=160410#c14">Comment # 14</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [ES2016] Implement Object.values"
href="https://bugs.webkit.org/show_bug.cgi?id=160410">bug 160410</a>
from <span class="vcard"><a class="email" href="mailto:utatane.tea@gmail.com" title="Yusuke Suzuki <utatane.tea@gmail.com>"> <span class="fn">Yusuke Suzuki</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=285496&action=diff" name="attach_285496" title="Patch">attachment 285496</a> <a href="attachment.cgi?id=285496&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=285496&action=review">https://bugs.webkit.org/attachment.cgi?id=285496&action=review</a>
Super nice!
<span class="quote">> Source/JavaScriptCore/builtins/ObjectConstructor.js:34
> + let nextKey = ownKeys[i];</span >
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)
<span class="quote">> Source/JavaScriptCore/builtins/ObjectConstructor.js:38
> + properties.push(object[nextKey]);</span >
Array.prototype.push can be replaced by users.
So, let's use `properties.@push(object[nextKey])`. (And adding the test for replacing Array.prototype.push is good).
<span class="quote">> Source/JavaScriptCore/builtins/ObjectConstructor.js:46
> +function values(o)</span >
Right, the spec says this argument as O. But `object` is more readable I think :)
<span class="quote">> 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)')");</span >
Use `if (object == null)` to unify the above 2 errors. See the other builtin JS for the error message cases. (e.g. ArrayPrototype.js).
<span class="quote">> Source/JavaScriptCore/builtins/ObjectConstructor.js:56
> + return @enumerableOwnProperties(o, 'value');</span >
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).</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>