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

readonly and mutable types are incorrectly considered equal #3

Closed
DetachHead opened this issue Feb 12, 2023 · 7 comments
Closed

readonly and mutable types are incorrectly considered equal #3

DetachHead opened this issue Feb 12, 2023 · 7 comments

Comments

@DetachHead
Copy link

import { test } from 'ts-spec'

interface Foo {
    readonly a: number
}

declare const foo: { a: number }

test('' as const, (t) => t.not.equal(foo)<Foo>()) // error

playground

geoffreytools added a commit that referenced this issue Feb 12, 2023
@geoffreytools geoffreytools added the help wanted Extra attention is needed label Feb 12, 2023
@geoffreytools
Copy link
Owner

This one is going to be challenging because to my knowledge Foo and typeof foo are literally the same type, readonly only being a key modifier in this context.

If I tried to infer readonly keys from a mixed object type for example, I would get all the keys regardless of whether they are readonly or not.

I'm keeping this issue open for now, because I could be wrong, but I am not sure I can do something about it.

@DetachHead
Copy link
Author

this method seems to work:

export type Equals<X, Y> =
    (<T>() => T extends X ? 1 : 2) extends
    (<T>() => T extends Y ? 1 : 2) ? true : false;

type A = Equals<{readonly a: number},{a: number}> // false
type B = Equals<{readonly a: number},{readonly a: number}> // true

though that has its own drawbacks, many of which are mentioned in microsoft/TypeScript#27024

@geoffreytools
Copy link
Owner

geoffreytools commented Feb 18, 2023

Very nice. I leveraged this implementation to infer readonly keys and to add them as an additional prop to the object. The change only affects equal for now, but I will make sure that extends and includes are also affected.

@geoffreytools geoffreytools removed the help wanted Extra attention is needed label Feb 18, 2023
@mscharley
Copy link

I haven't distilled this to a minimal reproduction yet, but I think this may have had some unintended consequences. I have two object types which don't use the readonly keyword but they aren't considered equal even though if you compare each key in the objects individually they all are considered equal.

@mscharley
Copy link

Ok, so that was easier than I expected... This used to pass, but now fails on the latest version of ts-spec:

import { test } from "ts-spec";

type Foo = { helloWorld: string };
interface Bar {
  helloWorld: string;
}

test("Foo == Bar" as const, (t) => t.equal<Foo, Bar>());

@geoffreytools
Copy link
Owner

Thanks for the quick feedback. It should now work as expected.

@geoffreytools
Copy link
Owner

By the way, as const is no longer required when writing test descriptions

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

No branches or pull requests

3 participants