refactor: enable Ivy and fix template type checking issues #17392

Splaktar · 2019-10-14T14:24:36Z

Changes

enable Ivy and fix template type checking in Bazel
- for libraries, examples, dev-app, CI
- add postinstall task for ivy-ngcc and configure it for our SystemJS setup
pin to Angular 9.0.0-next.10 for this PR
- Angular 9.0.0-next.11 brings in a lot of new template type checking errors
fix buggy show bounding box checkbox in connected-overlay-demo
fix stylelint to work with WebStorm plugin
restrict NodeJS version to 10.x
- We can't go to NodeJS 12.x without dropping or upgrading Gulp to 4.x

API Changes

export ProgressBarMode type from mdc-progress-bar and mat-progress-bar
export MatDrawerMode type from mat-drawer
Add | null to mat-tab-body's @Input() origin: number;

TODO

Remove need for any on @Input() stepControl: FormControlLike | any; in CdkStepper: stepper: Type 'FormGroup' is not assignable to type 'FormControlLike' #16606
Figure out how to resolve "Property 'focused' does not exist on type 'HTMLInputElement'." when binding to the focused property of a matInput through a template variable.
Figure out how to bind value field from template variables
remove (observableX | async)! workaround after cdkVirtualForOf is updated with better typing like ngForOf: ivy: cdkVirtualForOf template type errors #17411. Fixed in PR fix(cdk/scrolling): expand type for "cdkVirtualForOf" input to work with strict null checks #17421.
Figure out how to load ngModules in the browser after a successful Ivy compilation. Related issue: Uncaught exception using ivy angular#31314 (comment)
Figure out how to bind to [value] on a datepicker input w/o the need for any. This may be a conflict/bug with template type checking. There is some discussion of this here.
Determine what to do about the hundreds of template type checking errors for boolean and other @Inputs caused by Angular 9.0.0-next.11. Related discussion: fix(ivy): do not always accept undefined for directive inputs angular#33066 (comment) and FW-1475.

Notes

Settting "fullTemplateTypeCheck": false for dev-app doesn't remove the need for many of the changes in this PR. false here means, do partial template type checking. It doesn't turn off template type checking.
- Similarly "fullTemplateTypeCheck": false does not resolve the hundreds of errors from Angular 9.0.0-next.11.

Other related issues

QueryList is not iterable and can't be used directly in *ngFor with strict template type checking: QueryList is not iterable angular#29842

Fixes #16606. Closes #16373.

Splaktar · 2019-10-16T09:22:11Z

It looks like the issue with the page not loading due to moduleDef undefined errors, may be related to this

 --create-ivy-entry-points  If specified then new `*_ivy_ngcc` entry-points
                             will be added to package.json rather than modifying
                             the ones in-place.
                             For this to work you need to have custom resolution
                             set up (e.g. in webpack) to look for these new
                             entry-points.
                             The Angular CLI does this already, so it is safe to
                             use this option if the project is being built via
                             the CLI.

We haven't set up any custom resolution in our SystemJS configuration AFAIK.

Update: Yep, removing this --create-ivy-entry-points option and using only main as a property fixed this issue.

package.json

jelbourn

All the changes in here too good overall. The Angular version thing will conflict w/ my TypeScript 3.6 update when that's ready, since I'll need a higher version of Angular. I think we'll be able to move forward with fullTemplateTypeCheck = false for the time being.

jelbourn · 2019-10-17T19:03:33Z

.bazelrc

-# does not generate factory files which are needed for AOT.
-build --define=compile=legacy
+# Use the Ivy AOT compiler strategy. We want to compile with Ivy and with "ngtsc".
+build --define=compile=aot --keep_going=true


What are the required steps if someone wants to test something in ViewEngine?

To build for ViewEngine, you can use --define=compile=legacy here instead. I think that @devversion has some ideas on how to make this more configurable and hopefully add NPM scripts to the package.json for building with Ivy (default) or ViewEngine.

I think that the deploy scripts will need to be changed to always build with ViewEngine since, in Angular version 9, that's the only way to publish libraries.

package.json

src/a11y-demo/checkbox/checkbox-a11y.ts

src/dev-app/datepicker/datepicker-demo.ts

src/material/tabs/tab-group.html

.bazelrc

.circleci/config.yml

devversion · 2019-10-17T20:27:15Z

package.json

    "yarn": ">= 1.17.3"
  },
  "scripts": {
-    "postinstall": "node --preserve-symlinks --preserve-symlinks-main tools/bazel/postinstall-patches.js",
+    "postinstall": "node --preserve-symlinks --preserve-symlinks-main tools/bazel/postinstall-patches.js | ivy-ngcc --properties main",


It might be good to only do this when running within Bazel. ngcc is not very fast and otherwise we are running ngcc twice. We can tackle this in a follow-up though.

src/dev-app/checkbox/checkbox-demo.ts

src/dev-app/connected-overlay/connected-overlay-demo.ts

devversion · 2019-10-17T20:32:08Z

src/dev-app/datepicker/datepicker-demo.html

+          e.g. [value] is set for MatInput and MatDatepicker.
+          MatInput#value expects a string whereas MatDatepicker#value expects a Date.
+          This breaks strict template type checking. What should we do here?
+    -->


Re-opening a comment here again (we had one in the other PR for this). I'm not sure if anything has changed here. Not even sure if we have a ticket for it. I don't think we talked about it in the framework meeting yet.

Jeremy spoke to Alex R about it. It sounds like this is a real issue and that Alex was going to investigate a fix. We should probably open a ticket in JIRA?

Splaktar · 2019-10-18T07:58:13Z

I've got a WIP PR open that contains these changes plus 731b80d which starts to resolve all of the next.11 template type checking errors with "fullTemplateTypeCheck": true,.

Splaktar · 2019-10-21T18:10:05Z

It looks like @angular/schematics@9.0.0-next.12 requires TypeScript 3.6. I'm reverting the version of the Angular CLI related libraries to resolve this.

Splaktar · 2019-10-21T18:52:02Z

I had to revert Angular 9.0.0-next.11 as well as it runs into strange issues with ivy_test failing due to npm or yarn errors.

Splaktar · 2019-10-21T19:33:13Z

OK, hopefully this last push resolves the issues with the public_api_guard and updates the goldens so that everything is finally green.

crisbeto

LGTM

tools/defaults.bzl

src/dev-app/connected-overlay/connected-overlay-demo.ts

- enable Ivy and fix template type checking in Bazel - for libraries, examples, dev-app, CI - add `postinstall` task for `ivy-ngcc` and configure it for our SystemJS setup - pin to Angular `9.0.0-next.10` for this PR - Angular `9.0.0-next.11` brings in a lot of new template type checking errors - fix buggy show bounding box checkbox in connected-overlay-demo - fix stylelint to work with WebStorm plugin - restrict NodeJS version to 10.x - We can't go to NodeJS 12.x without dropping or upgrading Gulp to 4.x - export `ProgressBarMode` type from `mdc-progress-bar` and `mat-progress-bar` - export `MatDrawerMode` type from `mat-drawer` - Add `| null` to `mat-tab-body`'s `@Input() origin: number;` Fixes #16606. Closes #16373.

angular-automatic-lock-bot · 2019-11-22T00:06:04Z

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

Splaktar added in progress This issue is currently in progress refactoring This issue is related to a refactoring labels Oct 14, 2019

Splaktar self-assigned this Oct 14, 2019

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Oct 14, 2019

Splaktar force-pushed the devApp-enableIvy branch 4 times, most recently from 96a2981 to a2c0f5c Compare October 15, 2019 19:12

Splaktar mentioned this pull request Oct 15, 2019

stepper: Type 'FormGroup' is not assignable to type 'FormControlLike' #16606

Closed

Splaktar force-pushed the devApp-enableIvy branch 4 times, most recently from 3cba263 to 2adff8a Compare October 16, 2019 00:46

Splaktar force-pushed the devApp-enableIvy branch from 2adff8a to 813991a Compare October 16, 2019 18:33

devversion reviewed Oct 17, 2019

View reviewed changes

package.json Outdated Show resolved Hide resolved

Splaktar force-pushed the devApp-enableIvy branch 5 times, most recently from 32fe812 to e580a12 Compare October 17, 2019 18:09

Splaktar marked this pull request as ready for review October 17, 2019 18:41

Splaktar requested review from andrewseguin, crisbeto, jelbourn, mmalerba and a team as code owners October 17, 2019 18:41

jelbourn reviewed Oct 17, 2019

View reviewed changes

crisbeto reviewed Oct 17, 2019

View reviewed changes

devversion reviewed Oct 17, 2019

View reviewed changes

Splaktar force-pushed the devApp-enableIvy branch from a67fe70 to 14bb2f9 Compare October 18, 2019 04:54

Splaktar added this to the 9.0.0 milestone Oct 18, 2019

Splaktar added the P1 Impacts a large percentage of users; if a workaround exists it is partial or overly painful label Oct 18, 2019

Splaktar force-pushed the devApp-enableIvy branch 2 times, most recently from a9d7cf3 to a4546e5 Compare October 18, 2019 05:35

Splaktar mentioned this pull request Oct 18, 2019

refactor: attempt to resolve @Input template type errors #17441

Closed

Splaktar force-pushed the devApp-enableIvy branch 2 times, most recently from 54a3373 to 551e3f0 Compare October 21, 2019 17:53

Splaktar force-pushed the devApp-enableIvy branch 3 times, most recently from 03b0db8 to 8d123fd Compare October 21, 2019 18:49

Splaktar force-pushed the devApp-enableIvy branch 2 times, most recently from 106dc06 to a338aa2 Compare October 21, 2019 19:31

Splaktar force-pushed the devApp-enableIvy branch from a338aa2 to fb3dd42 Compare October 21, 2019 19:34

Splaktar added target: major This PR is targeted for the next major release and removed in progress This issue is currently in progress labels Oct 21, 2019

crisbeto approved these changes Oct 21, 2019

View reviewed changes

devversion approved these changes Oct 21, 2019

View reviewed changes

tools/defaults.bzl Outdated Show resolved Hide resolved

src/dev-app/connected-overlay/connected-overlay-demo.ts Show resolved Hide resolved

mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Oct 21, 2019

Splaktar force-pushed the devApp-enableIvy branch from fb3dd42 to 8f0f73c Compare October 22, 2019 06:38

mmalerba merged commit db283f1 into master Oct 22, 2019

mmalerba deleted the devApp-enableIvy branch October 22, 2019 18:05

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: enable Ivy and fix template type checking issues #17392

refactor: enable Ivy and fix template type checking issues #17392

Splaktar commented Oct 14, 2019 •

edited

Splaktar commented Oct 16, 2019 •

edited

jelbourn left a comment

jelbourn Oct 17, 2019

Splaktar Oct 18, 2019

devversion Oct 17, 2019

devversion Oct 17, 2019

Splaktar Oct 18, 2019

Splaktar commented Oct 18, 2019

Splaktar commented Oct 21, 2019 •

edited

Splaktar commented Oct 21, 2019

Splaktar commented Oct 21, 2019

crisbeto left a comment

angular-automatic-lock-bot bot commented Nov 22, 2019

refactor: enable Ivy and fix template type checking issues #17392

refactor: enable Ivy and fix template type checking issues #17392

Conversation

Splaktar commented Oct 14, 2019 • edited

Changes

API Changes

TODO

Notes

Other related issues

Splaktar commented Oct 16, 2019 • edited

jelbourn left a comment

Choose a reason for hiding this comment

jelbourn Oct 17, 2019

Choose a reason for hiding this comment

Splaktar Oct 18, 2019

Choose a reason for hiding this comment

devversion Oct 17, 2019

Choose a reason for hiding this comment

devversion Oct 17, 2019

Choose a reason for hiding this comment

Splaktar Oct 18, 2019

Choose a reason for hiding this comment

Splaktar commented Oct 18, 2019

Splaktar commented Oct 21, 2019 • edited

Splaktar commented Oct 21, 2019

Splaktar commented Oct 21, 2019

crisbeto left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Nov 22, 2019

Splaktar commented Oct 14, 2019 •

edited

Splaktar commented Oct 16, 2019 •

edited

Splaktar commented Oct 21, 2019 •

edited