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

Feature request: (maybe) rename .toMatchTypeOf #10

Open
papb opened this issue Jan 16, 2021 · 8 comments · May be fixed by #126
Open

Feature request: (maybe) rename .toMatchTypeOf #10

papb opened this issue Jan 16, 2021 · 8 comments · May be fixed by #126
Labels
New feature New feature or request

Comments

@papb
Copy link
Collaborator

papb commented Jan 16, 2021

Hi @mmkal! In mmkal/ts#30 I asked for clarification on .toMatchTypeOf, because its meaning wasn't clear to me from the name alone. You've updated the documentation and it is clear after reading it, but now that some time passed I had to check again because I again wasn't sure.

Original request

I'd like to request the addition of .toBeSubtypeOf as an alias to .toMatchTypeOf, and also request the addition of .toBeSupertypeOf while you're at it. What do you think?

@mmkal mmkal transferred this issue from mmkal/ts Sep 25, 2022
@mmkal mmkal changed the title Feature request: .toBeSubtypeOf and .toBeSupertypeOf Feature request: (maybe) rename .toMatchTypeOf Oct 24, 2022
@mmkal
Copy link
Owner

mmkal commented Oct 24, 2022

I'd like to separate out the idea of .toBeSuperTypeOf into another issue, since I'm not sure I think the added cognitive overhead is worth it. If you want to create a new issue with some use cases, feel free though.

Options as I see them:

(Also @papb I've invited you as a collaborator, which (maybe) will allow you to edit this comment, so if you think of another option, you can add here)

  1. expectTypeOf<Apple>().toMatchTypeOf<Fruit>():
    1. Pro: Incumbent
    2. Pro: Similar to jest expect({a: 1, b: 2}).toMatchObject({a: 1})
    3. Con: Unclear what it means for non-object types
    4. Con: Unclear generally what "match" means in a type context?
    5. Con: Kinda ambiguous even when you've really thought about it a lot
  2. expectTypeOf<Apple>().toBeSubTypeOf<Fruit>():
    1. Pro: Pretty clear if you're familiar with types
    2. Pro: Also clear for non-object types? expectTypeOf<'apple'>().toBeSubTypeOf<'apple' | 'banana'>()
    3. Cons: None?
  3. expectTypeOf<Apple>().toBeAssignableToTypeOf<Fruit>() (suggested in Update Extends to not distribute over union types #12)
    1. Pro: Unambiguous - corresponds directly to the real typescript feature
    2. Con: [IMO] not all that intuitive. Nobody's assigning anything, really, so it forces the user to imagine a hypothetical scenario

I think 2 probably is the best, aside from toMatchTypeOf already existing and being in use quite a lot. Maybe it could be kept around as an alias though, although I also don't love doing that.

@trevorade
Copy link
Collaborator

Yeah, I also like 2. above. Assignable is definitely not a good word to use because, as far as I'm aware, expect-type does not actually check for assignability anywhere and it's impossible for it to check for "not assignable"

One example I ran into recently with type-fest was this test using tsd (see PR):

expectNotAssignable<{[key: `#${string}`]: string}>({'missingLeading#': 'oops'});

In expect-type, the following does not build (though it presumably would were it truly toBeAssignableToTypeOf):

expectTypeOf({'missingLeading#': 'oops'}).not.toMatchTypeOf<{[key: `#${string}`]: string}>();

And the following does:

expectTypeOf({'missingLeading#': 'oops'}).toMatchTypeOf<{[key: `#${string}`]: string}>();

You can verify proper TS assignability in vanilla TS with something like:

function expectAssignable<Expected = never, Actual extends Expected = Expected>(arg: Actual) {}

(The extra generic param prevents arg from inferring Expected thus requiring that it is explicitly specified)

Used like:

expectAssignable<{[key: `#${string}`]: string}>({'#withLeading': 'yay'});

As far as I'm aware, you can't write vanilla TS for expectNotAssignable. Best I can do is expectAssignable + @ts-expect-error but that doesn't really scope what specifically is failing.

TS Playground with examples

@papb
Copy link
Collaborator Author

papb commented Oct 25, 2022

I was very happy to see the invite, @mmkal, thank you very much 😁

so if you think of another option, you can add here

None comes to mind 😅

I also prefer option 2.

Maybe it could be kept around as an alias though, although I also don't love doing that.

I also think it can be kept around as an alias, but with @deprecated.

@papb
Copy link
Collaborator Author

papb commented Oct 25, 2022

though it presumably would were it truly toBeAssignableToTypeOf

Why? You mean the fact that excess properties are allowed in object assignments in some cases?

To be honest I don't remember well what are the exact situations in which excess properties are allowed. But I noticed that your vanilla expectAssignable also does not allow it (directly from your TS Playground):

// @ts-expect-error
expectAssignable<{[key: `#${string}`]: string}>({'trailing#': 'oops'});

The PR you linked shows that even tsd's expectNotAssignable does not allow excess properties.

So maybe you're talking about something else, not excess properties? Sorry 😅

@trevorade
Copy link
Collaborator

trevorade commented Oct 25, 2022

Re: extends vs assignability. See this playground.

Basically extends from a type expression is not the same thing as assignability. Sometimes they are but sometimes they aren't.

I'm wondering if you could mimic it with a DeepBrand on each but I'm unsure.

@papb
Copy link
Collaborator Author

papb commented Oct 25, 2022

Also, by the way, maybe the new satisfies keyword from TS 4.9 can help us?

@mmkal
Copy link
Owner

mmkal commented Oct 3, 2023

I'm not against doing this still, but I think I will release a v1 with the API pretty much as it is since it has a decent amount of usage already, so it shouldn't really be on v0 anymore.

This can go into a v2 someday.

@mmkal
Copy link
Owner

mmkal commented Oct 9, 2024

Update for anyone interested - I am now leaning towards deprecating .toMatchTypeOf, and replacing it in the suggested usage documentation with .toMatchObjectType (which is as strict as .toEqualTypeOf but picks the keys from the expected object only - this is the most common way I see .toMatchTypeOf being used right now), or where you really want to check if A extends B, that would live in a better-name .toExtend.

PR: #126. Example:

type MyType = {readonly a: string; b: number; c: {some: {very: {complex: 'type'}}}; d?: boolean}

expectTypeOf<MyType>().toMatchObjectType<{a: string; b: number}>() // fails - forgot readonly
expectTypeOf<MyType>().toMatchObjectType<{readonly a: string; b?: number}>() // fails - b shouldn't be optional
expectTypeOf<MyType>().toMatchObjectType<{readonly a: string; d: boolean}>() // fails - d should be optional

expectTypeOf<MyType>().toMatchObjectType<{readonly a: string; b: number}>() // passes
expectTypeOf<MyType>().toMatchObjectType<{readonly a: string; d?: boolean}>() // passes

This still isn't .toBeAssignableTo, or .toSatisfy, but I think it's ok. There's still value in the new methods, and both assignability and satisfies are concepts relating to values, rather than types. I'm open to hearing use cases or implementation ideas for them though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants