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.

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

Support Actual extends Expected in conjunction with Expected extends Actual #2964

Closed
RebeccaStevens opened this issue Feb 9, 2022 · 4 comments · Fixed by #2969
Closed

Support Actual extends Expected in conjunction with Expected extends Actual #2964

RebeccaStevens opened this issue Feb 9, 2022 · 4 comments · Fixed by #2969

Comments

@RebeccaStevens
Copy link
Contributor

RebeccaStevens commented Feb 9, 2022

The Problem

The type DeepEqualAssertion requires that the Expected type extends the Actual type.
However, sometimes we want this relationship to be the other way around.

Consider the following example:

test("my test", (t) => {
  const expected = { foo: [1, 2, 3] } as const;
  const actual = functionToTest(1, 2, 3);

  t.deepEqual(actual, expected);
});

If the type of actual is inferred as { foo: number[] } then this does not extend the type of expected which is { readonly foo: readonly number[] }. Thus TypeScript reports an error.

Suggested Fix

Add a overload to DeepEqualAssertion to handle this case:

export interface DeepEqualAssertion {
	/**
	 * Assert that `actual` is [deeply equal](https://github.com/concordancejs/concordance#comparison-details) to
	 * `expected`, returning a boolean indicating whether the assertion passed.
	 */
	<Actual, Expected extends Actual>(actual: Actual, expected: Expected, message?: string): actual is Expected;

	/**
	 * Assert that `actual` is [deeply equal](https://github.com/concordancejs/concordance#comparison-details) to
	 * `expected`, returning a boolean indicating whether the assertion passed.
	 */
	<Actual extends Expected, Expected>(actual: Actual, expected: Expected, message?: string): expected is Actual;

	/** Skip this assertion. */
	skip(actual: any, expected: any, message?: string): void;
}
@novemberborn
Copy link
Member

I think this would be great. Would there be any compatibility issues with existing code?

@RebeccaStevens
Copy link
Contributor Author

I don't believe this change would introduce any compatibility issues.

@novemberborn
Copy link
Member

@RebeccaStevens do you want to submit a PR for this?

RebeccaStevens added a commit to RebeccaStevens/ava that referenced this issue Feb 14, 2022
RebeccaStevens added a commit to RebeccaStevens/ava that referenced this issue Feb 14, 2022
@RebeccaStevens
Copy link
Contributor Author

@novemberborn #2969

RebeccaStevens added a commit to RebeccaStevens/ava that referenced this issue Feb 22, 2022
RebeccaStevens added a commit to RebeccaStevens/ava that referenced this issue Feb 22, 2022
RebeccaStevens added a commit to RebeccaStevens/ava that referenced this issue Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants