Bug in Promise polyfill

[jamesnicholls]

The array check in the Promise.all method (line 296) will throw a TypeError. I think it should use Object.prototype.toString.

[monolithed]

Why Array.isArray has been replaced with String.prototype.toString.call?

[jonathantneal]

@jamesnicholls, you’re exactly right, and that’s exactly the fix. Did our tests not catch this?

[monolithed]

@jonathantneal, you'd use the A+ adapter:

'use strict';

/** @see https://github.com/promises-aplus/promises-tests */
var Promise = require('../index.js');

module.exports = {
    deferred: function () {
        var deferred = {};

        deferred.promise = new Promise(function (resolve, reject) {
            deferred.resolve = resolve;
            deferred.reject = reject;
        });

        return deferred;
    },

    resolved: function (value) {
        return Promise.resolve(value);
    },

    rejected: function (reason) {
        return Promise.reject(reason);
    }
};

And some custom tests like https://gist.github.com/monolithed/d18df15e2e6423d5c0f2

[triblondon]

@jonathantneal can we use the method from the original source polyfill? I'm struggling to see the benefit of your TO_STRING refactor.

[jonathantneal]

I avoided the need for Array.isArray as a check or a dependency. It’s just a typo. We should fix the typo. We should also make sure we have tests that would have prevented this kind of typo from even being committed.

[triblondon]

See https://github.com/Financial-Times/polyfill-service/pull/315. Jonathan, I think we should refactor this. Could you review #316?