Skip to content
This repository was archived by the owner on May 1, 2019. It is now read-only.

Fix several compilation errors with Typescript 2.2 #444

Merged
merged 2 commits into from
Feb 7, 2017

Conversation

TimvdLippe
Copy link
Contributor

Installing the analyzer with yarn pulls in Typescript 2.2. This resulted in some compiler errors which I fixed all but one.

In TS 2.2 operators are more thoroughly checked, most notably arithmetic expressions do not allow undefined or null. I fixed these errors.

The last compiler error is because lib.es2017.d.ts in typescript-analyzer.ts pulls in lib.es2016.d.ts which pulls in lib.es2015.d.ts which pulls in lib.es2015.collection.d.ts and that file has a definition of Map<K,V>. This definition forces the second argument value of to be non-null per this commit. I am not sure why the Typescript maintainers introduced this change, since on master value is allowed to be undefined. Therefore I pinned the version of Typescript to 2.1.4 for now.

  • CHANGELOG.md has not been updated, internal change

@TimvdLippe
Copy link
Contributor Author

The corresponding 2.2 PR which has stricter operand checks is microsoft/TypeScript#13483

@@ -28,6 +28,9 @@ function literalToValue(literal: estree.Literal): LiteralValue {
*/
function unaryToValue(unary: estree.UnaryExpression): LiteralValue {
const operand = expressionToValue(unary.argument);
if (operand === null || operand === undefined) {
return operand;
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we want to match javascript semantics here, even when it looks like a mistake. +null === 0 so when we encounter that code the value we should produce is 0.

Recommend changing those three expressions on operator to be like:

+(operator as any)

package.json Outdated
@@ -83,6 +83,6 @@
"shady-css-parser": "0.0.8",
"split": "^1.0.0",
"strip-indent": "^2.0.0",
"typescript": "^2.1.4"
"typescript": "2.1.4"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to just change libraryCache's type to Map<string, string|undefined>, because we want Typescript 2.2's ability to analyze mixins.

@rictic
Copy link
Contributor

rictic commented Feb 7, 2017

LGTM!

@rictic rictic merged commit c62341d into Polymer:master Feb 7, 2017
@TimvdLippe TimvdLippe deleted the typescript-2.2 branch February 7, 2017 00:20
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.

2 participants