Upgrade Angular, Node.js, and related dependencies #1243

maurizi · 2019-07-12T20:20:33Z

Overview

The main effort in this PR is to upgrade Angular from version 4 to 8.

As a consequence of that effort, the following other changes were also made:

Node.js upgraded from 6 to 10
ngx-bootstrap updated to the latest version, instead of using our forked version
Bootstrap updated from version 3 to 4
The climate-change-components repository was copied into this repository, instead of remaining a separate dependency, due to difficulties in upgrading and integrating with the new Angular CLI support for library projects
The difflib dependency (used by the password validation popup) was copied into this project with minor fixes, due to an error which was not fatal in our old CLI toolchain but became so in the newer toolchain.

Demo

N/A

Everything should continue to work as it did before, with no changes in behavior or styling.

Testing Instructions

scripts/update
scripts/server
Test everything in the front-end application (you should be able to skip the Django Admin site). Make a small change and verify that live reload works.
Access the production build at localhost:8000 by stopping scripts/server and running docker-compose up django nginx
Test everything in the application again using the production build.

Is the CHANGELOG.md updated with any relevant additions, changes, fixes or removals following the format of keepachangelog?

Closes #1202
Closes #263
Closes #876

Pre-upgrade steps from https://update.angular.io/#4.4:8.0.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-7.html#strict-class-initialization

http module moved into core.

See: angular/angular-cli#14553 (comment) Fixes build error "TS2554: Expected 2 arguments, but got 1."

Both are in compiler instead.

See: https://angular.io/guide/deprecations

Still on 2.x

FIXME: remove when upgrade complete.

This allows the temperate Angular app to successfully compile. PlanItApiHttp was removed in favor of plain old HttpClient. PlanItApiHttp was causing cryptic errors when requests were made, such as: 'XMLHttpRequest failed to open with scheme: HTTP://LOCALHOST:8081/API/USER/' Why caps? And why is that a scheme? Now we just get normal 401 authorized failures because we need to implement an HTTP Interceptor to add the token to all API requests.

- Adds token to API requests - Redirects to home page in case of auth failure

So that we match the old Azavea ngx-bootstrap typeaheadMinLength=0 behavior.

…etc) The token interceptor was trying to add an authorization token even when there was no logged in user, causing requests to be rejected.

The newer version Angular we're upgrading to strips whitespace. In the few places where we were depending on whitespace to add padding to inline elements, I've explicitly added non-breaking spaces to accommodate.

Without also setting [typeaheadSelectFirstItem]="false", tabbing out of the input won't work with [typeaheadIsFirstItemActive]="false" set.

This library was failing in AoT mode, due to attempting to assign to the '.name' property of an anonymous function. It is unclear to me why this was not an issue before, but is now. Regardless, since the library was last updated 7 years ago and has several similarly old unmerged pull requests, it seemed like the best solution was to copy it into our project and fix it there rather than attempting to fork it and submit a fix. to fork it and submit a pull

maurizi

I'm on the fence on if it's worth it to reformat the indentation in the copied-over components code - that mostly used 4-space indentation, while Temperate uses 2-space indentation.

If folks have strong opinions, please leave a 👍 or 👎

src/angular/planit/src/app/climate-api/api/models/historic-range.model.ts

src/angular/planit/src/app/climate-api/shared/index.ts

src/angular/planit/src/app/core/services/account-create.service.ts

ddohler

I successfully compiled the app and logged in, and I tested as much functionality as I could. I found one unrelated bug, and one thing that didn't seem to function as it should: on the indicator graphs, changing the Scenario, Dataset, and Model wasn't causing the graph to update for me. I also confirmed that live reload worked.

ddohler · 2019-07-16T13:16:42Z

src/angular/planit/src/app/action-steps/action-steps-overview.component.ts

@@ -3,7 +3,7 @@ import { ActivatedRoute, Params, Router } from '@angular/router';

 import { AlertModule } from 'ngx-bootstrap';

-import { Observable } from 'rxjs/Rx';
+import { Observable } from 'rxjs';


ddohler · 2019-07-16T13:20:21Z

