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

StrictExtends is laxer than Extends #37

Closed
JacobLey opened this issue Sep 10, 2023 · 2 comments · Fixed by #16
Closed

StrictExtends is laxer than Extends #37

JacobLey opened this issue Sep 10, 2023 · 2 comments · Fixed by #16

Comments

@JacobLey
Copy link

Hi, I was trying to use the Extends types (see #36) and ran into some unexpected behavior with StrictExtends.

My understanding is strict extension should be a subset of normal extension. That is to say, there should be no cases where a type strict extends another type, but does not traditionally extend it. If that is not the case, I would greatly appreciate some explanation on what these edge cases are.

Example:

type A = (args: string) => 5;
type B = (args: string | number) => number;

type doesExtend = A extends B ? true : false;
// doesExtend = false
// A cannot extend B because the parameters are too strict

expectTypeOf<
    Extends<A, B>
>().toEqualTypeOf<false>(); // Correct / expected


expectTypeOf<
    StrictExtends<A, B>
>().toEqualTypeOf<true>(); // Incorrect / unexpected?

My understanding is that is caused by this line: https://github.com/mmkal/expect-type/blob/main/src/index.ts#L43C11
It spreads the params + return equally.

extends of methods is a bit counter intuitive.

In order for method A to extend B:

  • A's return type must be "stricter" than B's (ReturnType<A> extends ReturnType<B>)
    • This is handled correctly (as far as I can tell)
  • A's parameters must be "laxer" than B's (Parameters<B> extends Parameters<A>)
    • This is not handled correctly
@mmkal
Copy link
Owner

mmkal commented Sep 30, 2023

I agree. The logic is a little fuzzy when it comes to functions - this was initially conceived more as a checker for plain objects and performs best with those, but I would like to cover as many edge cases as possible. It might require some careful thought and design though. I am maintaining this library in bursts, and I am a forgetful person, so would you mind opening up a PR containing a failing test to illustrate this point unambiguously (and more importantly - permanently, since the test can remain in the git history once it's made to pass)?

mmkal added a commit that referenced this issue Oct 1, 2023
re: #37 and #36
@mmkal
Copy link
Owner

mmkal commented Oct 1, 2023

Having said that, it's beyond the scope of this library to provide a carefully-designed utility type library. I'll be adding a note to the docs warning about this in #16 to close this issue. That PR also renames StrictExtends to more accurately reflect what it does. But there may well be a "real" bug that corresponds to this problem which surfaces through the official API (the .toBe... methods). If you find it please feel free to open a new issue framed that way @JacobLey.

@mmkal mmkal mentioned this issue Oct 1, 2023
2 tasks
@mmkal mmkal closed this as completed in #16 Oct 3, 2023
@mmkal mmkal closed this as completed in fd718dd Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants