Skip to content

Commit ab2e14f

Browse files
author
Kanchalai Tanglertsampan
committed
Addressing code review in previous PR. Combining below commits
Address comment: fix spelling mistakes Address comment: calling `checkApplicableSignatureForJsxOpeningLikeElement` from inside `checkApplicableSignature` Address comment: fix spelling, rename function to be more consistent Address comment: minor fix indentation, fix function name isObjectLiteralPropertyDeclaration => isObjectLiteralElement Address PR: gotoDefinition return the last signature when there is an error in statelss function component Address PR: convert Foreach to for...of Address comment: fix type, inline code, clarify name of variables
1 parent 747ab05 commit ab2e14f

6 files changed

Lines changed: 72 additions & 85 deletions

File tree

‎src/compiler/checker.ts‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11870,7 +11870,7 @@ namespace ts {
1187011870
* @param openingLikeElement a Jsx opening-like element
1187111871
* @return an anonymous type (similar to the one returned by checkObjectLiteral) in which its properties are attributes property.
1187211872
*/
11873-
function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, filter?:(symbol: Symbol)=>boolean) {
11873+
function createJsxAttributesTypeFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement, filter?: (symbol: Symbol) => boolean) {
1187411874
const attributes = openingLikeElement.attributes;
1187511875
let attributesTable = createMap<Symbol>();
1187611876
let spread: Type = emptyObjectType;
@@ -11924,8 +11924,8 @@ namespace ts {
1192411924
attributesTable = createMap<Symbol>();
1192511925
if (attributesArray) {
1192611926
forEach(attributesArray, (attr) => {
11927-
if (!filter || (filter && filter(attr))) {
11928-
attributesTable[attr.name] = attr;
11927+
if (!filter || filter(attr)) {
11928+
attributesTable.set(attr.name, attr);
1192911929
}
1193011930
});
1193111931
}
@@ -11963,7 +11963,7 @@ namespace ts {
1196311963
*/
1196411964
function checkJsxAttributesAssignableToTagnameAttributes(openingLikeElement: JsxOpeningLikeElement) {
1196511965
// The function involves following steps:
11966-
// 1. Figure out expected attributes type expected by resolving tag-name of the JSX opening-like element, tagetAttributesType.
11966+
// 1. Figure out expected attributes type expected by resolving tag-name of the JSX opening-like element, targetAttributesType.
1196711967
// During these steps, we will try to resolve the tag-name as intrinsic name, stateless function, stateful component (in the order)
1196811968
// 2. Solved Jsx attributes type given by users, sourceAttributesType, which is by resolving "attributes" property of the JSX opening-like element.
1196911969
// 3. Check if the two are assignable to each other
@@ -11977,7 +11977,7 @@ namespace ts {
1197711977
// i.e <div attr1={10} attr2="string" />
1197811978
// attr1 and attr2 are treated as JSXAttributes attached in the JsxOpeningLikeElement as "attributes". They resolved to be sourceAttributesType.
1197911979
const sourceAttributesType = createJsxAttributesTypeFromAttributesProperty(openingLikeElement,
11980-
(attribute: Symbol) => {
11980+
attribute => {
1198111981
return isUnhyphenatedJsxName(attribute.name) || !!(getPropertyOfType(targetAttributesType, attribute.name));
1198211982
});
1198311983

@@ -13109,12 +13109,6 @@ namespace ts {
1310913109
* @param excludeArgument
1311013110
*/
1311113111
function checkApplicableSignatureForJsxOpeningLikeElement(node: JsxOpeningLikeElement, signature: Signature, relation: Map<RelationComparisonResult>) {
13112-
const headMessage = Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1;
13113-
// Stateless function components can have maximum of three arguments: "props", "context", and "updater".
13114-
// However "context" and "updater" are implicit and can't be specify by users. Only the first parameter, props,
13115-
// can be specified by users through attributes property.
13116-
const paramType = getTypeAtPosition(signature, 0);
13117-
1311813112
// JSX opening-like element has correct arity for stateless-function component if the one of the following condition is true:
1311913113
// 1. callIsIncomplete
1312013114
// 2. attributes property has same number of properties as the parameter object type.
@@ -13126,6 +13120,11 @@ namespace ts {
1312613120
return true;
1312713121
}
1312813122

13123+
const headMessage = Diagnostics.Argument_of_type_0_is_not_assignable_to_parameter_of_type_1;
13124+
// Stateless function components can have maximum of three arguments: "props", "context", and "updater".
13125+
// However "context" and "updater" are implicit and can't be specify by users. Only the first parameter, props,
13126+
// can be specified by users through attributes property.
13127+
const paramType = getTypeAtPosition(signature, 0);
1312913128
const argType = checkExpressionWithContextualType(node.attributes, paramType, /*contextualMapper*/ undefined);
1313013129
const argProperties = getPropertiesOfType(argType);
1313113130
const paramProperties = getPropertiesOfType(paramType);
@@ -13146,6 +13145,9 @@ namespace ts {
1314613145
}
1314713146

1314813147
function checkApplicableSignature(node: CallLikeExpression, args: Expression[], signature: Signature, relation: Map<RelationComparisonResult>, excludeArgument: boolean[], reportErrors: boolean) {
13148+
if (isJsxOpeningLikeElement(node)) {
13149+
return checkApplicableSignatureForJsxOpeningLikeElement(<JsxOpeningLikeElement>node, signature, relation);
13150+
}
1314913151
const thisType = getThisTypeOfSignature(signature);
1315013152
if (thisType && thisType !== voidType && node.kind !== SyntaxKind.NewExpression) {
1315113153
// If the called expression is not of the form `x.f` or `x["f"]`, then sourceType = voidType
@@ -13620,12 +13622,10 @@ namespace ts {
1362013622
return result;
1362113623
}
1362213624

13623-
// Do not report any error if we are doing so for stateless function component as such error will be error will be handle in "resolveCustomJsxElementAttributesType".
1362413625
if (isJsxOpeningOrSelfClosingElement) {
13625-
// If this is a type resolution session, e.g. Language Service, just return undefined as the language service can decide how to proceed with this failure.
13626-
// (see getDefinitionAtPosition which simply get the symbol and return the first declaration of the JSXopeningLikeElement node)
13627-
// otherwise, just return the latest signature candidate we try so far so that when we report an error we will get better error message.
13628-
return produceDiagnostics ? candidateForArgumentError : undefined;
13626+
// If there is not result, just return the last one we try as a candidate.
13627+
// We do not report any error here because any error will be handled in "resolveCustomJsxElementAttributesType".
13628+
return candidateForArgumentError;
1362913629
}
1363013630

1363113631
// No signatures were applicable. Now report errors based on the last applicable signature with
@@ -13726,10 +13726,7 @@ namespace ts {
1372613726
}
1372713727
candidate = getSignatureInstantiation(candidate, typeArgumentTypes);
1372813728
}
13729-
if (isJsxOpeningOrSelfClosingElement && !checkApplicableSignatureForJsxOpeningLikeElement(<JsxOpeningLikeElement>node, candidate, relation)) {
13730-
break;
13731-
}
13732-
else if (!isJsxOpeningOrSelfClosingElement && !checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) {
13729+
if (!checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) {
1373313730
break;
1373413731
}
1373513732
const index = excludeArgument ? indexOf(excludeArgument, true) : -1;
@@ -14061,7 +14058,7 @@ namespace ts {
1406114058
}
1406214059
links.resolvedSignature = resolvingSignature;
1406314060

14064-
let callSignature = resolvedStatelessJsxOpeningLikeElement(openingLikeElement, elementType, candidatesOutArray);
14061+
let callSignature = resolveStatelessJsxOpeningLikeElement(openingLikeElement, elementType, candidatesOutArray);
1406514062
if (!callSignature || callSignature === unknownSignature) {
1406614063
const callSignatures = elementType && getSignaturesOfType(elementType, SignatureKind.Call);
1406714064
callSignature = callSignatures[callSignatures.length - 1];
@@ -14083,14 +14080,14 @@ namespace ts {
1408314080
* @return a resolved signature if we can find function matching function signature through resolve call or a first signature in the list of functions.
1408414081
* otherwise return undefined if tag-name of the opening-like element doesn't have call signatures
1408514082
*/
14086-
function resolvedStatelessJsxOpeningLikeElement(openingLikeElement: JsxOpeningLikeElement, elementType: Type, candidatesOutArray: Signature[]): Signature {
14083+
function resolveStatelessJsxOpeningLikeElement(openingLikeElement: JsxOpeningLikeElement, elementType: Type, candidatesOutArray: Signature[]): Signature {
1408714084
if (elementType.flags & TypeFlags.Union) {
1408814085
const types = (elementType as UnionType).types;
1408914086
let result: Signature;
14090-
types.map(type => {
14087+
for (const type of types) {
1409114088
// This is mainly to fill in all the candidates if there is one.
14092-
result = result || resolvedStatelessJsxOpeningLikeElement(openingLikeElement, type, candidatesOutArray);
14093-
});
14089+
result = result || resolveStatelessJsxOpeningLikeElement(openingLikeElement, type, candidatesOutArray);
14090+
}
1409414091

1409514092
return result;
1409614093
}
@@ -14117,7 +14114,7 @@ namespace ts {
1411714114
return resolveDecorator(<Decorator>node, candidatesOutArray);
1411814115
case SyntaxKind.JsxOpeningElement:
1411914116
case SyntaxKind.JsxSelfClosingElement:
14120-
return resolvedStatelessJsxOpeningLikeElement(<JsxOpeningLikeElement>node, checkExpression((<JsxOpeningLikeElement>node).tagName), candidatesOutArray);
14117+
return resolveStatelessJsxOpeningLikeElement(<JsxOpeningLikeElement>node, checkExpression((<JsxOpeningLikeElement>node).tagName), candidatesOutArray);
1412114118
}
1412214119
Debug.fail("Branch in 'resolveSignature' should be unreachable.");
1412314120
}

‎src/services/completions.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,11 +1381,11 @@ namespace ts.Completions {
13811381
return <JsxOpeningLikeElement>parent;
13821382
}
13831383
else if (parent.kind === SyntaxKind.JsxAttribute) {
1384-
// Currently we parse JsxOpeninLikeElement as:
1385-
// JsxOpeninLikeElement
1384+
// Currently we parse JsxOpeningLikeElement as:
1385+
// JsxOpeningLikeElement
13861386
// attributes: JsxAttributes
13871387
// properties: NodeArray<JsxAttributeLike>
1388-
return /*properties list*/parent./*attributes*/parent.parent as JsxOpeningLikeElement;
1388+
return parent.parent.parent as JsxOpeningLikeElement;
13891389
}
13901390
break;
13911391

@@ -1394,11 +1394,11 @@ namespace ts.Completions {
13941394
// whose parent is a JsxOpeningLikeElement
13951395
case SyntaxKind.StringLiteral:
13961396
if (parent && ((parent.kind === SyntaxKind.JsxAttribute) || (parent.kind === SyntaxKind.JsxSpreadAttribute))) {
1397-
// Currently we parse JsxOpeninLikeElement as:
1398-
// JsxOpeninLikeElement
1397+
// Currently we parse JsxOpeningLikeElement as:
1398+
// JsxOpeningLikeElement
13991399
// attributes: JsxAttributes
14001400
// properties: NodeArray<JsxAttributeLike>
1401-
return /*properties list*/parent./*attributes*/parent.parent as JsxOpeningLikeElement;
1401+
return parent.parent.parent as JsxOpeningLikeElement;
14021402
}
14031403

14041404
break;
@@ -1407,20 +1407,20 @@ namespace ts.Completions {
14071407
if (parent &&
14081408
parent.kind === SyntaxKind.JsxExpression &&
14091409
parent.parent && parent.parent.kind === SyntaxKind.JsxAttribute) {
1410-
// Currently we parse JsxOpeninLikeElement as:
1411-
// JsxOpeninLikeElement
1410+
// Currently we parse JsxOpeningLikeElement as:
1411+
// JsxOpeningLikeElement
14121412
// attributes: JsxAttributes
14131413
// properties: NodeArray<JsxAttributeLike>
14141414
// each JsxAttribute can have initializer as JsxExpression
1415-
return /*JsxExpression*/parent./*JsxAttribute*/parent./*JsxAttributes*/parent.parent as JsxOpeningLikeElement;
1415+
return parent.parent.parent.parent as JsxOpeningLikeElement;
14161416
}
14171417

14181418
if (parent && parent.kind === SyntaxKind.JsxSpreadAttribute) {
1419-
// Currently we parse JsxOpeninLikeElement as:
1420-
// JsxOpeninLikeElement
1419+
// Currently we parse JsxOpeningLikeElement as:
1420+
// JsxOpeningLikeElement
14211421
// attributes: JsxAttributes
14221422
// properties: NodeArray<JsxAttributeLike>
1423-
return /*properties list*/parent./*attributes*/parent.parent as JsxOpeningLikeElement;
1423+
return parent.parent.parent as JsxOpeningLikeElement;
14241424
}
14251425

14261426
break;

‎src/services/goToDefinition.ts‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,19 +102,19 @@ namespace ts.GoToDefinition {
102102
// object literal, lookup the property symbol in the contextual type, and use this for goto-definition.
103103
// For example
104104
// interface Props{
105-
// /first*/prop1: number
105+
// /*first*/prop1: number
106106
// prop2: boolean
107107
// }
108108
// function Foo(arg: Props) {}
109109
// Foo( { pr/*1*/op1: 10, prop2: true })
110-
const container = getContainingObjectLiteralElement(node);
111-
if (container) {
112-
const contextualType = typeChecker.getContextualType(node.parent.parent as Expression);
113-
if (contextualType) {
114-
let result: DefinitionInfo[] = [];
115-
forEach(getPropertySymbolsFromContextualType(typeChecker, container), contextualSymbol => {
116-
result = result.concat(getDefinitionFromSymbol(typeChecker, contextualSymbol, node));
117-
});
110+
const element = getContainingObjectLiteralElement(node);
111+
if (element) {
112+
if (typeChecker.getContextualType(element.parent as Expression)) {
113+
const result: DefinitionInfo[] = [];
114+
const propertySymbols = getPropertySymbolsFromContextualType(typeChecker, element);
115+
for (const propertySymbol of propertySymbols) {
116+
result.push(...getDefinitionFromSymbol(typeChecker, propertySymbol, node));
117+
}
118118
return result;
119119
}
120120
}

‎src/services/services.ts‎

Lines changed: 23 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1990,9 +1990,10 @@ namespace ts {
19901990
}
19911991
}
19921992

1993-
function isObjectLiteralPropertyDeclaration(node: Node): node is ObjectLiteralElement {
1993+
function isObjectLiteralElement(node: Node): node is ObjectLiteralElement {
19941994
switch (node.kind) {
19951995
case SyntaxKind.JsxAttribute:
1996+
case SyntaxKind.JsxSpreadAttribute:
19961997
case SyntaxKind.PropertyAssignment:
19971998
case SyntaxKind.ShorthandPropertyAssignment:
19981999
case SyntaxKind.MethodDeclaration:
@@ -2003,18 +2004,6 @@ namespace ts {
20032004
return false;
20042005
}
20052006

2006-
function getNameFromObjectLiteralElement(node: ObjectLiteralElement) {
2007-
if (node.name.kind === SyntaxKind.ComputedPropertyName) {
2008-
const nameExpression = (<ComputedPropertyName>node.name).expression;
2009-
// treat computed property names where expression is string/numeric literal as just string/numeric literal
2010-
if (isStringOrNumericLiteral(nameExpression)) {
2011-
return (<LiteralExpression>nameExpression).text;
2012-
}
2013-
return undefined;
2014-
}
2015-
return (<Identifier | LiteralExpression>node.name).text;
2016-
}
2017-
20182007
/**
20192008
* Returns the containing object literal property declaration given a possible name node, e.g. "a" in x = { "a": 1 }
20202009
*/
@@ -2024,11 +2013,11 @@ namespace ts {
20242013
case SyntaxKind.StringLiteral:
20252014
case SyntaxKind.NumericLiteral:
20262015
if (node.parent.kind === SyntaxKind.ComputedPropertyName) {
2027-
return isObjectLiteralPropertyDeclaration(node.parent.parent) ? node.parent.parent : undefined;
2016+
return isObjectLiteralElement(node.parent.parent) ? node.parent.parent : undefined;
20282017
}
20292018
// intentionally fall through
20302019
case SyntaxKind.Identifier:
2031-
return isObjectLiteralPropertyDeclaration(node.parent) &&
2020+
return isObjectLiteralElement(node.parent) &&
20322021
(node.parent.parent.kind === SyntaxKind.ObjectLiteralExpression || node.parent.parent.kind === SyntaxKind.JsxAttributes) &&
20332022
(<ObjectLiteralElement>node.parent).name === node ? node.parent as ObjectLiteralElement : undefined;
20342023
}
@@ -2037,27 +2026,28 @@ namespace ts {
20372026

20382027
/* @internal */
20392028
export function getPropertySymbolsFromContextualType(typeChecker: TypeChecker, node: ObjectLiteralElement): Symbol[] {
2040-
const objectLiteral = <ObjectLiteralExpression>node.parent;
2041-
const contextualType = typeChecker.getContextualType(objectLiteral);
2042-
const name = getNameFromObjectLiteralElement(node);
2043-
if (name && contextualType) {
2044-
const result: Symbol[] = [];
2045-
const symbol = contextualType.getProperty(name);
2046-
if (symbol) {
2047-
result.push(symbol);
2048-
}
2029+
const objectLiteral = <ObjectLiteralExpression | JsxAttributes>node.parent;
2030+
const contextualType = typeChecker.getContextualType(objectLiteral);
2031+
const name = getTextOfPropertyName(node.name);
2032+
if (name && contextualType) {
2033+
const result: Symbol[] = [];
2034+
const symbol = contextualType.getProperty(name);
2035+
if (contextualType.flags & TypeFlags.Union) {
2036+
forEach((<UnionType>contextualType).types, t => {
2037+
const symbol = t.getProperty(name);
2038+
if (symbol) {
2039+
result.push(symbol);
2040+
}
2041+
});
2042+
return result;
2043+
}
20492044

2050-
if (contextualType.flags & TypeFlags.Union) {
2051-
forEach((<UnionType>contextualType).types, t => {
2052-
const symbol = t.getProperty(name);
2053-
if (symbol) {
2054-
result.push(symbol);
2055-
}
2056-
});
2057-
}
2045+
if (symbol) {
2046+
result.push(symbol);
20582047
return result;
20592048
}
2060-
return undefined;
2049+
}
2050+
return undefined;
20612051
}
20622052

20632053
function isArgumentOfElementAccessExpression(node: Node) {

0 commit comments

Comments
 (0)