2
\$\begingroup\$

I have written this "Polyfill" for Chrome's Promise object and I have some concerns about the implementation;

  1. Does it actually work as designed? It seems to work but I'm not sure if there are any "edge cases" I am not accounting for.
  2. Is there a better way to do it? The inner function way seems a bit odd, as I don't like how I the promise[method] works.

Here is how the spread method works:

var resolvedArray = Promise.resolve([1, 2, 3]);

resolvedArray.spread(function(a, b, c) {
  console.log(a === 1); // true
  console.log(b === 2); // true
  console.log(c === 3); // true
});

var resolvedOther = Promise.resolve("Hello");

resolvedOther.spread(function(value) {
  console.log(value === "Hello"); // true
});

var rejectedArray = Promise.reject([1, 2, 3]);

rejectedArray.spread(undefined, function(a, b, c) {
  console.log(a === 1); // true
  console.log(b === 2); // true
  console.log(c === 3); // true
});

var rejectedObject = Promise.reject("Hello");

rejectedObject.spread(undefined, function(value) {
  console.log(value === "Hello"); // true
});

And here is my implementation:

Promise.prototype.spread = function() {
  var promise = this;

  var spread = function(promise, method, callback) {
    return promise[method](function(previousValue) {
      if (previousValue instanceof Array) {
        return callback.apply(undefined, previousValue);
      } else {
        return callback.call(undefined, previousValue);
      }
    });
  };

  if (arguments.length >= 1 && arguments[0] !== undefined) {
    promise = spread(promise, 'then', arguments[0]);
  }

  if (arguments.length >= 2 && arguments[1] !== undefined) {
    promise = spread(promise, 'catch', arguments[1]);
  }

  return promise;
};
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Quick question. What should be the behavior of spread() if the function returns an array that itself contains one or more promises? Should it wait on the array with Promise.all() like returning a promise from .then() would do? Or should it just return the array of promises and leave the .spread() caller to do that? I guess I'm thinking that it should wait on them, but since we don't have a well defined spec to go by, I thought I'd ask. \$\endgroup\$
    – jfriend00
    Mar 31, 2016 at 8:30
  • \$\begingroup\$ I've just realised that is probably the best "way" to do it, @jfriend00. I'll take a look at implementing that. \$\endgroup\$ Mar 31, 2016 at 16:40

1 Answer 1

3
\$\begingroup\$

Your implementation is taking .then(a, b) and mapping it to .then(a).catch(b) which is not exactly the same thing. An exception or return of a rejected promise in a in your implementation will hit the .catch(b), but in .then(a, b), it will not hit b, so I think your .spread() is not correctly imitating the .then() behavior in some circumstances.

Here's my implementation that should more closely mimic .then(a, b):

Promise.prototype.spread = function(a, b) {

    function doSpread(fn) {
        return function(args) {
            if (typeof fn === "function") {
                if (Array.isArray(args)) {
                    return fn.apply(null, args);
                } else {
                    return fn(args);
                }
            } else {
                return args;
            }
        }
    }
    if (typeof b !== "undefined") {
        // there is a reject handler
        return this.then(doSpread(a), doSpread(b));
    } else {
        // no reject handler
        return this.then(doSpread(a));
    }
}

// Array.isArray polyfill
if (!Array.isArray) {
  Array.isArray = function(arg) {
    return Object.prototype.toString.call(arg) === '[object Array]';
  };
}

Working demo and tests: https://jsfiddle.net/jfriend00/nqc0h10y/


And, here's a version that makes the change to wait on any promises that are returned in the array of values per our discussion in comments. I also changed it so that if you don't pass a function to .spread(), it will reject:

Promise.prototype.spread = function(a, b) {

    function doSpread(fn) {
        return function(args) {
            if (typeof fn === "function") {
                if (Array.isArray(args)) {
                    // in cases where there could be promises in the array, wait on those
                    return Promise.all(args).then(function(returnArgs) {
                        // split the array of results into separately passed arguments
                        return fn.apply(null, returnArgs);
                    });
                } else {
                    return fn(args);
                }
            } else {
                throw new Error("Must pass a function to .spread()");
            }
        }
    }
    if (typeof b !== "undefined") {
        // there is a reject handler
        return this.then(doSpread(a), doSpread(b));
    } else {
        // no reject handler
        return this.then(doSpread(a));
    }
}

// Array.isArray polyfill
if (!Array.isArray) {
  Array.isArray = function(arg) {
    return Object.prototype.toString.call(arg) === '[object Array]';
  };
}

Working demo: https://jsfiddle.net/jfriend00/om5auzs0/

\$\endgroup\$
4
  • \$\begingroup\$ @jackwilsdon - I guess I'm wondering what's the point of posting a solution if you're not going to even react or comment on it? \$\endgroup\$
    – jfriend00
    Mar 26, 2016 at 21:14
  • \$\begingroup\$ I think I see you doing two "helpful" automagic type-dependent behaviors here-- I would urge avoiding these, in favor of more straightforward type-independent behavior. Specifically, if args isn't an array, you fall back to regular "then" behavior; the danger is that people start using your spread as a normal "then", which will appear to work fine until the day when it gets called on a value that happens to be an array, and then it will do something wrong/unpredictable. Similarly for your fall-through when typeof fn !== "function"-- "then" doesn't do this, does it? I'd leave it out. \$\endgroup\$
    – Don Hatch
    Mar 30, 2016 at 9:03
  • \$\begingroup\$ @DonHatch - The option of an array or not is in the OP's implementation which I was attempting to emulate the behavior of since the usual goal here is offer better code that meets the same objective as what the OP posted. Is there an actual proposed standards specification for .spread() that should really be the implementation target? \$\endgroup\$
    – jfriend00
    Mar 30, 2016 at 17:40
  • \$\begingroup\$ @jackwilsdon - I added a new version that uses Promise.all() to wait on the returned array as discussed in comments. \$\endgroup\$
    – jfriend00
    Mar 31, 2016 at 21:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.