workerpool icon indicating copy to clipboard operation
workerpool copied to clipboard

Add promises-aplus tests for the promise library

Open forivall opened this issue 5 years ago • 3 comments

I was curious if the promise implementation you have is Promises/A+ conformant, and unfortunately, it's not, since it appears to be more of a jquery promise-compatible, which does some things synchronously that should be async.

The results summary:

266 passing (2m) 592 failing

forivall avatar Jun 10 '20 02:06 forivall

Thanks for your inputs Emily. The reason we couldn't use a standard Promise library (or nowadays the Promise implementation built in JS itself) is that we need support for cancellation. So I think it will not be possible to get the workerpool version of Promise standards compliant, though we could set some steps in that direction I guess.

What we could do is rename the (internal) Promise class to CancellablePromise or something like that to make it more clear that it's not a standard Promise implementation.

What do you think?

josdejong avatar Jun 10 '20 07:06 josdejong

Yup, i totally understand the need for cancellation.

Renaming makes sense, and another option would be to extend the native promise type, such as in https://github.com/sindresorhus/p-cancelable . Although, I also don't know if you need the tight control over synchronicity from your custom promise implementation.

forivall avatar Jun 10 '20 17:06 forivall

It would be nice to see if we can replace Promise implementation with an external library like p-cancelable. I'm not sure if there would be incompatibilities and if it's possible to achieve or not, but it may be worth an experiment. Anyone interested in trying this out?

josdejong avatar Jun 13 '20 08:06 josdejong