promises-aplus / constructor-spec

Discussion and drafts of a possible spec for creating and resolving promises
10 stars 4 forks source link

Should we include assimilation? #22

Closed domenic closed 11 years ago

domenic commented 11 years ago

In #18, I threw in assimilation, under the name Promise.from, because it seemed like a nice parallel. I'm beginning to think it doesn't pull its weight, however, especially in terms of being specced.

In particular, libraries like Q and when already implement assimilation under the names Q and when, so Q.Promise.from is awkward. The case of utility libraries being able to create promises is taken care of by the promise constructor (i.e. discoverability via promiseInstance.constructor). And although I gave a try at a clear parallel, it's not as clean as I'd hoped.

On the other hand, getting across the point that assimilation must occur in the next tick would be a good thing for the community to grasp, IMO.

Maybe we should stop trying to tie assimilation into construction, and do a separate spec for it. Or maybe we should factor out the "assimilation algorithm" into either its own spec or part of the base spec, since it has some impact there (see https://github.com/promises-aplus/promises-spec/issues/75).

juandopazo commented 11 years ago

Y.Promise.from here. It is awkward. We went with Y.when, same as Q.

novemberborn commented 11 years ago

I don't see why we'd have to include it at this stage actually.

erights commented 11 years ago

Hi @domenic , @kriskowal , per our conversation, Promises/A+ should clearly distinguish three flavors of objects:

Then assimilation becomes something like the Q function shown (as of this writing) at http://code.google.com/p/es-lab/source/browse/trunk/src/ses/makeQ.js#204

 /**
  * Returns the promise form of value.
  *
  * <p>If value is already a promise, return it. Else if it is
  * null, undefined, or a primitive value, then wrap it in a
  * promise that is already resolved to value, even if
  * Object(value) would be considered thenable. Else return a
  * promise now for the result of testing the thenability of value
  * in a separate turn. To prevent plan interference attacks, this
  * testing must be in a separate turn.
  *
  * <p>In that later turn, if value is a thenable, then use its
  * "then" behavior to determine the resolution of the previously
  * returned promise. Else, fulfil the previously returned promise
  * with value.
  */
 function Q(value) {
   if (isPromise(value)) { return value; }
   if (value !== Object(value)) {
     return new Promise(NearHandler, value);
   }
   return promise(function(resolve, reject) {
     setTimeout(function() {
       if (typeof value.then === 'function') {
         try {
           value.then(resolve, reject);
         } catch (reason) {
           reject(reason);
         }
       } else {
         resolve(Promise(NearHandler, value));
       }
     }, 0);
   });
 }

Of course, substitute something appropriate for the setTimeout above.

One potentially controversial aspect of the above code: If the thenable's then method throws prior to resolving or rejecting, then the returned promise becomes rejected with the thrown reason. This would make assimilation more parallel to promise construction, and is also more likely to preserve the diagnostic to where it may be noticed.

kriskowal commented 11 years ago

@erights, our implementation for Q.next is functionally equivalent to above from the day we talked.

https://github.com/kriskowal/q/blob/master/q.js#L783-L793

erights commented 11 years ago

@kriskowal Clever. But your Q is calling Q.isPromiseAlike synchronously, which isn't safe -- you're open to plan interference attacks. You have to wait to test whether it's a thenable until a later turn.

domenic commented 11 years ago

I think we shouldn't include assimilation in the minimal resolver specification, which seems to be converging around a constructor. Closing this, and updating #18.