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

Improve the Omit helper type #33524

Closed
wants to merge 1 commit into from
Closed

Improve the Omit helper type #33524

wants to merge 1 commit into from

Conversation

vipcxj
Copy link

@vipcxj vipcxj commented Sep 20, 2019

Fixes #31153
Since Omit is the offical helper type now, I think it should be a perfect version. The common version not work with type like { a: string, b: number, [key: string]: any, }, this version works. The detail see #31153

@vipcxj
Copy link
Author

vipcxj commented Sep 20, 2019

A few test failed. But I think it is the result of other bug.

function test<T extends { a: string, b:string }>(obj: T): T {
  let { a, ...rest } = obj;
  const test = rest as Omit<T, 'a'>;
  test.b // it's ok;
  const test2 = { a, ...test };
  console.log(test2.a) // it's ok;
  console.log(test2.b) // it's ok;
  test2.a = '1' // it's ok;
  test2.a = 1 // it's not ok, test2.a must be string;
  test2.b = '1' // it's ok
  test2.b = 1 // it's not ok, test2.b must be string;
  test2.c = 1 // it's not ok, c does not exist on type `type of test2`;
  // so it seems that the type of test2 should be { a: string, b:string }
  const test3 = test2 as T // not ok, TS2352, why? Is there a tool to see what's the real type of { a: string } & Omit<T, 'a'>
}

@dragomirtitian
Copy link
Contributor

The PR template mentions that you can submit fixes for issues in the backlog I believe (the issue you are referencing is a closed question)

Also I don't think adding this more complicated Omit is necessarily a good idea, since as I mention in the comments to 31153 this may need to be changed once we have generalized index signatures form #26797.

@vipcxj
Copy link
Author

vipcxj commented Sep 20, 2019

@dragomirtitian This is not a problem if the generalized index signatures form is properly restricted. I think the index signatures without restricted will be a disaster.

@RyanCavanaugh
Copy link
Member

Touching Omit in any fashion is going to be a massive breaking change. It is also intentional that Omit does not constrain its second argument.

Please only submit PRs for issues in the Backlog milestone.

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 this pull request may close these issues.

Exclude not work on [key: string]:any
3 participants