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

[bug] Type of nested properties are not fully resolved and fail equality test. #29

Closed
gomain opened this issue Apr 18, 2023 · 2 comments · Fixed by #21
Closed

[bug] Type of nested properties are not fully resolved and fail equality test. #29

gomain opened this issue Apr 18, 2023 · 2 comments · Fixed by #21
Labels
bug Something isn't working

Comments

@gomain
Copy link

gomain commented Apr 18, 2023

{
  type ObjectWithProp<PROP> = {
    prop: PROP;
  };
  type MakeNestObject<NEST_OBJECT extends { nestProp: ObjectWithProp<any> }> = {
    nestObj: NEST_OBJECT;
  };

  /* as expected */
  expectTypeOf<
    MakeNestObject<{ nestProp: ObjectWithProp<"one"> }>
  >().toEqualTypeOf<{ nestObj: { nestProp: { prop: "one" } } }>();
  expectTypeOf<
    MakeNestObject<{ nestProp: ObjectWithProp<"two"> }>
  >().toEqualTypeOf<{ nestObj: { nestProp: { prop: "two" } } }>();

  /* but */
  expectTypeOf<
    MakeNestObject<{ nestProp: ObjectWithProp<"one"> }>
  >().toEqualTypeOf<MakeNestObject<{ nestProp: ObjectWithProp<"two"> }>>();

  /* assignability test, the old fashion way */
  let o1: MakeNestObject<{ nestProp: ObjectWithProp<"one"> }> = null as any;
  let o2: MakeNestObject<{ nestProp: ObjectWithProp<"two"> }> = null as any;
  // @ts-expect-error - type "two" is not assignable to type "one"
  o1 = o2;
  // @ts-expect-error - type "one" is not assignable to type "two"
  o2 = o1;

  /* if the nested object type were by different name */
  type AnotherName<PROP> = {
    prop: PROP;
  }
  expectTypeOf<
    MakeNestObject<{ nestProp: ObjectWithProp<"one"> }>
  >().toEqualTypeOf<MakeNestObject<{ nestProp: AnotherName<"one"> }>>();
  expectTypeOf<
    MakeNestObject<{ nestProp: ObjectWithProp<"one"> }>
  >().not.toEqualTypeOf<MakeNestObject<{ nestProp: AnotherName<"two"> }>>();
}

Playground

@gomain
Copy link
Author

gomain commented May 8, 2023

This may be a related use case:

  expectTypeOf<number[]>().toEqualTypeOf<any[]>(); // passes
  expectTypeOf<any[]>().toEqualTypeOf<number[]>(); // passes

@mmkal mmkal added the bug Something isn't working label May 10, 2023
@mmkal
Copy link
Owner

mmkal commented May 10, 2023

@gomain yes, this is a bug - those definitely shouldn't pass.

mmkal added a commit that referenced this issue May 10, 2023
@mmkal mmkal closed this as completed in #21 May 10, 2023
mmkal added a commit that referenced this issue May 10, 2023
…xclusively (#21)

Fixes #29
Fixes #26
Fixes #5

> Note: I extracted a very small part of this PR to
#20

This is a breaking change as I opted to remove the types that were no
longer needed. They are exported though so it's likely some people
depend on them. I can add these back as desired.

This took a lot of tinkering. This topic and this equality check is
discussed extensively at
microsoft/TypeScript#27024

The main three edge-cases this implementation worked around are:
1. Explicitly handling `any` separately
2. Supporting identity unions
3. Supporting identity intersections

The only remaining known issue with this implementation is:

```ts
  // @ts-expect-error This is the bug.
  expectTypeOf<{foo: number} & {bar: string}>().toEqualTypeOf<{foo: number; bar: string}>()
```

@shicks and I could not find a tweak to the `Equality` check to make
this work.

Instead, I added a workaround in the shape of a new `.simplified`
modifier that works similar to `.not`:

```ts
  // The workaround is the new optional .simplified modifier.
  expectTypeOf<{foo: number} & {bar: string}>().simplified.toEqualTypeOf<{foo: number; bar: string}>()
```

I'm not entirely sure what to do with documenting `.simplified` because
it's something you should never use unless you need it. The simplify
operation tends to lose information about the types being tested (e.g.,
functions become `{}` and classes lose their constructors). I'll
definitely update this PR to reference the `.simplified` modifier but I
wanted to get a review on this approach first. One option would be to
keep around all the `DeepBrand` stuff and to have `.deepBranded` or
something being the modifier instead. That would have the benefit of
preserving all the exported types making this less of a breaking change.

---------

Co-authored-by: Misha Kaletsky <15040698+mmkal@users.noreply.github.com>
Co-authored-by: Misha Kaletsky <mmkal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants