Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(chai): add containSubset #71972

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
+29 −3
Open

feat(chai): add containSubset #71972

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c71a520
feat(chai): add containSubset
perrin4869 Feb 18, 2025
f1a3da3
fix
perrin4869 Feb 18, 2025
487ed38
attempt fix
perrin4869 Feb 23, 2025
dab1a3f
fix
perrin4869 Feb 23, 2025
629f43f
peer dependency
perrin4869 Feb 25, 2025
fc60a07
Revert "peer dependency"
perrin4869 Feb 25, 2025
484f2a4
peer dependency again
perrin4869 Feb 25, 2025
6263efb
Merge remote-tracking branch 'upstream/master' into feat/chai/add-con…
perrin4869 Feb 25, 2025
95cffdf
rerun
perrin4869 Feb 26, 2025
cefad68
revert
perrin4869 Feb 26, 2025
d8d1cf2
not sure what broke
perrin4869 Feb 26, 2025
1a4869b
try again?
perrin4869 Mar 4, 2025
c042421
revert typing to any
perrin4869 Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions types/chai-as-promised/chai-as-promised-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ thenableNum = chai.expect(thenableNum).to.eventually.have.ownProperty("foo");
thenableNum = chai.expect(thenableNum).to.eventually.have.ownProperty(Symbol.for("bar"));
thenableNum = chai.expect(thenableNum).to.eventually.have.ownPropertyDescriptor("foo");
thenableNum = chai.expect(thenableNum).to.eventually.have.ownPropertyDescriptor(Symbol.for("bar"));
thenableNum = chai.expect(thenableNum).to.eventually.containSubset({});
thenableNum = chai.expect(thenableNum).to.become(3);
thenableNum = chai.expect(thenableNum).to.be.fulfilled;
thenableNum = chai.expect(thenableNum).to.be.rejected;
Expand All @@ -33,6 +34,7 @@ thenableNum = thenableNum.should.be.fulfilled;
thenableNum = thenableNum.should.eventually.deep.equal(3);
thenableNum = thenableNum.should.eventually.become(3);
thenableNum = thenableNum.should.become(3);
thenableNum = thenableNum.should.eventually.containSubset(3);
thenableNum = thenableNum.should.be.rejected;
thenableNum = thenableNum.should.be.rejectedWith(Error);
thenableNum = thenableNum.should.be.rejectedWith("Error");
Expand Down
5 changes: 5 additions & 0 deletions types/chai-as-promised/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ declare global {
Arguments: PromisedAssertion;
equal: PromisedEqual;
equals: PromisedEqual;
containSubset: PromisedContainSubset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work? I don't see this in their repo: https://github.com/search?q=repo%3Achaijs%2Fchai-as-promised%20containSubset&type=code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that works because chai-as-promised will iterate over all the available assertions dynamically and install new assertions based on that. It doesn't have a list of assertions that it maintains manually. This way, it will also work with assertions that come from plugins, for example. I have been using chai-as-promised with chai-subset for years for example

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of the other declarations here for addon packages?

It seems strange to declare that every addon exists, even if they aren't installed, or aren't supported (like for older versions of chai).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

containSubset is not an add-on anymore, it was merged into chai proper, therefore adding it officially here now. What I was saying is that in the past I was using it as an add-on, and in that case I manually added the typings in my individual projects

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, yes, but I'm specifically talking about people using chai<5.2.0 with chai-subset and chai-as-promised.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

those people get the typing for free now?
not sure I get what's the concern...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, misspoke; I meant chai<5.2.0 and chai-as-promise but without chai-subset, so they'd see a declaration which wouldn't work because nothing provides it.

It's probably unavoidable; it'd be nicer if the things tacked onto chai for promises were derived from the rest of chai, but I suspect that wouldn't actually work due to circularities without a major restructuring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well that'd be true of any assertion added to chai in the past or in the future, that's how the typings seem to have been developed?
for what it's worth, this update suggests that it requires chai>=5.2.0 by increasing the version too
one last point to consider is, chai devs have expressed interest in upstreaming chai-as-promised in the future as well

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. To be clear, I think this PR is probably fine, I'm just making sure that we do the right thing for an extremely popular set of packages.

eq: PromisedEqual;
eql: PromisedEqual;
eqls: PromisedEqual;
Expand Down Expand Up @@ -174,6 +175,10 @@ declare global {
(value: any, message?: string): PromisedAssertion;
}

interface PromisedContainSubset {
(value: any): PromisedAssertion;
}

interface PromisedProperty {
(name: string | symbol, value?: any, message?: string): PromisedAssertion;
}
Expand Down
4 changes: 2 additions & 2 deletions types/chai-subset/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
"projects": [
"https://github.com/debitoor/chai-subset"
],
"dependencies": {
"@types/chai": "*"
"peerDependencies": {
"@types/chai": "<5.2.0"
},
"devDependencies": {
"@types/chai-subset": "workspace:."
Expand Down
5 changes: 5 additions & 0 deletions types/chai/chai-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ function equal() {
should.equal(undefined, void (0));
}

function containSubset() {
expect({}).to.containSubset({});
({}).should.containSubset({});
}

function _typeof() {
expect("test").to.be.a("string");
"test".should.be.a("string");
Expand Down
15 changes: 15 additions & 0 deletions types/chai/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ declare global {
eq: Equal;
eql: Equal;
eqls: Equal;
containSubset: ContainSubset;
property: Property;
ownProperty: Property;
haveOwnProperty: Property;
Expand Down Expand Up @@ -329,6 +330,10 @@ declare global {
(value: any, message?: string): Assertion;
}

interface ContainSubset {
(expected: any): Assertion;
}

interface Property {
(name: string | symbol, value: any, message?: string): Assertion;
(name: string | symbol, message?: string): Assertion;
Expand Down Expand Up @@ -523,6 +528,16 @@ declare global {
*/
deepStrictEqual<T>(actual: T, expected: T, message?: string): void;

/**
* Partially matches actual and expected.
*
* T Type of the objects.
* @param actual Actual value.
* @param expected Potential expected value.
* @param message Message to display on error.
*/
containSubset<T>(val: T, exp: Partial<T>, msg?: string): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double checking this, but this seems to only work for objects and a single depth.

The implementation (https://github.com/chaijs/chai/blob/ce4c403d0f60818e1f5e1d4aa472e2a2b0a1ad8a/lib/chai/core/assertions.js#L4076) appears to actually support arrays, and the whole thing is deeply matched. That and Dates are not deeply matched.

This makes me wonder if this needs some sort of DeepPartial or something to capture the full behavior (and chai-subset itself took any instead of specific types).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I was hoping to improve on the original typing but any might be correct here, since there is nothing really stopping anyone from passing an unrelated object?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, no...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified it to use any


/**
* Asserts valueToCheck is strictly greater than (>) valueToBeAbove.
*
Expand Down
2 changes: 1 addition & 1 deletion types/chai/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"private": true,
"name": "@types/chai",
"version": "5.0.9999",
"version": "5.2.9999",
"type": "module",
"projects": [
"http://chaijs.com/"
Expand Down