Mostly stylistic comments + objection to shipping deref/relying on defer in
tests that might be addressed in a separate CL
https://codereview.chromium.org/99573002/diff/1/src/promise.js
File src/promise.js (right):
https://codereview.chromium.org/99573002/diff/1/src/promise.js#newcode230
src/promise.js:230: typeof (then = x.then) === 'function') {
Can we not have assignments in conditions? Pretty please? Replace with
if (IS_NULL_OR_UNDEFINED(x)) return x;
var then = x.then;
if (typeof(then) !== 'function') return x;
https://codereview.chromium.org/99573002/diff/1/src/promise.js#newcode257
src/promise.js:257: var deferred = %_CallFunction(this,
PromiseDeferred);
Instead of using %_CallFunction here and above, let's have utility
method
PromiseDeferredImpl, replace these calls with PromiseDeferredImpl(this),
and
Promise.defer = function(this) { PromiseDeferredImpl(this); }
Even better, I suggest not shipping defer (see below) :)
https://codereview.chromium.org/99573002/diff/1/src/promise.js#newcode270
src/promise.js:270: function(r) { deferred.reject(r) }
Why 'count > 0' condition disappeared here? What if more than one of
values fail?
https://codereview.chromium.org/99573002/diff/1/src/promise.js#newcode281
src/promise.js:281: var deferred = %_CallFunction(this,
PromiseDeferred);
Ditto re: %_CallFunction
https://codereview.chromium.org/99573002/diff/1/src/promise.js#newcode301
src/promise.js:301: "defer", PromiseDeferred,
So, my understanding is that 'defer' is non-standard as of now? Let's
not ship non-standard API maybe? (maybe a separate CL)
https://codereview.chromium.org/99573002/diff/1/test/mjsunit/harmony/promises.js
File test/mjsunit/harmony/promises.js (right):
https://codereview.chromium.org/99573002/diff/1/test/mjsunit/harmony/promises.js#newcode343
test/mjsunit/harmony/promises.js:343: var deferred = Promise.defer()
Non necessary to fix in this CL, but I think it would be useful to make
this whole test more portable by not relying on non-standard
Promise.defer() API. Use a "user-land" deferred implementation for the
test.
https://codereview.chromium.org/99573002/
--
--
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].
For more options, visit https://groups.google.com/groups/opt_out.
