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

[Master] fix15742 #15789

Merged
merged 6 commits into from
May 12, 2017
Merged

[Master] fix15742 #15789

merged 6 commits into from
May 12, 2017

Conversation

yuit
Copy link
Contributor

@yuit yuit commented May 11, 2017

Fix #15742
Note: There is still an issue when using generic with class see https://github.com/Microsoft/TypeScript/compare/master-fix15742?expand=1#diff-c5ae2fab8e9a760ddeda06b1b9128151

TODO

  • port to release

  • add more tests

}
return getIntersectionType(propsApprentType);
}
return propsType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if it is not an intersection, you do not want to call getApparentType on it?

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the special case for intersection types needed? I thought that getApparentType of an intersection type would do the right thing.

function getApparentTypeOfJsxPropsType(propsType: Type): Type {
if (propsType) {
if (propsType.flags & TypeFlags.Intersection) {
const propsApprentType: Type[] = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo:Apparent

if (propsType) {
if (propsType.flags & TypeFlags.Intersection) {
const propsApprentType: Type[] = [];
for (const t of (<UnionOrIntersectionType>propsType).types) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't getApparentType of an intersection type do this loop for you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later: no it doesn't. :( You should be able to express this as getIntersectionType(t.types.map(getApparentType))) though, right?

@@ -13548,6 +13548,20 @@ namespace ts {
return _jsxElementChildrenPropertyName;
}

function getApparentTypeOfJsxPropsType(propsType: Type): Type {
if (propsType) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this better:

if (!propsType) {
  return undefined;
}

@sandersn
Copy link
Member

Never mind, I looked more closely at getApparentType and it doesn't recursively call getApparentType on types.

@yuit
Copy link
Contributor Author

yuit commented May 12, 2017

@sandersn I wasn't expected that as well

@mhegazy mhegazy merged commit d68b436 into master May 12, 2017
@mhegazy
Copy link
Contributor

mhegazy commented May 12, 2017

@yuit can you please port this to release-2.3

@mhegazy mhegazy deleted the master-fix15742 branch May 12, 2017 19:24
@mhegazy
Copy link
Contributor

mhegazy commented May 12, 2017

never mind, I ported it.

@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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.

Generic JSX components
4 participants