alkemics/CancelablePromise

Awaiting promise should be resolved immidiately

Closed this issue · 5 comments

Gaunt commented

I think have found inconsistency between ES Promise and Cancellable promise

describe('ESPromise', function() {
    it("should be resolved on first await", async function() {
        let promiseResolved = false;
        (async() => {
            await new Promise((resolve, reject) => {
                this.resolve = resolve;
            });
            promiseResolved = true;
        })();
        this.resolve();
        await (async() => {})();
        assert.isTrue(promiseResolved)
    });
});

describe('CancelablePromise', function() {
    it("should be resolved on first await", async function() {
        let promiseResolved = false;
        (async() => {
            await new CancelablePromise((resolve, reject) => {
                this.resolve = resolve;
            });
            promiseResolved = true;
        })();
        this.resolve();
        await (async() => {})();
        assert.isFalse(promiseResolved)
    });
});`

CancelablePromise is not resolved

@Gaunt Thank you for your feedback. I think there are some internal mysteries here which I can't handle, so IMHO it's not a bug. You can try this by yourself in your browser:

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Promises</title>
</head>
<body>
  <script type="module">
    import { CancelablePromise } from 'https://unpkg.com/cancelable-promise@4.2.1/esm/CancelablePromise.min.mjs';
    import PCancelable from 'https://unpkg.com/p-cancelable@3.0.0/index.js';

    class CPromise extends Promise {}

    async function run(P) {
      console.group(P)
      let resolvePromise = () => {};
      let result = false;
      (async () => {
        const p = new P((resolve) => {
          resolvePromise = resolve;
        });
        console.log('Wait for internal promise');
        await p;
        console.log('Set result');
        result = true;
      })();
      console.log('Resolve promise');
      resolvePromise();
      console.log('Wait for fulfilled promise');
      await Promise.resolve();
      console.log('Result =', result);
      await new Promise((r) => setTimeout(r, 10));
      console.groupEnd();
    }

    async function start() {
      await run(Promise);
      await run(CPromise);
      await run(PCancelable);
      await run(CancelablePromise);
    }

    start();
  </script>
</body>
</html>

Output:

Screenshot 2021-09-20 at 12 26 22

Unless if it's a direct Promise instance, setting the result is always deferred. In the other hand, it's not an issue you can't handle, your example is quite unrealistic, because you are expecting some results as it was synchronous (await (async() => {});: this line is useless, it's the same as await true for example). A more realistic use case could be to wait for something really asynchronous, such as:

describe('CancelablePromise', function() {
    it("should be resolved on first await", async function() {
        let promiseResolved = false;
        (async() => {
            await new CancelablePromise((resolve, reject) => {
                this.resolve = resolve;
            });
            promiseResolved = true;
        })();
        this.resolve();
        await new Promise((r) => setTimeout(r, 1));
        assert(promiseResolved);
    });
});

or to wait for your promise itself

describe('CancelablePromise', function() {
    it("should be resolved on first await", async function() {
        let promiseResolved = false;
        const p = (async() => {
            await new CancelablePromise((resolve, reject) => {
                this.resolve = resolve;
            });
            promiseResolved = true;
        })();
        this.resolve();
        await p;
        assert(promiseResolved);
    });
});
Gaunt commented

Thank you for the answer. Yes I know it is not a big issue, only wanted to know if it is something that can be changed. I tried to use it as a drop-in replacement and encountered this issue.

It is not unrealistic, you can await an async function that contains await only in a condition.

async condAwait(cond) {
     if (cond) {
          await something; 
    }
}

then you expect scheduled calls will execute anyway

Yes indeed, I think you forgot to exec the async method: await (async() => {}) VS await (async() => {})()

Gaunt commented

yes. it was a mistake, but the problem remains.

Yes I saw the problem remains, but as I demonstrated in my examples, it's not specific to this library. My point is: you should not rely on such logic, I think you have better alternatives. If you have a concrete example which does not work as expected, I'll be happy to help. But I unfortunately I don't have the time to deep dive in the Promise spec to understand why this behavior is different between Promise and an extended Promise such as class ExtendedPromise extends Promise {}.