build: update to TypeScript 3.4 #29372

filipesilva · 2019-03-18T14:45:27Z

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

What kind of change does this PR introduce?

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

What is the new behavior?

Update to TS 3.4: https://devblogs.microsoft.com/typescript/announcing-typescript-3-4-rc/

Does this PR introduce a breaking change?

Yes
No

Other information

filipesilva · 2019-03-18T14:50:30Z

Similar to #29004

mary-poppins · 2019-03-18T14:52:06Z

You can preview 977f357 at https://pr29372-977f357.ngbuilds.io/.

mary-poppins · 2019-03-18T14:58:22Z

You can preview 16112c9 at https://pr29372-16112c9.ngbuilds.io/.

mary-poppins · 2019-03-18T15:08:35Z

You can preview fb59648 at https://pr29372-fb59648.ngbuilds.io/.

mary-poppins · 2019-03-18T18:23:18Z

You can preview f874d19 at https://pr29372-f874d19.ngbuilds.io/.

mary-poppins · 2019-03-18T18:40:40Z

You can preview f240b7f at https://pr29372-f240b7f.ngbuilds.io/.

filipesilva · 2019-03-18T18:41:35Z

Related to angular/tsickle#1004

rkirov · 2019-03-18T19:11:15Z

packages/compiler-cli/src/ngcc/src/rendering/renderer.ts

@@ -91,7 +91,7 @@ export abstract class Renderer {
    const renderedFiles: FileInfo[] = [];

    // Transform the source files.
-    this.bundle.src.program.getSourceFiles().map(sourceFile => {
+    this.bundle.src.program.getSourceFiles().forEach(sourceFile => {


what happened here?

TS was showing an error here that the return value was not used: https://circleci.com/gh/angular/angular/248206

packages/compiler-cli/src/ngcc/src/rendering/renderer.ts:94:5 - error TS21222: return value is unused. See http://tsetse.info/check-return-value 94 this.bundle.src.program.getSourceFiles().map(sourceFile => { ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 95 const compiledFile = decorationAnalyses.get(sourceFile); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ... 101 } ~~~~~~~ 102 });

Interesting that TS3.4 will trigger that, it was not used before too.

Interesting indeed. Now that I look at it with fresh eyes, this isn't a TypeScript error proper, but a TseTse error: https://github.com/bazelbuild/rules_typescript/blob/b4bc468bfb03c05b68dde0b4f846bcd1a8fb7b01/internal/tsetse/tests/check_return_value/BUILD#L19

I wonder why TseTse wasn't catching this before.

cc @alexeagle

my guess is that we only look for usages of Array#map, while program.getSourceFiles() maybe was some other iterable type before like ReadonlyArray I guess now it became assignable to Array?

filipesilva · 2019-03-19T08:38:26Z

aio/src/app/shared/reporting-error-handler.ts

-      this.window.onerror(error);
-    } else {
-      this.window.onerror(error.message, undefined, undefined, undefined, error);
+    if (this.window.onerror) {


TS 3.4 was failing compilation with the error below https://circleci.com/gh/angular/angular/248678

ERROR in src/app/shared/reporting-error-handler.ts(32,7): error TS2721: Cannot invoke an object which is possibly 'null'. src/app/shared/reporting-error-handler.ts(34,7): error TS2721: Cannot invoke an object which is possibly 'null'.

filipesilva · 2019-03-19T08:39:14Z

aio/tests/e2e/onerror.e2e-spec.ts


  async function callOnError(message, url, line, column, error) {
    await browser.executeScript(function() {
      // reset the ga queue
      (window as any).ga.q.length = 0;
      // post the error to the handler
-      window.onerror(arguments[0], arguments[1], arguments[2], arguments[3], arguments[4]);
+      if (window.onerror) {


TS was failing compilation with the error below https://circleci.com/gh/angular/angular/248820

[18:29:57] E/launcher - Error: TSError: ⨯ Unable to compile TypeScript tests/e2e/onerror.e2e-spec.ts (193,7): Cannot invoke an object which is possibly 'null'. (2721)

filipesilva · 2019-03-19T08:41:05Z

aio/tests/e2e/onerror.e2e-spec.ts

@@ -1,5 +1,5 @@
-import { browser } from 'protractor';
-import { SitePage } from './app.po';
+import {browser} from 'protractor';


Saving this file triggered the clang auto format. I could also save without formatting, but that begs the question of why this wasn't formatted to begin with.

The files under aio/ are not formatted with clang-format (when using gulp format for example).
Some of the files definitely should not be formatted (e.g. aio/content/examples/, which should follow the Angular Style Guide) and although the app files themselves (in aio/src/) could be formatted, they historically haven't been 😁

(Note: The annoying auto-formatting will go away, once I find some time to clean-up #28842 😇)

Saved without formatting then, thanks for the context.

mary-poppins · 2019-03-19T09:27:11Z

You can preview b9bb9df at https://pr29372-b9bb9df.ngbuilds.io/.

RinMinase · 2019-04-03T02:24:00Z

@filipesilva Typescript 3.4.1 is out, but your package dependencies file states ~3.4.0-rc, would this be applicable to use ^3.4.1 or would this introduce another set of breaking changes?

filipesilva · 2019-04-03T09:47:34Z

@RinMinase this PR was just an initial test when the RC came out to see how much work would be necessary on this repository. We will change it to support a stable version before merging.

alexeagle · 2019-04-05T22:14:29Z

aio/package.json

@@ -157,7 +157,7 @@
    "tree-kill": "^1.1.0",
    "ts-node": "^3.3.0",
    "tslint": "~5.9.1",
-    "typescript": "~3.3.3333",
+    "typescript": "~3.4.0-rc",


we are ready to merge this, could you update this to the released 3.4?

ngbot · 2019-04-05T22:16:07Z

I see that you just added the PR action: merge label, but the following checks are still failing:
     conflicts with base branch "master"
     missing required labels: PR target: *
     status "google3" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

alexeagle · 2019-04-05T22:16:56Z

caretaker Igor: on your plate to clean this up and check the peerDep ranges everywhere

mary-poppins · 2019-04-06T11:33:41Z

You can preview 86aebb3 at https://pr29372-86aebb3.ngbuilds.io/.

filipesilva · 2019-04-10T13:30:46Z

integration/dynamic-compiler/package.json

@@ -5,7 +5,8 @@
  "scripts": {
    "build": "npm run clean && npm run ngc && npm run rollup && npm run rollup:lazy && npm run es5 && npm run es5:lazy",
    "clean": "rm -rf dist",
-    "es5": "tsc --target es5 --skipLibCheck --allowJs dist/bundle.es2015.js --out dist/bundle.js",
+    "//es5-note": "`--typeRoots ./dummy_folder_name/@types` is a workaround for https://github.com/Microsoft/TypeScript/issues/30845",
+    "es5": "tsc --target es5 --skipLibCheck --allowJs dist/bundle.es2015.js --out dist/bundle.js --typeRoots ./dummy_folder_name/@types",


--typeRoots ./dummy_folder_name/@types is a workaround for microsoft/TypeScript#30845.

This test does not depend on @types/node, but they are present in the root node_modules, both via a direct dependency and transitively through the rollup dependency.

mary-poppins · 2019-04-10T13:38:10Z

You can preview 5835c4a at https://pr29372-5835c4a.ngbuilds.io/.

alexeagle

thanks @filipesilva nice work 🌹

IgorMinar

Other than the one missed version everything else looks great. Thank you.

IgorMinar · 2019-04-10T16:08:16Z

integration/bazel-schematics/test.sh

@@ -30,7 +30,7 @@ function testBazel() {
  yarn
  # Force more recent TS version until new Angular CLI projects also use it.
  # --ignore-scripts is necessary because there is a postinstall script that uses ngc.
-  yarn add typescript@3.3.3333 --dev --ignore-scripts
+  yarn add typescript@3.4.0-rc --dev --ignore-scripts


Why not 3.4.2? Did you miss to update this file?

Yes, I missed this one, thank you for catching it 👍

Fixed now.

mary-poppins · 2019-04-10T18:36:28Z

You can preview 34c815b at https://pr29372-34c815b.ngbuilds.io/.

PR Close angular#29372

angular-automatic-lock-bot · 2019-09-14T22:53:37Z

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.}

googlebot added the cla: yes label Mar 18, 2019

rkirov reviewed Mar 18, 2019

View reviewed changes

filipesilva commented Mar 19, 2019

View reviewed changes

mhevery added the area: build & ci label Mar 19, 2019

ngbot bot added this to the needsTriage milestone Mar 19, 2019

alexeagle reviewed Apr 5, 2019

View reviewed changes

alexeagle added the merge: caretaker note label Apr 5, 2019

ngbot bot added the action: merge label Apr 5, 2019

filipesilva added the target: major label Apr 6, 2019

Airblader mentioned this pull request Apr 8, 2019

Support Typescript 3.4 #29766

Closed

filipesilva marked this pull request as ready for review April 10, 2019 13:28

filipesilva requested review from a team as code owners April 10, 2019 13:28

filipesilva commented Apr 10, 2019

View reviewed changes

alexeagle approved these changes Apr 10, 2019

View reviewed changes

IgorMinar approved these changes Apr 10, 2019

View reviewed changes

build: update to TypeScript 3.4

34c815b

IgorMinar closed this in ef85336 Apr 10, 2019

wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019

build: update to TypeScript 3.4 (angular#29372)

037bc46

PR Close angular#29372

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: update to TypeScript 3.4 #29372

build: update to TypeScript 3.4 #29372

filipesilva commented Mar 18, 2019 •

edited

Loading

filipesilva commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

filipesilva commented Mar 18, 2019

rkirov Mar 18, 2019

filipesilva Mar 18, 2019

rkirov Mar 19, 2019

filipesilva Mar 19, 2019

alexeagle Apr 3, 2019

filipesilva Mar 19, 2019

filipesilva Mar 19, 2019

filipesilva Mar 19, 2019

gkalpak Mar 19, 2019

filipesilva Mar 19, 2019

mary-poppins commented Mar 19, 2019

RinMinase commented Apr 3, 2019 •

edited

Loading

filipesilva commented Apr 3, 2019

alexeagle Apr 5, 2019

ngbot bot commented Apr 5, 2019

alexeagle commented Apr 5, 2019

mary-poppins commented Apr 6, 2019

filipesilva Apr 10, 2019

mary-poppins commented Apr 10, 2019

alexeagle left a comment

IgorMinar left a comment

IgorMinar Apr 10, 2019

filipesilva Apr 10, 2019

mary-poppins commented Apr 10, 2019

angular-automatic-lock-bot bot commented Sep 14, 2019

build: update to TypeScript 3.4 #29372

build: update to TypeScript 3.4 #29372

Conversation

filipesilva commented Mar 18, 2019 • edited Loading

PR Checklist

PR Type

What is the new behavior?

Does this PR introduce a breaking change?

Other information

filipesilva commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

mary-poppins commented Mar 18, 2019

filipesilva commented Mar 18, 2019

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

mary-poppins commented Mar 19, 2019

RinMinase commented Apr 3, 2019 • edited Loading

filipesilva commented Apr 3, 2019

Choose a reason for hiding this comment

ngbot bot commented Apr 5, 2019

alexeagle commented Apr 5, 2019

mary-poppins commented Apr 6, 2019

Choose a reason for hiding this comment

mary-poppins commented Apr 10, 2019

alexeagle left a comment

Choose a reason for hiding this comment

IgorMinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mary-poppins commented Apr 10, 2019

angular-automatic-lock-bot bot commented Sep 14, 2019

filipesilva commented Mar 18, 2019 •

edited

Loading

RinMinase commented Apr 3, 2019 •

edited

Loading