Spread & rest over intersection and unions #12552
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nd distribute spread&rest over unions
mhegazy
changed the title
sandersn
reviewed
Nov 29, 2016
| @@ -0,0 +1,38 @@ | |||
| === tests/cases/compiler/restIntersectionOrIntersection.ts === | |||
Member
There was a problem hiding this comment.
should be named restIntersectionOrUnion.ts, right?
sandersn
requested changes
Nov 29, 2016
Member
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
You should copy some fourslash tests from the original spread PR, pre-object-only. In particular, rename was tricky, as I recall.
sandersn
approved these changes
Nov 29, 2016
Member
sandersn
left a comment
sandersn
left a comment
There was a problem hiding this comment.
Couple of tiny updates, otherwise looks good.
src/compiler/checker.ts
Outdated
| return getUnionType(map(types, t => getSpreadType(left, t, isFromObjectLiteral))); | ||
| } | ||
| else { | ||
| right = emptyObjectType |
| !!! error TS2300: Duplicate identifier 'b'. | ||
| let duplicatedSpread = { ...o, ...o } | ||
|
|
||
| // null, undefined and primitives are not allowed |
Contributor
Author
|
@ahejlsberg can you take a look |
vladima
reviewed
Nov 30, 2016
src/compiler/checker.ts
Outdated
| return anyType; | ||
| } | ||
|
|
||
| if (left.flags & TypeFlags.Union) { |
Contributor
There was a problem hiding this comment.
nit: these two parts of code for left and right looks quite similar, can they be extracted to a function?
vladima
reviewed
Nov 30, 2016
src/compiler/checker.ts
Outdated
| } | ||
|
|
||
| if (left.flags & TypeFlags.Union) { | ||
| const types = filter((<UnionType>left).types, t => !(t.flags & TypeFlags.Nullable)); |
Contributor
There was a problem hiding this comment.
this pattern filter out nullable types is repeated at least 3 times
vladima
approved these changes
Nov 30, 2016
mhegazy
added a commit
to microsoft/tslib
that referenced
this pull request
Nov 30, 2016
ahejlsberg
reviewed
Nov 30, 2016
src/compiler/checker.ts
Outdated
| return anyType; | ||
| } | ||
|
|
||
| if (left.flags & TypeFlags.Union) { |
Member
There was a problem hiding this comment.
You can replace all of these new lines with just this:
left = filterType(left, t => !(t.flags & TypeFlags.Nullable));
if (left.flags & TypeFlags.Never) {
return right;
}
right = filterType(right, t => !(t.flags & TypeFlags.Nullable));
if (right.flags & TypeFlags.Never) {
return left;
}
if (left.flags & TypeFlags.Union) {
return mapType(left, t => getSpreadType(t, right, isFromObjectLiteral));
}
if (right.flags & TypeFlags.Union) {
return mapType(right, t => getSpreadType(left, t, isFromObjectLiteral));
}
ahejlsberg
reviewed
Nov 30, 2016
| } | ||
| } | ||
|
|
||
| function isValidSpreadType(type: Type): boolean { |
Member
There was a problem hiding this comment.
The functional style is a lot shorter:
function isValidSpreadType(type: Type): boolean {
return !!(type.flags & (TypeFlags.Any | TypeFlags.Null | TypeFlags.Undefined) ||
type.flags & TypeFlags.Object && !isGenericMappedType(type) ||
type.flags & TypeFlags.UnionOrIntersection && !forEach((<UnionOrIntersectionType>type).types, t => !isValidSpreadType(t)));
}
ahejlsberg
reviewed
Nov 30, 2016
src/compiler/checker.ts
Outdated
| return symbol ? getLocalTypeParametersOfClassOrInterfaceOrTypeAlias(symbol) : undefined; | ||
| } | ||
|
|
||
| function filterNulableTypes(union: UnionType): Type[] { |
Member
There was a problem hiding this comment.
Spelling. Not sure you need to keep this function, but if you do, have it operate on any kind of type in the same way as mapType and filterType.
Jessidhia
reviewed
Nov 30, 2016
src/compiler/checker.ts
Outdated
|
|
||
| function isValidSpreadType(type: Type): boolean { | ||
| if (type.flags & (TypeFlags.Any | TypeFlags.Null | TypeFlags.Undefined)) { | ||
| return true; |
mhegazy
added a commit
to microsoft/tslib
that referenced
this pull request
Nov 30, 2016
Port null/undefined support in __rest from microsoft/TypeScript#12552
Member
|
👍 |
Merged
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #12520 , #12534 and #12460
//CC @sandersn