Skip to content

Commit

Permalink
Merge pull request #15595 from Microsoft/master-fix15463
Browse files Browse the repository at this point in the history
[Master] fix 15463
  • Loading branch information
yuit authored May 8, 2017
2 parents 0b0a2d0 + 1e32b10 commit 1b520fc
Show file tree
Hide file tree
Showing 53 changed files with 1,228 additions and 196 deletions.
81 changes: 51 additions & 30 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8690,29 +8690,6 @@ namespace ts {
return Ternary.False;
}

// Check if a property with the given name is known anywhere in the given type. In an object type, a property
// is considered known if the object type is empty and the check is for assignability, if the object type has
// index signatures, or if the property is actually declared in the object type. In a union or intersection
// type, a property is considered known if it is known in any constituent type.
function isKnownProperty(type: Type, name: string, isComparingJsxAttributes: boolean): boolean {
if (type.flags & TypeFlags.Object) {
const resolved = resolveStructuredTypeMembers(<ObjectType>type);
if (resolved.stringIndexInfo || resolved.numberIndexInfo && isNumericLiteralName(name) ||
getPropertyOfType(type, name) || isComparingJsxAttributes && !isUnhyphenatedJsxName(name)) {
// For JSXAttributes, if the attribute has a hyphenated name, consider that the attribute to be known.
return true;
}
}
else if (type.flags & TypeFlags.UnionOrIntersection) {
for (const t of (<UnionOrIntersectionType>type).types) {
if (isKnownProperty(t, name, isComparingJsxAttributes)) {
return true;
}
}
}
return false;
}