...ar/planit/src/app/action-wizard/steps/implementation-step/implementation-step.component.html

@@ -21,7 +21,9 @@ <h3 class="step-title">Details</h3>
             autocomplete="off"
             formControlName="action_type"
             [typeahead]="actionTypes"
-             [typeaheadMinLength]="0">
+             [typeaheadMinLength]="0"
+             [typeaheadIsFirstItemActive]="false"


ddohler · 2019-07-16T13:59:25Z

src/angular/planit/src/app/core/services/city-profile.service.ts


 import { environment } from '../../../environments/environment';


 @Injectable()
 export class CityProfileService {

-  constructor(private apiHttp: PlanItApiHttp) {}
+  constructor(private http: HttpClient) {}


Just for my own understanding, what changed to make the subclass unnecessary?

We added these HTTP interceptors 3eb24a3 (a new feature introduced in Angular 5), which take the place of the custom code we had added to the subclass.

A new lint was added since Angular 4 that enforces this naming scheme These had already been renamed in the components repo before it was folded into this repository

maurizi · 2019-07-16T16:50:22Z

@ddohler I fixed the issue with the indicator charts in 7e5916b, and also fixed running the tests, and cleaned up some formatting and whitespace (including the mixed indentation of the climate-api module).

ddohler

I think it'd be good for someone else (probably @flibbertigibbet ) to take a second look at this before it goes in, given how large it is, but everything seems to be working pretty smoothly for me!

flibbertigibbet

The code looks good to me. I haven't been able to test it yet.

I ran into an unrelated error while attempting to build the containers:

flibbertigibbet · 2019-07-17T14:35:07Z

src/angular/planit/src/app/core/services/action-type.service.ts


 @Injectable()
 export class ActionTypeService {

-  constructor(private apiHttp: PlanItApiHttp,
+  constructor(private http: HttpClient,


flibbertigibbet · 2019-07-17T14:36:21Z

src/angular/planit/src/app/core/services/auth.service.ts

-        return new User(user);
+    return this.http.get(url).pipe(map(response => {
+      if (response) {
+        return new User(response);


flibbertigibbet · 2019-07-17T14:38:06Z

src/angular/planit/src/app/core/services/city-profile.service.ts

  }

  listOptions(): Observable<{[key: string]: CityProfileOption[]}> {
    const url = `${environment.apiUrl}/api/city-profile-options/`;
-    return this.apiHttp.get(url).map(response => response.json());
+    return this.http.get<{[key: string]: CityProfileOption[]}>(url);


flibbertigibbet · 2019-07-17T14:39:08Z

src/angular/planit/src/app/core/services/download.service.ts

      // Parse string to array of arrays
-      const csvData = papa.parse(resp['_body'], { newline: '\r\n' });
+      const csvData = papa.parse(resp, { newline: '\r\n' });


maurizi · 2019-07-17T15:18:41Z

I think if you run scripts/update again it will succeed this time. This is a similar issue to #1195 that was introduced by the Area indicators work being pulled into this repository.

flibbertigibbet and others added 30 commits July 12, 2019 10:12

Update all packages to latest

e054090

Upgrade to node 11

5d2b0c7

Prepare for upgrade

1647004

Pre-upgrade steps from https://update.angular.io/#4.4:8.0.

ng update @angular/cli

110faa4

Disable strictPropertyInitialization

b12e6ca

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-7.html#strict-class-initialization

arxjs-5-to-6-migrate -p tsconfig.json

bf907d2

Fix typescript version to <3.5

3285531

Update references to @angular/common/http

21fb2ca

http module moved into core.

Update Observable map references

1bd61e4

Add geojson types

0b07f7e

Explicitly make all ViewChildren static

38d673b

See: angular/angular-cli#14553 (comment) Fixes build error "TS2554: Expected 2 arguments, but got 1."

Remove deprecated linter configurations

5ae5e15

Both are in compiler instead.

Upgrade Angular Http client usage

7405fa5

See: https://angular.io/guide/deprecations

Fix assorted Observable errors from rxjs upgrade

5d5b761

Fix corejs version

e3b9f6c

Still on 2.x

Fix ngx-bootstrap modal import

48e185a

Fix serve command

5d73a96

Add library reference

95a9621

Fix HttpClientModule import

4e6ba1d

Fix build:prod command

98c96a1

Package updates

95a43f0

Install local packages in docker

e11ca74

FIXME: remove when upgrade complete.

Revert to current node LTS release

bc5ba57

Implement HTTP AuthInterceptor

3eb24a3

- Adds token to API requests - Redirects to home page in case of auth failure

Update to use typeaheadIsFirstItemActive property

8e48bee

So that we match the old Azavea ngx-bootstrap typeaheadMinLength=0 behavior.

Fix live reloading

00f205f

Fix displaying error messages returned by API

4b50ed1

Remove broken/unused linting rule

c36b220

Fix unauthenticated API use (new user registration, forgot password, …

fbb1df8

…etc) The token interceptor was trying to add an authorization token even when there was no logged in user, causing requests to be rejected.

maurizi added 9 commits July 12, 2019 10:13

Fix CSV download service

ca788a5

Fix styling reliant on whitespace for padding of inline elements

bae6a18

The newer version Angular we're upgrading to strips whitespace. In the few places where we were depending on whitespace to add padding to inline elements, I've explicitly added non-breaking spaces to accommodate.

Fix loading data for indicator charts

9761f2d

Run ng lint --fix

df46477

Fix typeahead usage

02b7049

Without also setting [typeaheadSelectFirstItem]="false", tabbing out of the input won't work with [typeaheadIsFirstItemActive]="false" set.

Fix remaining lint

1daba88

Fix unit tests

79035b4

Update CHANGELOG

95dda61

maurizi mentioned this pull request Jul 12, 2019

WIP: Upgrade Angular #1227

Closed

1 task

maurizi commented Jul 12, 2019

View reviewed changes

src/angular/planit/src/app/climate-api/api/models/historic-range.model.ts Outdated Show resolved Hide resolved

src/angular/planit/src/app/climate-api/shared/index.ts Outdated Show resolved Hide resolved

maurizi commented Jul 12, 2019

View reviewed changes

src/angular/planit/src/app/core/services/account-create.service.ts Outdated Show resolved Hide resolved

ddohler reviewed Jul 16, 2019

View reviewed changes

maurizi added 4 commits July 16, 2019 11:27

Fix shellcheck violation

eadf041

Fix running Angular tests from scripts/test

455256c

Rename callbacks to match correct naming scheme

7e5916b

A new lint was added since Angular 4 that enforces this naming scheme These had already been renamed in the components repo before it was folded into this repository

Whitespace cleanup

3089863

ddohler approved these changes Jul 16, 2019

View reviewed changes

flibbertigibbet approved these changes Jul 17, 2019

View reviewed changes

maurizi merged commit 122cd08 into develop Jul 17, 2019

maurizi deleted the feature/mvm/upgrade-angular branch July 17, 2019 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Angular, Node.js, and related dependencies #1243

Upgrade Angular, Node.js, and related dependencies #1243

maurizi commented Jul 12, 2019

maurizi left a comment

ddohler left a comment

ddohler Jul 16, 2019

ddohler Jul 16, 2019

ddohler Jul 16, 2019

maurizi Jul 16, 2019

maurizi commented Jul 16, 2019 •

edited

Loading

ddohler left a comment

flibbertigibbet left a comment

flibbertigibbet Jul 17, 2019

flibbertigibbet Jul 17, 2019

flibbertigibbet Jul 17, 2019

flibbertigibbet Jul 17, 2019

maurizi commented Jul 17, 2019

Upgrade Angular, Node.js, and related dependencies #1243

Upgrade Angular, Node.js, and related dependencies #1243

Conversation

maurizi commented Jul 12, 2019

Overview

Demo

Testing Instructions

maurizi left a comment

Choose a reason for hiding this comment

ddohler left a 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

maurizi commented Jul 16, 2019 • edited Loading

ddohler left a comment

Choose a reason for hiding this comment

flibbertigibbet left a 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

maurizi commented Jul 17, 2019

maurizi commented Jul 16, 2019 •

edited

Loading