chai icon indicating copy to clipboard operation
chai copied to clipboard

Add support for Promise

Open BryanHuntNV opened this issue 1 year ago • 16 comments
trafficstars

It appears that chai-as-promised does not work with chai v5 and is no longer under development. Given the prevalence of programming with Promise, would it make sense to include the functionality of chai-as-promised into chai proper?

BryanHuntNV avatar Jan 05 '24 17:01 BryanHuntNV

that is what i'm leaning towards

think it makes sense to have native support for things like toThrow on a promise, etc

43081j avatar Jan 05 '24 17:01 43081j

An innate problem with adding promises to core is that all existing assertions synchronously throw, and there is no very clear signal as to what would then turn that into an asynchronous throw. chai-as-promised adds the eventually chainable property, which we could add but the await keyword is something that developers are much more likely to be aware of.

One way we could go about this change is to make all assertions return a Promise, but that's a big breaking change (though arguably one that gives us future flexibility for all kinds of async stuff). Another solution could be to extensively document how one would unpack promises within the existing assertions (which typically looks like expect(await ...), but the far bigger problem today exists with how to properly handle rejections which involves some code gymnastics.

keithamus avatar Jan 09 '24 11:01 keithamus

FWIW, my typical use-case for chai-as-promised (using mocha) is:

return expect(testClass.testFunction(args)).to.eventually.be.rejectedWith(Error);

BryanHuntNV avatar Jan 09 '24 15:01 BryanHuntNV

i recall in some other test libraries, some smarts were done to change the return type

e.g.


expect(someSyncThing).to.equal(10);

await expect(someAsyncThing).to.equal(10);

and basically have a conditional return:

function equal(expectation: unknown): T extends PromiseLike<unknown> ? Promise<void> : void;

but that may make my fancy types im doing in the typescript branch a bit more difficult 😬

since i'm not sure we can do a type guard return of a promise

43081j avatar Jan 10 '24 11:01 43081j

I would also vote in favor of adding the eventually, become, rejectedWith, etc APIs found in chai-as-promised, I've used those since forever ago, and it's the one thing preventing me from upgrading to v5

perrin4869 avatar May 02 '24 07:05 perrin4869

As much as I'm all for getting these into chai itself, chai-as-promised works perfectly fine for me at least with chai v5.

mscharley avatar May 02 '24 09:05 mscharley

Hmm, now I look at the original issue, I can't remember why I ended up subscribed to this ticket. I just updated a project to chai v5 which was using chai-as-promised and it seems to be working fine.

// mocha.env.js
const { use } = await import("chai");
use((await import("chai-as-promised")).default);
file: ["mocha.env.js"]

And then just go about using chai-as-promised how you normally would. I just wrote up a few intentionally broken tests to make sure the matchers are working properly and everything seems fine for ESM projects at least.

import { expect } from "chai";

describe("promise tests", () => {
  it("should succeed", async () => {
    await expect(Promise.resolve(10)).to.eventually.eq(10);
  });

  it("should catch rejections", async () => {
    await expect(Promise.reject(new Error("Oh no!"))).to.eventually.be.rejectedWith("Oh no!");
  });

  it("should fail", async () => {
    await expect(Promise.resolve(10)).to.eventually.eq(20);
  });

  it("should fail to catch rejections", async () => {
    await expect(Promise.reject(new Error("Oh no!"))).to.eventually.be.rejectedWith("Oops!");
  });
});
  promise tests
    ✔ should succeed
    ✔ should catch rejections
    1) should fail
    2) should fail to catch rejections

mscharley avatar May 02 '24 09:05 mscharley

yeah, I expect the code is compatible, and if you force it, npm will allow you to run chai-as-promised with chai@5, but it's not allowed as per chai-as-promised with the peer dependencies it has set.

One way we could go about this change is to make all assertions return a Promise, but that's a big breaking change (though arguably one that gives us future flexibility for all kinds of async stuff).

I don't like this idea at all to be honest 😅 But having built in constructs to run assertions on promises seems like a good candidate to be built in to be honest, considering that at this point, Promises are the base of all async JS APIs.

perrin4869 avatar May 02 '24 09:05 perrin4869

yeah, I expect the code is compatible, and if you force it, npm will allow you to run chai-as-promised with chai@5, but it's not allowed as per chai-as-promised with the peer dependencies it has set.

Yeah, I did notice that after I posted but didn't know what to make of it. npm definitely didn't complain at all for me. npm peer dependencies working as intended yet again, I guess. I'd definitely still prefer to see it in chai directly, but maybe a short-term solution is just republishing the original package with an updated peer dependency so that people aren't blocked while a longer term solution is worked out.

mscharley avatar May 02 '24 10:05 mscharley

so it sounds like chai-as-promised just needs the peer dependency semver changing to include 5?

im not sure how active @domenic is anymore in that repo, so we might struggle to update it, but could be worth opening a PR

ultimately, i think this issue still stands as we should really natively support promises in chai IMO

43081j avatar May 03 '24 19:05 43081j

I'd be happy to transfer the chai-as-promised repo to the chaijs GitHub organization and the same for the npm package.

domenic avatar May 04 '24 14:05 domenic

Thanks @domenic we’d be happy to take it! I’ve invited you to the GitHub org (I actually thought you were already a member) so you can initiate the transfer.

keithamus avatar May 04 '24 17:05 keithamus

Thanks @domenic for transferring the repo! If you have time could you please also go to https://www.npmjs.com/package/chai-as-promised/access and invite the user https://www.npmjs.com/~chaijs to be able to publish packages. This account is the account we use for publishing all chai packages. Thanks!

keithamus avatar May 09 '24 08:05 keithamus

Sorry for the (mostly) unrelated comment, but I noticed that @domenic is also the maintainer of sinon-chai, which I'm also using. Would that plugin also be welcome in this org? I could also help maintain it if there is not enough throughput

perrin4869 avatar May 09 '24 09:05 perrin4869

Happy to house any and all chai plugins under the org, existing maintainers remain as such.

keithamus avatar May 09 '24 09:05 keithamus

Transferred and access given for both chai-as-promised and sinon-chai.

domenic avatar May 09 '24 12:05 domenic