Upgrade angular 9 #2234

timdeschryver · 2019-11-08T18:02:21Z

PR Checklist

Please check if your PR fulfills the following requirements:

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

PR Type

What kind of change does this PR introduce?

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

What is the current behavior?

Closes #2211

What is the new behavior?

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Other information

modules/schematics/src/container/index.ts

timdeschryver · 2019-11-08T18:08:39Z

modules/effects/spec/types/effect_creator.spec.ts

@@ -47,7 +47,7 @@ describe('createEffect()', () => {
      expectSnippet(`
        const effect = createEffect(() => ({ foo: 'a' }), { dispatch: false });
      `).toFail(
-        /Type '{ foo: string; }' is not assignable to type 'Observable<{}> | ((...args: any[]) => Observable<{}>)'./
+        /Type '{ foo: string; }' is not assignable to type 'Observable<unknown> | ((...args: any[]) => Observable<unknown>)'./


I have to take a look why this happens

Did you sync? :)
f70600f

This seems to be because TS is upgraded.
It only occurs when a non-observable is returned.

For a non-dispatching effect that's returning an observable, the effect has the correct returntype:

loginSuccess$ = createEffect( () => this.actions$.pipe( ofType(AuthApiActions.loginSuccess), tap(() => this.router.navigate(['/'])) ), { dispatch: false } );

Returns

AuthEffects.loginSuccess$: Observable<{ user: User; } & TypedAction<"[Auth/API] Login Success">> & CreateEffectMetadata

timdeschryver · 2019-11-08T18:10:22Z

projects/example-app/tsconfig.app.json

@@ -10,7 +11,7 @@
    "lib": ["es2017", "dom"],
    "outDir": "../out-tsc/app",
    "target": "es5",
-    "module": "es2015",
+    "module": "esnext",


To be able to use the dynamic import syntax:

import('@example-app/books/books.module').then(m => m.BooksModule),

timdeschryver · 2019-11-08T18:11:21Z

projects/example-app/tsconfig.app.json

@@ -1,4 +1,5 @@
 {
+  "extends": "../../tsconfig.json",


Otherwise, we had problems to run the example app:

ERROR in Unable to write a reference to EffectsRootModule

Followed the following doc https://hackmd.io/zhu28RUcRiaG0DZq5bLy1A?view

angular.json

timdeschryver · 2019-11-08T18:12:55Z

Tests and example app are working.
Will take a look at bazel now.

timdeschryver · 2019-11-09T19:20:45Z

package.json

-    "@angular-devkit/build-angular": "~0.800.0",
-    "@bazel/bazel": "0.26.0",
+    "@angular-devkit/build-angular": "~0.900.0-rc.0",
+    "@bazel/bazel": "1.0.0",


Perhaps I took it too far here to also upgrade bazel, I can revert/create a separate PR for it.
Having some troubles to get it working during CI tho, and I'm not able to reproduce it locally.

Probably need to bump the cache key here for CI to 12 https://github.com/ngrx/platform/blob/master/.circleci/config.yml#L13

Should we modify the cache key from the master branch?

I tried bumping the version of this branch, but with the same result.
Seems related to bazelbuild/bazel#9366 and bazel-contrib/rules_nodejs#1027, I already tried re-running multiple times.

alex-okrushko · 2019-11-11T17:45:05Z

modules/effects/spec/types/effect_creator.spec.ts

@@ -47,7 +47,7 @@ describe('createEffect()', () => {
      expectSnippet(`
        const effect = createEffect(() => ({ foo: 'a' }), { dispatch: false });
      `).toFail(
-        /Type '{ foo: string; }' is not assignable to type 'Observable<{}> | ((...args: any[]) => Observable<{}>)'./
+        /Type '{ foo: string; }' is not assignable to type 'Observable<unknown> | ((...args: any[]) => Observable<unknown>)'./


Did you sync? :)
f70600f

projects/example-app/src/app/auth/components/logout-confirmation-dialog.component.spec.ts

projects/example-app/src/app/auth/containers/login-page.component.spec.ts

BREAKING CHANGE: Must use Angular 9

Ref: angular/angular-cli@e92c46a

timdeschryver · 2019-11-11T19:07:55Z

@alex-okrushko I did sync again but it's still inferring to unknown.

About material, did you take a look at a specific commit?
They should be all modified to the specific import, the two you highlighted are already resolved.

brandonroberts · 2019-11-11T22:12:49Z

We'll need to add enableIvy: false to the tsconfig-build.json files for each package so we're still building against View Engine for the npm packages.

alex-okrushko · 2019-11-12T09:17:57Z

@alex-okrushko I did sync again but it's still inferring to unknown.

About material, did you take a look at a specific commit?
They should be all modified to the specific import, the two you highlighted are already resolved.

Am I looking at the wrong place? Both imports were resolved by importing general MaterialModule instead of specific imports.

timdeschryver · 2019-11-12T10:38:40Z

@alex-okrushko it's the material module from @example-app (not from @material).
I can use the specific import for our spec files if you'd like.

alex-okrushko · 2019-11-12T10:49:36Z

Thanks @timdeschryver ! Looks like I was looking at the wrong place :)

brandonroberts · 2019-11-12T12:40:40Z

This file needs to be updated also https://github.com/ngrx/platform/blob/master/tools/defaults.bzl with version 9

ngrxbot · 2019-11-12T18:38:19Z

Preview docs changes for 4bbde2a at https://previews.ngrx.io/pr2234-4bbde2a/

timdeschryver · 2019-11-12T18:59:56Z

.circleci/config.yml

@@ -169,14 +175,14 @@ jobs:
      - run: npm rebuild node-sass
      - run: yarn build-for next --progress false --base-href /pr$CIRCLE_PULL_REQUEST_NUMBER-$SHORT_GIT_HASH/ --output-path dist/ngrx.io/pr$CIRCLE_PULL_REQUEST_NUMBER-$SHORT_GIT_HASH/ && yarn copy-404-page
      - run: cp -rf src/extra-files/next/. dist/ngrx.io/pr$CIRCLE_PULL_REQUEST_NUMBER-$SHORT_GIT_HASH/
-      - run: yarn --cwd ../../ install && yarn --cwd ../../ run deploy:preview
+      - run: yarn --cwd ../../ install --ignore-engines && yarn --cwd ../../ run deploy:preview


Added --ignore-engines because we have a version mismatch between the docs and the rest of the packages, otherwise the install command would fail.

timdeschryver · 2019-11-12T20:18:24Z

I reached a point where I could use some guidance to fix the errors during the CI 😅.

brandonroberts · 2019-11-12T22:20:53Z

😂 I'll take a look

It isn't part of the migration see angular/angular#33727 and angular/angular-cli#16145

timdeschryver · 2019-11-16T15:26:19Z

Seems like some builds are now succeeding after the decrease of jobs 🤔
(The update to rc.2 didn't resolve the CI issues)

brandonroberts · 2019-12-06T02:21:25Z

@timdeschryver is this ready to merge? I guess we'll see how CI holds up after it lands.

sumitparakh · 2019-12-06T03:11:25Z

I'll continue with following issue once this PR is merged..
#2240

timdeschryver · 2019-12-08T17:06:40Z

@brandonroberts It's ready (I just pushed a commit to upgrade to the latest rc).
The only problem is the CI... 😅

timdeschryver · 2019-12-08T20:13:26Z

(Had to retrigger the CI in order to get it green)

sumitparakh · 2020-01-07T14:14:35Z

Great!. Will work on #2240 now.

timdeschryver commented Nov 8, 2019

View reviewed changes

angular.json Show resolved Hide resolved

timdeschryver force-pushed the upgrade-angular-9 branch 3 times, most recently from da1128a to f51f623 Compare November 9, 2019 17:28

timdeschryver commented Nov 9, 2019

View reviewed changes

brandonroberts mentioned this pull request Nov 11, 2019

Platform: Update usage of TestBed.get to TestBed.inject #2240

Closed

3 tasks

alex-okrushko requested changes Nov 11, 2019

View reviewed changes

timdeschryver added 6 commits November 11, 2019 19:40

chore: update deps to Angular 9

8795211

BREAKING CHANGE: Must use Angular 9

chore(example): upgdate deps to Angular 9

6592755

build(ci): update node to 12.9.1

9cf4e29

build: update bazel

c5e955c

chore: bump circleci cache

bbde03d

fix(schematics): use skipTests, spec is deprecated

442539b

Ref: angular/angular-cli@e92c46a

timdeschryver force-pushed the upgrade-angular-9 branch from 9f094d5 to 442539b Compare November 11, 2019 18:42

chore: add bazel-bin to gitignore

9672c24

build: build using the angular view engine

6e92f23

build: bump angular and rxjs versions

77040f5

timdeschryver force-pushed the upgrade-angular-9 branch 5 times, most recently from 2af1101 to 6bd86b0 Compare November 12, 2019 16:36

timdeschryver force-pushed the upgrade-angular-9 branch 2 times, most recently from db0e4d1 to b5b0874 Compare November 12, 2019 18:10

ci: build docs with node v10.12

0b7ac9f

timdeschryver force-pushed the upgrade-angular-9 branch from b5b0874 to 0b7ac9f Compare November 12, 2019 18:29

timdeschryver commented Nov 12, 2019

View reviewed changes

brandonroberts and others added 3 commits November 12, 2019 21:36

chore: decrease jobs

da7c575

chore: update to rc.2

31f1a4d

fix: remove ngcc postinstall hook

b3372ce

It isn't part of the migration see angular/angular#33727 and angular/angular-cli#16145

timdeschryver added the 9.x label Nov 18, 2019

chore: update to rc.5

4bbde2a

timdeschryver marked this pull request as ready for review December 8, 2019 17:06

brandonroberts approved these changes Jan 7, 2020

View reviewed changes

brandonroberts merged commit b146af5 into master Jan 7, 2020

timdeschryver deleted the upgrade-angular-9 branch January 30, 2020 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade angular 9 #2234

Upgrade angular 9 #2234

timdeschryver commented Nov 8, 2019 •

edited

Loading

timdeschryver Nov 8, 2019

alex-okrushko Nov 11, 2019

timdeschryver Nov 12, 2019

timdeschryver Nov 8, 2019

timdeschryver Nov 8, 2019

timdeschryver commented Nov 8, 2019

timdeschryver Nov 9, 2019

brandonroberts Nov 11, 2019

timdeschryver Nov 11, 2019 •

edited

Loading

alex-okrushko Nov 11, 2019

timdeschryver commented Nov 11, 2019

brandonroberts commented Nov 11, 2019

alex-okrushko commented Nov 12, 2019

timdeschryver commented Nov 12, 2019

alex-okrushko commented Nov 12, 2019

brandonroberts commented Nov 12, 2019 •

edited

Loading

ngrxbot commented Nov 12, 2019 •

edited

Loading

timdeschryver Nov 12, 2019

timdeschryver commented Nov 12, 2019

brandonroberts commented Nov 12, 2019

timdeschryver commented Nov 16, 2019

brandonroberts commented Dec 6, 2019

sumitparakh commented Dec 6, 2019 •

edited

Loading

timdeschryver commented Dec 8, 2019

timdeschryver commented Dec 8, 2019

sumitparakh commented Jan 7, 2020

Upgrade angular 9 #2234

Upgrade angular 9 #2234

Conversation

timdeschryver commented Nov 8, 2019 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

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

timdeschryver commented Nov 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timdeschryver Nov 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timdeschryver commented Nov 11, 2019

brandonroberts commented Nov 11, 2019

alex-okrushko commented Nov 12, 2019

timdeschryver commented Nov 12, 2019

alex-okrushko commented Nov 12, 2019

brandonroberts commented Nov 12, 2019 • edited Loading

ngrxbot commented Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

timdeschryver commented Nov 12, 2019

brandonroberts commented Nov 12, 2019

timdeschryver commented Nov 16, 2019

brandonroberts commented Dec 6, 2019

sumitparakh commented Dec 6, 2019 • edited Loading

timdeschryver commented Dec 8, 2019

timdeschryver commented Dec 8, 2019

sumitparakh commented Jan 7, 2020

timdeschryver commented Nov 8, 2019 •

edited

Loading

timdeschryver Nov 11, 2019 •

edited

Loading

brandonroberts commented Nov 12, 2019 •

edited

Loading

ngrxbot commented Nov 12, 2019 •

edited

Loading

sumitparakh commented Dec 6, 2019 •

edited

Loading