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

Use Partial types for Object.assign #13611

Closed
wants to merge 1 commit into from

Conversation

falsandtru
Copy link
Contributor

Fixes #11100

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Jan 21, 2017

@falsandtru
Copy link
Contributor Author

Yep, reverted my deletions. These problems (by my deletions) will be resolved by rest type. However as I said in #11100, I think partial type is needed too. So this pr can merge without waiting to merge #10727.

@sandersn
Copy link
Member

The partial type is not right for assign for a couple of reasons.

  1. It's not a useful inference source. So function f<T>(pt: Partial<T>): T is essentially equivalent to function f<T>(t: T): T unless the type argument is given explicitly.
  2. It's too strict; see @HerringtonDarkholme's example.

Also, the variadic overload should come last, right? Otherwise it will preclude matching other overloads.

I can see three options:

  1. Do nothing. Variadic Object.assign continues to be illegal in Typescript, although (hard-coded) variadic spread is now legal.
  2. Punt. Make variadic assign take and return any.
  3. Be strict. Use T as the type parameter instead of Partial<T>.

@falsandtru
Copy link
Contributor Author

I see, thanks for your explanation!

@falsandtru falsandtru closed this Jan 23, 2017
@falsandtru falsandtru deleted the lib/Object.assign branch January 23, 2017 22:57
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants