[WIP] Using overload for JSX stateless function component #11871

yuit · 2016-10-26T22:08:52Z

This PR is still WIP. Some clean up will be needed. It is branch off from @sandersn PR on spread operator.

Fix: #11187 (see this test: tsxSpreadAttributesResolution1), #9703

Overview of the change

Treat JSXAttributes similar to ObjectLiteralExpression. We change the AST structure and store JSXAttributes as a AST Node instead of NodeArray
Using overload resolution logic in choosing signature of stateless function component
This change rely on @sandersn spread operator to resolve the attributs

Todo

- Handle of spreading with any

Currently when we see spread of any type, the entire attributes has any type which means that follow example is allowed even though we should give an error:

declare function MainBuntton(attr1: {to: string}): JSX.Element;
declare function MainBuntton(attr2: {onClick: ()=> void}): JSX.Element;
declare var obj3: any;
const b = <MainBuntton {...obj3} to={10} />;

Add tests for quick-info, goto-definition, completion to make sure that we reflect the choosen overload
Add tests when using stateless with type arguments.

@RyanCavanaugh @mhegazy

…rification

Convert attributes property in JsxOpeningElement and JsxSelfClosingElement to use JsxAttributes type Convert jsxAttributes to be based on ObjectLiteralExpressionBase Fix how to get NodeEmitFlag in factory Update JsxSpreadAttribute

…LikeElement to clarify that it get the attributes type by resolving opening-like element's tagName

wip-get attributes from opening-like-element attributes Node Check jsx attributes for intrinsic element to base on object-literal logic Change the spreadElementType

…state-less component wip-resovle custom JSX attributes type wip-using overload resolution to figure out stateless functions wip-complete using resolve signature logic to try picking stateless function

Update comments Update comment

…Type Update baselines

…Attribute

…n objectLiteral-like

Add tests for stateless-function overload Update tests

…TypeScript into wip-statelessOverload

Lock tslint version to 4.0.0-dev.0, because 4.0.0-dev.1 complains about unnecessary semicolons following properties

Fix 'keyof' for constrained type parameters

* add test for the fix for overwrite emitting error * cr feedback

Ensure transformFlags are correct before visiting a node.

Mark property referenced in the destructuring as referenced

# Conflicts: # src/compiler/checker.ts # src/compiler/types.ts # src/harness/harness.ts # src/services/completions.ts

…g in generic

sandersn

Some initial comments. I'm starting on checker now and will have more comments later.

sandersn · 2016-11-04T23:20:03Z

src/compiler/types.ts