function hasExcessProperties(source: FreshObjectLiteralType, target: Type, reportErrors: boolean): boolean {
if (maybeTypeOfKind(target, TypeFlags.Object) && !(getObjectFlags(target) & ObjectFlags.ObjectLiteralPatternWithComputedProperties)) {
const isComparingJsxAttributes = !!(source.flags & TypeFlags.JsxAttributes);
Expand Down Expand Up @@ -13276,6 +13253,8 @@ namespace ts {
let spread: Type = emptyObjectType;
let attributesArray: Symbol[] = [];
let hasSpreadAnyType = false;
let explicitlySpecifyChildrenAttribute = false;
const jsxChildrenPropertyName = getJsxElementChildrenPropertyname();

for (const attributeDecl of attributes.properties) {
const member = attributeDecl.symbol;
Expand All @@ -13294,6 +13273,9 @@ namespace ts {
attributeSymbol.target = member;
attributesTable.set(attributeSymbol.name, attributeSymbol);
attributesArray.push(attributeSymbol);
if (attributeDecl.name.text === jsxChildrenPropertyName) {
explicitlySpecifyChildrenAttribute = true;
}
}
else {
Debug.assert(attributeDecl.kind === SyntaxKind.JsxSpreadAttribute);
Expand Down Expand Up @@ -13352,11 +13334,11 @@ namespace ts {
}
}

// Error if there is a attribute named "children" and children element.
// This is because children element will overwrite the value from attributes
const jsxChildrenPropertyName = getJsxElementChildrenPropertyname();
if (!hasSpreadAnyType && jsxChildrenPropertyName && jsxChildrenPropertyName !== "") {
if (attributesTable.has(jsxChildrenPropertyName)) {
// Error if there is a attribute named "children" explicitly specified and children element.
// This is because children element will overwrite the value from attributes.
// Note: we will not warn "children" attribute overwritten if "children" attribute is specified in object spread.
if (explicitlySpecifyChildrenAttribute) {
error(attributes, Diagnostics._0_are_specified_twice_The_attribute_named_0_will_be_overwritten, jsxChildrenPropertyName);
}

Expand All @@ -13378,8 +13360,7 @@ namespace ts {
*/
function createJsxAttributesType(symbol: Symbol, attributesTable: Map<Symbol>) {
const result = createAnonymousType(symbol, attributesTable, emptyArray, emptyArray, /*stringIndexInfo*/ undefined, /*numberIndexInfo*/ undefined);
const freshObjectLiteralFlag = compilerOptions.suppressExcessPropertyErrors ? 0 : TypeFlags.FreshLiteral;
result.flags |= TypeFlags.JsxAttributes | TypeFlags.ContainsObjectLiteral | freshObjectLiteralFlag;
result.flags |= TypeFlags.JsxAttributes | TypeFlags.ContainsObjectLiteral;
result.objectFlags |= ObjectFlags.ObjectLiteral;
return result;
}
Expand Down Expand Up @@ -13898,6 +13879,34 @@ namespace ts {
checkJsxAttributesAssignableToTagNameAttributes(node);
}

/**
* Check if a property with the given name is known anywhere in the given type. In an object type, a property
* is considered known if the object type is empty and the check is for assignability, if the object type has
* index signatures, or if the property is actually declared in the object type. In a union or intersection
* type, a property is considered known if it is known in any constituent type.
* @param targetType a type to search a given name in
* @param name a property name to search
* @param isComparingJsxAttributes a boolean flag indicating whether we are searching in JsxAttributesType
*/
function isKnownProperty(targetType: Type, name: string, isComparingJsxAttributes: boolean): boolean {
if (targetType.flags & TypeFlags.Object) {
const resolved = resolveStructuredTypeMembers(<ObjectType>targetType);
if (resolved.stringIndexInfo || resolved.numberIndexInfo && isNumericLiteralName(name) ||
getPropertyOfType(targetType, name) || isComparingJsxAttributes && !isUnhyphenatedJsxName(name)) {
// For JSXAttributes, if the attribute has a hyphenated name, consider that the attribute to be known.
return true;
}
}
else if (targetType.flags & TypeFlags.UnionOrIntersection) {
for (const t of (<UnionOrIntersectionType>targetType).types) {
if (isKnownProperty(t, name, isComparingJsxAttributes)) {
return true;
}
}
}
return false;
}

/**
* Check whether the given attributes of JSX opening-like element is assignable to the tagName attributes.
* Get the attributes type of the opening-like element through resolving the tagName, "target attributes"
Expand Down Expand Up @@ -13930,7 +13939,19 @@ namespace ts {
error(openingLikeElement, Diagnostics.JSX_element_class_does_not_support_attributes_because_it_does_not_have_a_0_property, getJsxElementPropertiesName());
}
else {
checkTypeAssignableTo(sourceAttributesType, targetAttributesType, openingLikeElement.attributes.properties.length > 0 ? openingLikeElement.attributes : openingLikeElement);
// Check if sourceAttributesType assignable to targetAttributesType though this check will allow excess properties
const isSourceAttributeTypeAssignableToTarget = checkTypeAssignableTo(sourceAttributesType, targetAttributesType, openingLikeElement.attributes.properties.length > 0 ? openingLikeElement.attributes : openingLikeElement);
// After we check for assignability, we will do another pass to check that all explicitly specified attributes have correct name corresponding in targetAttributeType.
// This will allow excess properties in spread type as it is very common pattern to spread outter attributes into React component in its render method.
if (isSourceAttributeTypeAssignableToTarget && !isTypeAny(sourceAttributesType) && !isTypeAny(targetAttributesType)) {
for (const attribute of openingLikeElement.attributes.properties) {
if (isJsxAttribute(attribute) && !isKnownProperty(targetAttributesType, attribute.name.text, /*isComparingJsxAttributes*/ true)) {
error(attribute, Diagnostics.Property_0_does_not_exist_on_type_1, attribute.name.text, typeToString(targetAttributesType));
// We break here so that errors won't be cascading
break;
}
}
}
}
}

Expand Down
76 changes: 76 additions & 0 deletions tests/baselines/reference/checkJsxChildrenProperty12.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
//// [file.tsx]
import React = require('react');

interface ButtonProp {
a: number,
b: string,
children: Button;
}

class Button extends React.Component<ButtonProp, any> {
render() {
let condition: boolean;
if (condition) {
return <InnerButton {...this.props} />
}
else {
return (<InnerButton {...this.props} >
<div>Hello World</div>
</InnerButton>);
}
}
}

interface InnerButtonProp {
a: number
}

class InnerButton extends React.Component<InnerButtonProp, any> {
render() {
return (<button>Hello</button>);
}
}


//// [file.jsx]
"use strict";
var __extends = (this && this.__extends) || (function () {
var extendStatics = Object.setPrototypeOf ||
({ __proto__: [] } instanceof Array && function (d, b) { d.__proto__ = b; }) ||
function (d, b) { for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p]; };
return function (d, b) {
extendStatics(d, b);
function __() { this.constructor = d; }
d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};
})();
exports.__esModule = true;
var React = require("react");
var Button = (function (_super) {
__extends(Button, _super);
function Button() {
return _super !== null && _super.apply(this, arguments) || this;
}
Button.prototype.render = function () {
var condition;
if (condition) {
return <InnerButton {...this.props}/>;
}
else {
return (<InnerButton {...this.props}>
<div>Hello World</div>
</InnerButton>);
}
};
return Button;
}(React.Component));
var InnerButton = (function (_super) {
__extends(InnerButton, _super);
function InnerButton() {
return _super !== null && _super.apply(this, arguments) || this;
}
InnerButton.prototype.render = function () {
return (<button>Hello</button>);
};
return InnerButton;
}(React.Component));
80 changes: 80 additions & 0 deletions tests/baselines/reference/checkJsxChildrenProperty12.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
=== tests/cases/conformance/jsx/file.tsx ===
import React = require('react');
>React : Symbol(React, Decl(file.tsx, 0, 0))

interface ButtonProp {
>ButtonProp : Symbol(ButtonProp, Decl(file.tsx, 0, 32))

a: number,
>a : Symbol(ButtonProp.a, Decl(file.tsx, 2, 22))

b: string,
>b : Symbol(ButtonProp.b, Decl(file.tsx, 3, 14))

children: Button;
>children : Symbol(ButtonProp.children, Decl(file.tsx, 4, 14))
>Button : Symbol(Button, Decl(file.tsx, 6, 1))
}

