Not possible to call progress without setTimeout

[pke]

This code will crash:

new ProgressPromise((resolve, reject, progress) => {
  progress()
})

because at this time, the construction of the object has not been completed. One can call resolve and reject that way though. Any idea how to make this possible for progress too?

[numtel]

Interesting but it doesn't seem like there's any use case for this.

I've had ChatGPT rewrite the class as using a prototype so that the listeners property can be accessed before invoking the executor function but since the progress happens before a progress listener can be added, you'll never see that progress message.

See the test case I created on this branch. It never logs the value.

https://github.com/numtel/progress-promise/commit/971d1b8e2c6dafcdecb660b95a34f0f69cb5c466

[pke]

I found a solution already. There is a use case for this. Lets say the executor wants to display an initial progress message before it starts any (async) work. This message would be lost then. But it is display in my fix:

test("progress is called", async () => {
  const promise = new PromiseWithProgress<void, string>((resolve, _reject, progress) => {
    progress("Running...")
    setTimeout(() => {
      progress("Done!")
      resolve()
    }, 0)
  })
  const progressHandler1 = jest.fn()
  const progressHandler2 = jest.fn()
  await promise.progress(progressHandler1).progress(progressHandler2)
  expect(progressHandler1).toHaveBeenNthCalledWith(1, "Running...")
  expect(progressHandler1).toHaveBeenNthCalledWith(2, "Done!")
  expect(progressHandler2).toHaveBeenNthCalledWith(1, "Running...")
  expect(progressHandler2).toHaveBeenNthCalledWith(2, "Done!")
})

I cue the pending progress, that happened during ctor time for all later attached progress handlers. So none of them misses the initial progress values.

[numtel]

Queuing events before adding the handler fixes it, yes, but why not just do that in the function where you create the ProgressPromise in the first place? Seems like adding more complexity than necessary to this class.

[pke]

How do you mean @numtel? btw: very cool that you so quickly reacted to this year old repo when a new issue popped up! 👍

[numtel]

Like, if it's happening synchronously, just move it outside the promise?

console.log('Running...');
await (new ProgressPromise((resolve, reject, progress) => {
  setTimeout(() => {
    progress('Done');
    resolve();
  });
})).progress(val => console.log(val));

You caught me while working on something less fun. This subclass is still one of the favorite NPM packages I've made. It's so simple. I was always hoping to see other promise subclasses that extend it with other features.

[pke]

but my progress handlers are not just simple console logs. They do UI updates and maybe screen transitions. My main issue with this, that it moves business logic (what is the first progress update) outside the business function.

Imagine:

function cleanSomeCacheFiles() {
  return new ProgressPromise(resolve, reject, progress) {
    const numFiles = 5
    progress(`Cleaning ${numFiles} cached filed...`)
  })
}

While your code is KISS, which I love, it has this potential crash bug, that you can not call progress while the promise is still constructed. I see your prototype code does not have the same problem, but its also not simple anymore ;)

I was always hoping to see other promise subclasses that extend it with other features.

In fact I was in process to convert the WinJS Promise with its progress feature to TypeScript when I discovered/remembered your repo. I made the implementation type safe now and I am adding AbortController support for the executor to check whether the promise has been requested to be cancelled.

[numtel]

I'm just trying to understand it. It does seem like it could be a worthy upgrade. Make a PR and we can do an update even if it does make the code less simple.

Using a prototype instead of class...extends doesn't change that it's showing how to subclass a built-in object, in contrast to code that modifies built-in objects, preserving the spirit of the code.

[pke]

my code would be typescript though and using jest for tests

[numtel]

Fine with me, just include the compiled js in there too so that it can still be used with the github url import dependency in package.json.

[pke]

Great, thanks for being open to extend this package. Never set up a TS lib, this will take some time.

Good luck with "the less fun" stuff now!

Kudos for replying like instantly!

[pke]

@numtel is there a reason for using the this[Symbol] approach over this.progressListeners?

[numtel]

At the time, Symbols and built-in promises were new things so I thought it would be cool to use them. Using the symbol creates a private property on the object since that symbol is not accessible outside this source file's closure. If you want to modify the listeners after they're added, maybe follow the standard removeEventListener pattern instead of exposing the array.

Never set up a TS lib

I bet ChatGPT can get 90% of what you want.

[pke]

Thanks for the explaination. In my TS implementation the listeners are private and not exposed.

Maybe I give ChatGPT a test.

[pke]

Thats how cancelling looks like:

test("rejects when cancelled", async () => {
  const abortController = new AbortController()

  const promise = new PromiseEx((resolve, reject, _progress, signal) => {
    setTimeout(() => {
      if (signal?.aborted) {
        reject(new Error("Cancel"))
      }
      resolve()
    }, 0)
  }, abortController.signal)

  abortController.abort()

  await expect(promise).rejects.toThrow("Cancel")
})