-    export interface JsxAttribute extends Node {
-        kind: SyntaxKind.JsxAttribute;
+    // @kind(SyntaxKind.JsxAttribute)
+    export interface JsxAttribute extends ObjectLiteralElement {


doesn't JsxAttribute still need a kind: SyntaxKind.JsxAttribute; ?

you are right. will fix that

also don't need the // @kind(...) any more

sandersn · 2016-11-04T23:21:23Z

src/compiler/types.ts

@@ -2704,6 +2710,8 @@ namespace ts {
        ContainsObjectLiteral   = 1 << 22,  // Type is or contains object literal type
        /* @internal */
        ContainsAnyFunctionType = 1 << 23,  // Type is or contains object literal type
+        /* @internal */
+        JsxAttributes           = 1 << 24,  // Jsx attributes type


My preliminary reaction is that JsxAttributes can't produces types that are very different from normal typescript types, so I expect that this flag is not really needed. But I still need to read the rest of the code.

Oh, it's because of the ObjectLiteralBase type. Hm.

Yea the reason I need this flag is because in checking "hasExcessProperties" jsx attributes have different requirement

sandersn

A few more comments for now. I have quite a bit more of the checker to read still.

sandersn · 2016-11-07T17:54:20Z

src/compiler/checker.ts

+                        return true;
+                    }
+                    else if (getPropertyOfType(type, name) || (isComparingJsxAttributes && !isUnhyphenatedJsxName(name))) {
+                        // For JSXAttributes, if the attribute has hyphenated name considered the attribute to be known


rewrite: "if the attribute has a hyphenated name**, consider** the attribute to be known"

sandersn · 2016-11-07T18:03:03Z

src/compiler/checker.ts

+            let sourceAttributesType = anyType as Type;
+            let isSourceAttributesTypeEmpty = true;
+            if (symbolArray) {
+                // Filter out any hyphenated names as those are not play any role in type-checking unless there are corresponding properties in the target type


as these do not play

sandersn · 2016-11-07T18:04:29Z

src/compiler/checker.ts

+                    else if (resolved.stringIndexInfo || (resolved.numberIndexInfo && isNumericLiteralName(name))) {
+                        return true;
+                    }
+                    else if (getPropertyOfType(type, name) || (isComparingJsxAttributes && !isUnhyphenatedJsxName(name))) {


It's a little sad to see !isUnhyphenated instead of isHyphenated. But the other 3 uses are positive and they would get more confusing to read if they were !isHyphenated. Still, it might be worth adding another function named isHyphenated.

I agree the double negative here is a bit confused. It is the only place that check for hyphenate so I am hesistant to just add a function for that

sandersn · 2016-11-07T18:14:16Z

src/compiler/checker.ts

+                                    reportError(Diagnostics.Property_0_does_not_exist_on_type_1, symbolToString(prop), typeToString(target));
+                                }
+                                else {
+                                    errorNode = prop.valueDeclaration;


Why not use prop.valueDeclaration as the errorNode for JSX too?

If you can do (1), why not just push isJsxAttributes down into the first argument of reportError?

The reason we don't want to just use prop.valueDecalration is that when we report an error we want it to highlight entire JSX-attributes not just one individual attribute.

sandersn · 2016-11-07T18:18:05Z

src/compiler/checker.ts

+            // When we trying to resolve JsxOpeningLikeElement as a stateless function element, we will already give JSXAttributes a contextual type
+            // which is a type of the parameter  of the signature we are trying out. This is not the case if it is a statefull Jsx (i.e ReactComponenet class)
+            // So if that is the case, just return the type of the JsxAttribute in such contextual type with out going into resolving of the JsxOpeningLikeElement again
+            if ((<JsxAttributes>attribute.parent).contextualType) {


This is probably wrong. Any time I played games with contextual typing, it turned out that I was missing the root of the problem. I need to read the rest of the review to see whether I can figure out what the root actually is, though.

At least, would it work to express this as

const attributesType = getContextualType(attribute.parent); if (attributesType) { return isJsxAttribute(attribute) ? getTypeOfPropertyOfType(attributesType, attribute.name.text) : undefined; }

Or maybe you need an additional predicate next to attributesType to find out if attribute.parent is a stateful component or not.

Actually, as we looked at it together, I think you can simplify it even more with something like

const attributesType = getContextualType(attribute.parent) || getAttributesTypeFromJsxOpeningLikeElement(attribute.parent.parent); if (kind === SyntaxKind.JsxAttribute) ...

sandersn

More comments on the contextual typing problem

sandersn · 2016-11-07T19:29:29Z

src/compiler/checker.ts

@@ -10497,17 +10545,24 @@ namespace ts {
        }

        function getContextualTypeForJsxAttribute(attribute: JsxAttribute | JsxSpreadAttribute) {
+            // When we trying to resolve JsxOpeningLikeElement as a stateless function element, we will already give JSXAttributes a contextual type
+            // which is a type of the parameter  of the signature we are trying out. This is not the case if it is a statefull Jsx (i.e ReactComponenet class)


typo:stateful

sandersn · 2016-11-07T19:30:43Z

src/compiler/checker.ts

+            // When we trying to resolve JsxOpeningLikeElement as a stateless function element, we will already give JSXAttributes a contextual type
+            // which is a type of the parameter  of the signature we are trying out. This is not the case if it is a statefull Jsx (i.e ReactComponenet class)
+            // So if that is the case, just return the type of the JsxAttribute in such contextual type with out going into resolving of the JsxOpeningLikeElement again
+            if ((<JsxAttributes>attribute.parent).contextualType) {


At least, would it work to express this as

const attributesType = getContextualType(attribute.parent); if (attributesType) { return isJsxAttribute(attribute) ? getTypeOfPropertyOfType(attributesType, attribute.name.text) : undefined; }

sandersn · 2016-11-07T19:31:13Z

src/compiler/checker.ts

+            // When we trying to resolve JsxOpeningLikeElement as a stateless function element, we will already give JSXAttributes a contextual type
+            // which is a type of the parameter  of the signature we are trying out. This is not the case if it is a statefull Jsx (i.e ReactComponenet class)
+            // So if that is the case, just return the type of the JsxAttribute in such contextual type with out going into resolving of the JsxOpeningLikeElement again
+            if ((<JsxAttributes>attribute.parent).contextualType) {


Or maybe you need an additional predicate next to attributesType to find out if attribute.parent is a stateful component or not.

sandersn · 2016-11-07T22:28:49Z

src/compiler/checker.ts

+            // When we trying to resolve JsxOpeningLikeElement as a stateless function element, we will already give JSXAttributes a contextual type
+            // which is a type of the parameter  of the signature we are trying out. This is not the case if it is a statefull Jsx (i.e ReactComponenet class)
+            // So if that is the case, just return the type of the JsxAttribute in such contextual type with out going into resolving of the JsxOpeningLikeElement again
+            if ((<JsxAttributes>attribute.parent).contextualType) {


Actually, as we looked at it together, I think you can simplify it even more with something like

const attributesType = getContextualType(attribute.parent) || getAttributesTypeFromJsxOpeningLikeElement(attribute.parent.parent); if (kind === SyntaxKind.JsxAttribute) ...

sandersn

More comments, although I am still reviewing through the resolve stateless function stuff.

sandersn · 2016-11-07T23:07:20Z

src/compiler/checker.ts

+         * @param openingLikeElement a Jsx opening-like element
+         * @return a symbol table resulted from resolving "attributes" property or undefined if any of the attribute resolved to any or there is no attributes.
+         */
+        function getJsxAttributesSymbolArrayFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement): Symbol[] | undefined {


Why not return a SymbolTable? You create one already: attributesTable, and both callers of this function immediately create another one.

The name is ungainly. What about getJsxAttributeSymbolsFromJsxElement (or JsxOpeningLikeElement`). That's a little better anyway :)

And if you do (1) then you'll not need attributesArray any more, just a boolean to track whether attributesTable is empty or not.

In fact, it looks like both callers immediately call createJsxAttributesType. I would do that as the last action of this function instead and call this getJsxAttributesTypeFromJsxElement.

Talk offline, I will try to refactor the code here

sandersn · 2016-11-07T23:07:53Z

src/compiler/checker.ts

+        function getJsxAttributesSymbolArrayFromAttributesProperty(openingLikeElement: JsxOpeningLikeElement): Symbol[] | undefined {
+            const attributes = openingLikeElement.attributes;
+            let attributesTable = createMap<Symbol>();
+            let spread: Type = emptyObjectType


missing semi-colon. (I can't believe I caught that, I do this all the time myself)

sandersn · 2016-11-07T23:11:12Z

src/compiler/checker.ts

+            for (const attributeDecl of attributes.properties) {
+                const member = attributeDecl.symbol;
+                if (isJsxAttribute(attributeDecl)) {
+                    let exprType: Type;


I would just use a ternary operator for this since the predicate is short.

sandersn · 2016-11-07T23:22:43Z

src/compiler/checker.ts

+                        attributesTable = createMap<Symbol>();
+                    }
+                    const exprType = checkExpression(attributeDecl.expression);
+                    const widenExprType = getWidenedType(exprType);


probably better spelt as widenedExprType ?

why do you have to widen the type here?

After reviewing this, I agree. I shouldn't widen here since if it is "undefined" then we shouldn't allow it to be spread.

ES standard spread allows { ...undefined } but defines it as equal to {}. Same for null. Should JSX differ here?

sandersn · 2016-11-07T23:36:05Z

src/compiler/checker.ts

+         * @param jsxAttributesSymb a JsxAttributes node containing attributes in attributesTable
+         * @param attributesTable a symbol table of attributes property
+         */
+        function createJsxAttributesType(jsxAttributesSymb: Symbol, attributesTable: Map<Symbol>) {


can you un-abbreviate to either symbol or jsxAttributesSymbol? I prefer symbol because this is a short function. Also attributes would be a fine name for the second parameter.

sandersn · 2016-11-08T00:02:59Z

src/compiler/checker.ts

+            if (symbolArray) {
+                const symbolTable = createMap<Symbol>();
+                forEach(symbolArray, (attr) => {
+                    symbolTable[attr.name] = attr;


what about hyphenated names? The other caller handles hyphenated names differently.

sandersn · 2016-11-08T00:12:29Z

src/compiler/checker.ts

-         * us which attributes are valid on a given element.
+         * Get attributes type of the given intrinsic opening-like Jsx element by resolving the tag name.
+         * The function is intended to be called from a function which has checked that the opening element is an intrinsic element.
+         * @param node an intrinsic JSX oopening-like element


typo:opening

can you add an assert to verify that it's an intrinsic attribute, like Debug.assert(symbol) after calling getIntrinsicTagSymbol or something?

sandersn · 2016-11-08T00:15:17Z

src/compiler/checker.ts

+         * The function is intended to be called from a function which has handle intrinsic Jsx element already.
+         * @param node a custom Jsx opening-like element
+         */
+        function getCustomJsxElementAttributesType(node: JsxOpeningLikeElement, shouldIncludeAllStatelessAttributesType = false): Type {


since all callers are new, can you just make the boolean required? It's a little more straightforward to read

sandersn · 2016-11-08T00:25:11Z

src/compiler/checker.ts

                }), /*subtypeReduction*/ true);
            }

            // If the elemType is a string type, we have to return anyType to prevent an error downstream as we will try to find construct or call signature of the type
-            if (elemType.flags & TypeFlags.String) {
+            // If the elemType is a string type, we have to return anyType to prevent an error downstream as we will try to find construct or call signature of the type


sandersn · 2016-11-08T00:29:28Z

src/compiler/checker.ts

+         * @return a resolved signature if we can find function matching function signature through resolve call or a first signature in the list of functions.
+         *         otherwise return undefined if tag-name of the opening-like element doesn't have call signatures
+         */
+        function resolvedStateLessJsxOpeningLikeElement(openingLikeElement: JsxOpeningLikeElement, elementType: Type, candidatesOutArray: Signature[]): Signature {


Oh no! The MicroSoft disease has turned Stateless into StateLess!

sandersn

Here are my comments for now. I'm switching to the cleaned-up PR so I can use codeflow.

sandersn · 2016-11-08T17:38:04Z

src/compiler/checker.ts

+        /**
+         * Resolve attributes type of the given node. The function is intended to initially be called from getAttributesTypeFromJsxOpeningLikeElement which already handle JSX-intrinsic-element.
+         * @param openingLikeElement a non-intrinsic JSXOPeningLikeElement
+         * @param elementType an instance type of the given node


Add a param tag for shouldIncludeAllStatelessAttributesType or just remove the param tags.

sandersn · 2016-11-08T17:48:41Z

src/compiler/checker.ts

-                        return paramType;
-                    }
-                }
+            // Is this is a stateless function component? See if its single signature's return type is assignable to the JSX Element Type


update comment

sandersn · 2016-11-08T17:52:41Z

src/compiler/checker.ts

+                    getResolvedJSXStatelessFunctionSignature(openingLikeElement, elementType, candidatesOutArray);
+                    let result: Type;
+                    let defaultResult: Type;
+                    for (const candidate of candidatesOutArray) {


Consider pulling this code into a separate function, maybe named getAttributesTypeOfMatchingSignatures.

Then maybe you can combine these two functions back into one that takes a boolean to decide whether to return attributes for all candidates or just one.

You can also pull the body of tryGetDefault out as something like getAttributesTypeOfSingleSignature and call that from the loop of getAttributesTypeOfMatchingSignatures.

Then, I think, you can do something like getAttributesTypeOfMatchingSignatures(shouldIncludeAllStatelessAttributesType ? candidatesOutArray ? [callSignature])

sandersn · 2016-11-08T18:32:05Z

src/compiler/checker.ts

+                    const candidatesOutArray: Signature[] = [];
+                    getResolvedJSXStatelessFunctionSignature(openingLikeElement, elementType, candidatesOutArray);
+                    let result: Type;
+                    let defaultResult: Type;


I would rename these variables to allMatchingAttributesType and allAttributesType.

sandersn · 2016-11-08T19:04:13Z

src/compiler/checker.ts

+            return links.resolvedJsxElementAttributesType;
+        }
+
+        function getAllAttributesTypeFromJsxOpeningLikeElement(node: JsxOpeningLikeElement): Type {


can you document that this is meant to be called from the services layer? (if it is — I think that is what you told me.)

sandersn · 2016-11-08T19:09:25Z

src/compiler/checker.ts

+         *      Check assignablity between given attributes property, "attributes" and the target attributes resulted from resolving tag-name
+         * @param openingLikeElement an opening-like JSX element to check its JSXAttributes
+         */
+        function checkJSXAttributesAssignableToTagnameAttributes(openingLikeElement: JsxOpeningLikeElement) {


seems like other names use Jsx instead of JSX.

sandersn · 2016-11-08T19:18:50Z

src/compiler/checker.ts

+            if (isJsxOpeningLikeElement(node)) {
+                // For JSX opening-like element, we will ignore regular arity check (which is what is done here).
+                // Instead, the arity check will be done in "checkApplicableSignatureForJsxOpeningLikeElement" as we are required to figure out
+                // all property inside give attributes.


rewrite: "inside the given attributes"

sandersn · 2016-11-08T19:25:48Z

src/compiler/checker.ts

+                        if (isJsxOpeningOrSelfClosingElement && !checkApplicableSignatureForJsxOpeningLikeElement(<JsxOpeningLikeElement>node, candidate, relation)) {
+                            break;
+                        }
+                        else if (!isJsxOpeningOrSelfClosingElement && !checkApplicableSignature(node, args, candidate, relation, excludeArgument, /*reportErrors*/ false)) {


Normally I like this style, but I think this is actually easier to read as

if (isJsx...) { if (!checkApplicable...) { break; } } else if (!checkApplicable...) { break; }

sandersn · 2016-11-08T19:26:19Z

src/compiler/checker.ts

+         * @param elementType
+         * @param candidatesOutArray
+         */
+        function getResolvedJSXStatelessFunctionSignature(openingLikeElement: JsxOpeningLikeElement, elementType: Type, candidatesOutArray: Signature[]): Signature {


Should use Jsx like other names

mhegazy · 2016-12-30T23:04:39Z

@yuit should this be closed in favour of #12107?

Kanchalai Tanglertsampan added 30 commits October 14, 2016 16:13

Rename resolvedJsxType to be resolvedJSXElementAttributesType for cla…

618da6f

…rification

Add JsxOpeningLikeElement to CallLikeExpression

8d02f9c

Rename parameters in resolveJsxElementAttributesType function

07c60a3

Split get attributes of intrinsic element and custom element

aeb7a3a

Rename getJsxElementAttributesType to getAttributesTypeFromJsxOpening…

d263318

…LikeElement to clarify that it get the attributes type by resolving opening-like element's tagName

Check JsxAttributes of an intrinsic element

cd0aabd

wip-get attributes from opening-like-element attributes Node Check jsx attributes for intrinsic element to base on object-literal logic Change the spreadElementType

Using overload resolution logic when trying to pick a signature of a …

9b8b854

…state-less component wip-resovle custom JSX attributes type wip-using overload resolution to figure out stateless functions wip-complete using resolve signature logic to try picking stateless function

Add and update comments

6b0d648

Update comments Update comment

Clean up prototyping overload-react

fab4a84

Update react.d.ts that is used in testing

790ce8c

Update comment

999a585

Change the type of default attribute (without initializer) to be true…

6d283b0

…Type Update baselines

Add tests: using spread-type in React.Component

2cb95fc

Update baselines: using spread-type in React.Component

a7c405e

Add tests: optional property and overwrite spread property

8d2fcc0

Update baselines: optional property and overwrite spread property

2dc82cd

Update react lib file for testing

2a3b43b

Update baselines from update library test file, react.d.ts

2f18a62

Treat JSXAttribute as declaration; Update .types and .symbol baselines

5d66909

Use contextual type to find-all-reference and goto-definition for JSX…

979f645

…Attribute

Add "(attribute)" to be use in quick-info display for JSXAttribute

df34e29

Update comment why we using contextual type in goto-definition when i…

c38f6c1

…n objectLiteral-like

Update incorrect baseline

5b454e6

Add tsx quick-info tests

33a5d98

Cache testing lib files so we don't have to create source file everytime

f79f6d7

Add tests for stateless-function overload

96a4311

Add tests for stateless-function overload Update tests

Disallow excess property when check jsx attributes

8bac89e

Merge branch 'wip-statelessOverload' of https://github.com/Microsoft/…

cf83268

…TypeScript into wip-statelessOverload

Return the latest tried candidate when choosing overload

9a1ed32

Andy and others added 17 commits November 3, 2016 11:07

Merge pull request #12021 from Microsoft/tslint_version

9705791

Lock tslint version to 4.0.0-dev.0, because 4.0.0-dev.1 complains about unnecessary semicolons following properties

Merge branch 'master' into keyofAndConstraints

48f2b78

Merge pull request #12026 from Microsoft/keyofAndConstraints

702efd5

Fix 'keyof' for constrained type parameters

Get attributes completion for JSX statelss function component

c6a5a56

Update Jsx completion tests to be more restrictive

0027cee

Add Jsx singature helps tests

7cb51d3

Correctly display stateless function in signature help

fb6846d

Ensure transformFlags are correct before visiting a node.

cbec19a

Port #12027, #11980 and #11932 to master (#12037)

1c004bf

* add test for the fix for overwrite emitting error * cr feedback

Merge pull request #12039 from Microsoft/fix12010

defc053

Ensure transformFlags are correct before visiting a node.

Merge pull request #11998 from Microsoft/unusedProperty

4a90614

Mark property referenced in the destructuring as referenced

Add goto-definition tests for JSX and object literal

30afb33

Handle goto-definition of stateless function component

0d4c9fa

Update Jsx completion tests

9a25fa9

Add JSX rename and find all references tests

e188191

Merge branch 'object-spread' into wip-statelessOverload

98062a9

# Conflicts: # src/compiler/checker.ts # src/compiler/types.ts # src/harness/harness.ts # src/services/completions.ts

Update get attributes symbol arrays and report an error when spreadin…

126260b

…g in generic

sandersn reviewed Nov 4, 2016

View reviewed changes

vladima and others added 4 commits November 4, 2016 21:54

add missing bind calls to properly set parent on token nodes (#12057)

ed4fead

Merge branch 'master' into wip-statelessOverload

c52be4a

Merge branch 'object-spread' into wip-statelessOverload

12bb35a

Update baselines. Only allow objec type in spread (e.g no generic)

3f32fa1

sandersn requested changes Nov 7, 2016

View reviewed changes

sandersn requested changes Nov 8, 2016

View reviewed changes

yuit closed this Jan 26, 2017

yuit deleted the wip-statelessOverload branch May 9, 2017 16:04

microsoft locked and limited conversation to collaborators Jun 19, 2018

[WIP] Using overload for JSX stateless function component #11871

[WIP] Using overload for JSX stateless function component #11871

Conversation

yuit commented Oct 26, 2016 • edited Loading

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhegazy commented Dec 30, 2016

yuit commented Oct 26, 2016 •

edited

Loading