build: update to google-closure-compiler@20171023.0.1

[thelgevold]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

Upgrading Closure compiler to the latest version

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[x] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

[alexeagle]

Let's see if it's green - do we need to change closure.conf?

[thelgevold]

@alexeagle It went green after I added externs for some of Angular's internal dependencies.

I have noticed that these externs are sometimes needed, but not always. I guess it depends on whether or not CC encounters these symbols in the source during compilation.

Externs:

/**
* @fileoverview This is an externs file.
* @externs
*/

var System;
var Hammer;
var global;
var COMPILED;

[thelgevold]

The alternative is --jscomp_off=checkVars, but my thinking is that this may mask issues from undeclared variables?

[steveblue]

@thelgevold Hammer is only needed for @angular/animations packages I believe, but System and global are required externs by even the most basic of projects. COMPILED just cropped up prior to 5.0.0 launch in platform-browser.

[Toxicable]

System is used by SystemJsNgModuleLoader.
In other words it's used by the router

[alexeagle]

@thelgevold can you think of a case where we want Closure Compiler to find "issues from undeclared variables", that the earlier TS tooling would not have found?
If there is such a case, maybe we should improve something in the dev roundtrip to find that issue sooner, like a new tslint check.
I think generally the dev mode tooling should include everything needed to say whether or not your program is "valid" in whatever sense TS and Angular agree is per the spec.
The only reason I'd want Closure to output anything at all is in cases where the app works in dev mode but will be broken after optimization, and closure says "hey, if I optimize this app, I'll break it, here's why".
In (all?) the cases I've seen, closure doesn't warn about breaking your app, it's just warning because it expects to be the first tool in the chain that processes your sources.

Given that, my guess is that --jscomp_off=checkVars is the right solution.

/cc @evmar @rkirov

[alexeagle]

We definitely don't want users to have to own such a file. If we decide to pass externs, they should ship with Angular.

[alexeagle]

Need to resolve the externs question...

[thelgevold]

@alexeagle Since TS will always run before this, I agree that --jscomp_off=checkVars is an appropriate solution here. I have updated the PR.

[alexeagle]

FYI, we really should add comments here, but I don't know of a syntax to do so.
If you feel like tackling google/closure-compiler#2413 it would be a nice enhancement

[alexeagle]

thanks!

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}