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

Types is incorrect after omitting keys #2134

Closed
nilennoct opened this issue Oct 8, 2021 · 9 comments · Fixed by #3297
Closed

Types is incorrect after omitting keys #2134

nilennoct opened this issue Oct 8, 2021 · 9 comments · Fixed by #3297
Labels
awaiting upstream released This issue/pull request has been released. type:bug Changes fix a minor bug

Comments

@nilennoct
Copy link
Contributor

Description

After omitting some fields from existing Schema types, the types of remained fields become unknown.

Steps to reproduce

const api = new Gitlab({ token: 'token' });
const branch = await api.Branches.show('projectId', 'master');
const commitDiff = await api.Commits.diff('projectId', branch.commit.id);
// TS2345: Argument of type 'unknown' is not assignable to parameter of type 'string'.

Expected behaviour

The type of branch.commit.id should be string.

Actual behaviour

The type of branch.commit.id is unknown.

Possible fixes

All Schema types extend from Record<string, unknown>. For example:

interface Foo extends Record<string, unknown> {
    x: string;
    y: number;
    z: boolean;
}

type K0 = keyof Foo; // string | number
type K1 = Exclude<K0, 'x'>; // string | number
type T = Omit<Foo, 'x'>; // { [x: string]: unknown, [x: number]: unknown }

While the other is not extended from Record type:

interface Bar {
    x: string;
    y: number;
    z: boolean;
}

type K0 = keyof Bar; // 'x' | 'y' | 'z'
type K1 = Exclude<K0, 'x'>; // 'y' | 'z'
type T = Omit<Bar, 'x'>; // { y: number, z: boolean }

Because Omit is implemented by Exclude, so the excluded keys is incorrect if the object type extends from Record type.

@nilennoct nilennoct added the bug label Oct 8, 2021
@jdalrymple
Copy link
Owner

microsoft/TypeScript#36981 Looks like a larger problem

@nilennoct
Copy link
Contributor Author

Seems that this issue has been rescheduled. The related PR resolve the problem by writing Omit with remapping (microsoft/TypeScript#42524). However, key remapping via as was introduced since TypeScript 4.1, users who use an old version of TypeScript will get syntax error.

@WiseBird
Copy link

@jdalrymple Sorry for the ping, but probably it won't be fixed any time soon.

As a workaround, I suggest using a utility type instead of Omit. Exept provided by type-fest works fine it seems.

I prepared an example with BranchSchema:

export interface BranchSchema extends Record<string, unknown> {
    ...
    commit: Omit<CommitSchema, 'web_url' | 'created_at'>;
}

Direct usage of CommitSchema works just fine:

let date: Date | undefined;
let title: string;

const commit: CommitSchema = {} as any;

title = commit.title;       // no errors
date = commit.created_at;   // no errors

However, for BranchSchema it shows an error:

const branch: BranchSchema = {} as any;

title = branch.commit.title;      // Type 'unknown' is not assignable to type 'string'.(2322)
date = branch.commit.created_at;  // Type 'unknown' is not assignable to type 'Date | undefined'.(2322)

In another example I replaced Omit with Except and now only expected error is shown:

export interface BranchSchema extends Record<string, unknown> {
    ...
    commit: Except<CommitSchema, 'web_url' | 'created_at'>;
}

const branch: BranchSchema = {} as any;

title = branch.commit.title;      // no errors
date = branch.commit.created_at;  // Type 'unknown' is not assignable to type 'Date | undefined'.(2322)

@jdalrymple
Copy link
Owner

Good find! I keep this in mind to make the tweak when i have a moment!

@jdalrymple jdalrymple added type:bug Changes fix a minor bug and removed bug labels Mar 16, 2023
@WiseBird
Copy link

WiseBird commented Jun 6, 2023

@jdalrymple Is it fixed? I still see Omit usages in the types.

@jdalrymple
Copy link
Owner

jdalrymple commented Jun 6, 2023

Now that i review this, it looks like i didn't fully solve this previously. I will now though! Should have it back up in an hour or so

@jdalrymple jdalrymple reopened this Jun 6, 2023
@WiseBird
Copy link

WiseBird commented Jun 6, 2023

I see that you are in Montreal, I should definitely buy you a beer someday

@jdalrymple
Copy link
Owner

Gonna try this type: microsoft/TypeScript#54451, looks alot stricter than Omit, tell me if you run into any problems once i deploy!

@jdalrymple
Copy link
Owner

🚀 Issue was released in 38.12.0 🚀

@jdalrymple jdalrymple added the released This issue/pull request has been released. label Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting upstream released This issue/pull request has been released. type:bug Changes fix a minor bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants