Issue while Implementing Promise class with JavaScript

Question

I'm trying to implement Promise Class with JavaScript (currently only with resolve, no reject).
For some reason, when I use it without setTimeout around the resolve function, I get double console.logs from the 'then' methods , and otherwise it works fine.

That's my code

const STATUSES = {
  PENDING: 'PENDING',
  RESOLVED: 'RESOLVED',
  REJECTED: 'REJECTED',
}

class CustomPromise {
  #value = null
  #status = STATUSES.PENDING
  #thenCallbacks = []

  constructor(cb) {
    cb(this.#resolve)
  }

  #updateState = (status, value) => {
    this.#status = status
    this.#value = value
  }

  #resolve = (value) => {
    this.#updateState(STATUSES.RESOLVED, value)
    this.#thenCallbacks.reduce((acc, callback) => callback(acc), this.#value)
  }

  then = (cb) => {
    this.#thenCallbacks.push(cb)
    if (this.#status === STATUSES.RESOLVED) {
      this.#thenCallbacks.reduce((acc, callback) => callback(acc), this.#value)
    }

    return this
  }
}


new CustomPromise((resolve) => {
    resolve(1)
  })
  .then((value) => {
    console.log('first value', value)
    return 2
  })
  .then((value) => {
    console.log('second value', value)
    return null
  })

the result:

Wrapping the 'resolve' like so makes the code run as expected:

setTimeout(() => {
      resolve(1)
     }, [1000])

thanks!

trincot · Answer

The problem is in the then method: when you find that the promise already resolved, you should only call the newly provided callback -- not the others. In this case the other callbacks would already have been called.

Some other comments:

There is no accumulation like you suggest with reduce. All callbacks should be treated equally, each getting the value as argument.
It is not allowed for a promise to resolve more than once, so you should ignore such a request when you find that the promise status is no longer pending.

Here is a correction for the double output you got:

const STATUSES = {
  PENDING: 'PENDING',
  RESOLVED: 'RESOLVED',
  REJECTED: 'REJECTED',
}

class CustomPromise {
  #value = null;
  #status = STATUSES.PENDING;
  #thenCallbacks = [];

  constructor(cb) {
    cb(this.#resolve);
  }

  #updateState = (status, value) => {
    this.#status = status;
    this.#value = value;
  }

  #resolve = (value) => {
    if (this.#status !== STATUSES.PENDING) return; // Ignore!
    this.#updateState(STATUSES.RESOLVED, value);
    this.#thenCallbacks.forEach(callback => callback(value)); // No accumulation
  }

  then = (cb) => {
    this.#thenCallbacks.push(cb)
    if (this.#status === STATUSES.RESOLVED) {
      cb(this.#value); // Only this one. The other callbacks were already called
    }

    return this;
  }
}


new CustomPromise((resolve) => {
    resolve(1);
  })
  .then((value) => {
    console.log('first value', value);
    return 2;
  })
  .then((value) => {
    console.log('second value', value);
    return null;
  })

Note that you have much more work to do, including:

then callbacks should be called asynchronously.
then should return a new promise. Because this is not happening in your code, and a promise value cannot be changed, the output is now always the same value.
Promises should implement the promise resolution procedure.

You probably want to test your implementation against the Promises/A+ Compliance Test Suite. This way you can gradually improve your implementation until it passes all tests. Be warned... there is a long road ahead, but worth the experience.

To completely fix your code so it complies with Promises/A+ specification, a lot of change is required to your code. As I once did this effort of implementing a compliant implementation, I just provide you with the class version of what I provided in this answer:

class CustomPromise {
    static Deferred = class {
        constructor() {
            this.state = 'pending';
            this.value = undefined;
            this.consumers = [];
            this.promise = Object.create(CustomPromise.prototype, {
                then: { value: this.then.bind(this) }
            });
        }
        // 2.1.1.1: provide only two ways to transition
        fulfill(value) {
            if (this.state !== 'pending') return; // 2.1.2.1, 2.1.3.1: cannot transition anymore
            this.state = 'fulfilled'; // 2.1.1.1: can transition
            this.value = value; // 2.1.2.2: must have a value
            this.broadcast();
        }
        reject(reason) {
            if (this.state !== 'pending') return; // 2.1.2.1, 2.1.3.1: cannot transition anymore
            this.state = 'rejected'; // 2.1.1.1: can transition
            this.value = reason; // 2.1.3.2: must have a reason
            this.broadcast();
        }
        // A promise’s then method accepts two arguments:
        then(onFulfilled, onRejected) {
            var consumer = new CustomPromise.Deferred();
            // 2.2.1.1 ignore onFulfilled if not a function
            consumer.onFulfilled = typeof onFulfilled === 'function' ? onFulfilled : null;
            // 2.2.1.2 ignore onRejected if not a function
            consumer.onRejected = typeof onRejected === 'function' ? onRejected : null;
            // 2.2.6.1, 2.2.6.2: .then() may be called multiple times on the same promise
            this.consumers.push(consumer);
            // It might be that the promise was already resolved... 
            this.broadcast();
            // 2.2.7: .then() must return a promise
            return consumer.promise;
        }
        broadcast() {
            var promise = this;
            // 2.2.2.1, 2.2.2.2, 2.2.3.1, 2.2.3.2 called after promise is resolved
            if (this.state === 'pending') return;
            // 2.2.6.1, 2.2.6.2 all respective callbacks must execute
            var callbackName = this.state == 'fulfilled' ? 'onFulfilled' : 'onRejected';
            var resolver = this.state == 'fulfilled' ? 'resolve' : 'reject';
            // 2.2.4 onFulfilled/onRejected must be called asynchronously
            setTimeout(function() {
                // 2.2.6.1, 2.2.6.2 traverse in order, 2.2.2.3, 2.2.3.3 called only once
                promise.consumers.splice(0).forEach(function(consumer) {
                    try {
                        var callback = consumer[callbackName];
                        // 2.2.1.1, 2.2.1.2 ignore callback if not a function, else
                        // 2.2.5 call callback as plain function without context
                        if (callback) {
                            // 2.2.7.1. execute the Promise Resolution Procedure:
                            consumer.resolve(callback(promise.value)); 
                        } else {
                            // 2.2.7.3 resolve in same way as current promise
                            consumer[resolver](promise.value);
                        }
                    } catch (e) {
                        // 2.2.7.2
                        consumer.reject(e);
                    };
                })
            });
        }
        // The Promise Resolution Procedure: will treat values that are thenables/promises
        // and will eventually call either fulfill or reject/throw.
        resolve(x) {
            var wasCalled, then;
            // 2.3.1
            if (this.promise === x) {
                throw new TypeError('Circular reference: promise value is promise itself');
            }
            // 2.3.2
            if (x instanceof CustomPromise) {
                // 2.3.2.1, 2.3.2.2, 2.3.2.3
                x.then(this.resolve.bind(this), this.reject.bind(this));
            } else if (x === Object(x)) { // 2.3.3
                try {
                    // 2.3.3.1
                    then = x.then;
                    if (typeof then === 'function') {
                        // 2.3.3.3
                        then.call(x, function resolve(y) {
                            // 2.3.3.3.3 don't allow multiple calls
                            if (wasCalled) return;
                            wasCalled = true;
                            // 2.3.3.3.1 recurse
                            this.resolve(y);
                        }.bind(this), function reject(reasonY) {
                            // 2.3.3.3.3 don't allow multiple calls
                            if (wasCalled) return;
                            wasCalled = true;
                            // 2.3.3.3.2
                            this.reject(reasonY);
                        }.bind(this));
                    } else {
                        // 2.3.3.4
                        this.fulfill(x);
                    }
                } catch(e) {
                    // 2.3.3.3.4.1 ignore if call was made
                    if (wasCalled) return;
                    // 2.3.3.2 or 2.3.3.3.4.2
                    this.reject(e);
                }
            } else {
                // 2.3.4
                this.fulfill(x);
            }
        }
    }
    constructor(executor) {
        // A Promise is just a wrapper around a Deferred, exposing only the `then`
        // method, while `resolve` and `reject` are available in the constructor callback
        var df = new CustomPromise.Deferred();
        // Provide access to the `resolve` and `reject` methods via the callback
        executor(df.resolve.bind(df), df.reject.bind(df));
        return df.promise;
    }
}

new CustomPromise((resolve) => {
    resolve(1);
  })
  .then((value) => {
    console.log('first value', value);
    return 2;
  })
  .then((value) => {
    console.log('second value', value);
    return null;
  });

Issue while Implementing Promise class with JavaScript

1 Answers