class Button extends React.Component<ButtonProp, any> {
>Button : Symbol(Button, Decl(file.tsx, 6, 1))
>React.Component : Symbol(React.Component, Decl(react.d.ts, 158, 55))
>React : Symbol(React, Decl(file.tsx, 0, 0))
>Component : Symbol(React.Component, Decl(react.d.ts, 158, 55))
>ButtonProp : Symbol(ButtonProp, Decl(file.tsx, 0, 32))

render() {
>render : Symbol(Button.render, Decl(file.tsx, 8, 55))

let condition: boolean;
>condition : Symbol(condition, Decl(file.tsx, 10, 5))

if (condition) {
>condition : Symbol(condition, Decl(file.tsx, 10, 5))

return <InnerButton {...this.props} />
>InnerButton : Symbol(InnerButton, Decl(file.tsx, 24, 1))
>this.props : Symbol(React.Component.props, Decl(react.d.ts, 166, 37))
>this : Symbol(Button, Decl(file.tsx, 6, 1))
>props : Symbol(React.Component.props, Decl(react.d.ts, 166, 37))
}
else {
return (<InnerButton {...this.props} >
>InnerButton : Symbol(InnerButton, Decl(file.tsx, 24, 1))
>this.props : Symbol(React.Component.props, Decl(react.d.ts, 166, 37))
>this : Symbol(Button, Decl(file.tsx, 6, 1))
>props : Symbol(React.Component.props, Decl(react.d.ts, 166, 37))

<div>Hello World</div>
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2399, 45))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react.d.ts, 2399, 45))

</InnerButton>);
>InnerButton : Symbol(InnerButton, Decl(file.tsx, 24, 1))
}
}
}

interface InnerButtonProp {
>InnerButtonProp : Symbol(InnerButtonProp, Decl(file.tsx, 20, 1))

a: number
>a : Symbol(InnerButtonProp.a, Decl(file.tsx, 22, 27))
}

class InnerButton extends React.Component<InnerButtonProp, any> {
>InnerButton : Symbol(InnerButton, Decl(file.tsx, 24, 1))
>React.Component : Symbol(React.Component, Decl(react.d.ts, 158, 55))
>React : Symbol(React, Decl(file.tsx, 0, 0))
>Component : Symbol(React.Component, Decl(react.d.ts, 158, 55))
>InnerButtonProp : Symbol(InnerButtonProp, Decl(file.tsx, 20, 1))

render() {
>render : Symbol(InnerButton.render, Decl(file.tsx, 26, 65))

return (<button>Hello</button>);
>button : Symbol(JSX.IntrinsicElements.button, Decl(react.d.ts, 2385, 43))
>button : Symbol(JSX.IntrinsicElements.button, Decl(react.d.ts, 2385, 43))
}
}

86 changes: 86 additions & 0 deletions tests/baselines/reference/checkJsxChildrenProperty12.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
=== tests/cases/conformance/jsx/file.tsx ===
import React = require('react');
>React : typeof React

interface ButtonProp {
>ButtonProp : ButtonProp

a: number,
>a : number

b: string,
>b : string

children: Button;
>children : Button
>Button : Button
}

class Button extends React.Component<ButtonProp, any> {
>Button : Button
>React.Component : React.Component<ButtonProp, any>
>React : typeof React
>Component : typeof React.Component
>ButtonProp : ButtonProp

render() {
>render : () => JSX.Element

let condition: boolean;
>condition : boolean

if (condition) {
>condition : boolean

return <InnerButton {...this.props} />
><InnerButton {...this.props} /> : JSX.Element
>InnerButton : typeof InnerButton
>this.props : ButtonProp & { children?: React.ReactNode; }
>this : this
>props : ButtonProp & { children?: React.ReactNode; }
}
else {
return (<InnerButton {...this.props} >
>(<InnerButton {...this.props} > <div>Hello World</div> </InnerButton>) : JSX.Element
><InnerButton {...this.props} > <div>Hello World</div> </InnerButton> : JSX.Element
>InnerButton : typeof InnerButton
>this.props : ButtonProp & { children?: React.ReactNode; }
>this : this
>props : ButtonProp & { children?: React.ReactNode; }

<div>Hello World</div>
><div>Hello World</div> : JSX.Element
>div : any
>div : any

</InnerButton>);
>InnerButton : typeof InnerButton
}
}
}

interface InnerButtonProp {
>InnerButtonProp : InnerButtonProp

a: number
>a : number
}

class InnerButton extends React.Component<InnerButtonProp, any> {
>InnerButton : InnerButton
>React.Component : React.Component<InnerButtonProp, any>
>React : typeof React
>Component : typeof React.Component
>InnerButtonProp : InnerButtonProp

render() {
>render : () => JSX.Element

return (<button>Hello</button>);
>(<button>Hello</button>) : JSX.Element
><button>Hello</button> : JSX.Element
>button : any
>button : any
}
}

Loading

0 comments on commit 1b520fc

Please sign in to comment.