Upgrade to Angular 13 #1249

cheehongw · 2024-02-20T03:49:22Z

Summary:

Upgrade to Angular 13 and Node 16
This pull request addresses some of the outdated packages in our application.

Partially addresses #1192

Changes Made:

Upgrade to Angular 13 using ng update
See: https://update.angular.io/?l=3&v=12.0-13.0
- We are unaffected by most changes mentioned in the update guide.
tslint config got removed in angular.json due to it being deprecated.
- Awaiting a separate PR Migrate from TSLint to ESLint #1250.
Upgrade dependency [email protected] from 12.0.1 to support Angular 13
ComponentFactory and ComponentFactoryResolver got removed in Angular 13. Updated LoadingService and its spec file to use the component directly.
MatSpinner got renamed to MatProgressSpinner in Angular 13
Upgrade [email protected] to 3.0.1 to support Angular 13
- See: https://the-guild.dev/graphql/apollo-angular/docs/migration
- Re-added ApolloModule under AppModule
- Skipped the extract-files step since we do not use Apollo to upload files
Updated karma.conf.js because tests with --code-coverage were failing. Fix was found here

Proposed Commit Message:

Upgrade to Angular 13

Some of our packages are old and outdated. We should actively maintain 
and keep these packages up-to-date so it is easier to maintain in the 
future.

Let's upgrade to Angular 13 to keep our packages up-to-date.

cheehongw · 2024-03-04T05:05:21Z

Loading service still works

CATcher.3.5.2.-.CATcher-org_bugreporting.-.Google.Chrome.2024-03-04.12-02-31.mp4

src/app/shared/issue-tables/issue-paginator.ts

vigneshsankariyer1234567890

LGTM for the most part! 🚀 code looks fine

Works alright on my machine as well

We have upgraded to Angular 13. Lets bump ngx-markdown to v13 to support the newer Angular version.

This line gave me typescript erorr TS2362.

In Angular 13, FactoryComponent and FactoryComponentResolver API got deprecated. This commit removes the deprecated API in favor of other APIs where Component classes can be used directly. The affected classes are loading.service.ts and its spec file.

The `[email protected]` library does not support Angular 13. As such, we are upgrading to `[email protected]`. With this change, there are several breaking changes that we must add to our codebase. See: Migration guide to apollo-angular v3 https://the-guild.dev/graphql/apollo-angular/docs

vigneshsankariyer1234567890

LGTM 🚀 Again, most of the changes are dependency based, and seems to work alright.

luminousleek · 2024-04-27T11:57:56Z

System

OS: macOS Sonoma 14.4.1
Browser: Safari
Browser Version: Version 17.4.1 (19618.1.15.11.14)
Local Angular Version: 12.2.18
node Version: v14.19.3
npm Version: 6.14.17

Actions Run

npm install: success
npm run ng:serve:web: - had to run npx browserslist --updatedb for it to work
npm run test: - had to delete package-lock.json and node-modules and re-run npm install for it to work
npm run e2e: success

Tested Phases

Bug Reporting Phase
Team's Response Phase
Tester's Response Phase

No bugs found

luminousleek · 2024-04-27T14:48:28Z

Once we use Node 16 for the GitHub CI tests we need to update the branch protection to require linux-setup-and-tests (16.x) instead of linux-setup-and-tests (14.x)

cheehongw · 2024-04-27T15:32:06Z

Finally got the CI working. This comment will detail the changes made to the environment for the Angular 13 upgrade:

Upgrade to Node 16, npm8 from Node14, npm6
@octokit/core isn't used directly as a dependency in our project, but @octokit/rest depends on it and npm8 installs the latest one (v6.1.2), which doesnt support Node 16.
- As such, this PR includes a new ~~dev dependency~~ dependency to freeze @octokit/core at v4, which is the last version to support Node 16
- We should consider removing it once we upgrade to Angular 15, which supports Node 18.
Manually install firefox for the macos-latest runner. As of this comment, macos-latest uses a macos 14 arm64 image, which doesnt include firefox. This seems to be a very recent change, since macos-latest always pointed to a x64 image which came pre-installed with firefox.

cheehongw · 2024-04-27T15:34:48Z

System

OS: Windows 10
Browser: Chrome
Browser Version: 124.0.6367.91
Local Angular Version: 13.4.0
node Version: 16.20.2
npm Version: 8.19.4

Actions Run

npm install
npm run ng:serve:web
npm run test
npm run e2e

All success

Tested Phases

Bug Reporting Phase
Team's Response Phase
Tester's Response Phase

No bugs found

chunweii · 2024-04-27T15:47:08Z

Finally got the CI working. This comment will detail the changes made to the environment for the Angular 13 upgrade:

Upgrade to Node 16, npm8 from Node14, npm6

@octokit/core isn't used directly as a dependency in our project, but @octokit/rest depends on it and npm8 installs the latest one (v6.1.2), which doesnt support Node 16.

As such, this PR includes a new dev dependency to freeze @octokit/core at v4, which is the last version to support Node 16

We should consider removing it once we upgrade to Angular 15, which supports Node 18.

Manually install firefox for the macos-latest runner. As of this comment, macos-latest uses a macos 14 arm64 image, which doesnt include firefox. This seems to be a very recent change, since macos-latest always pointed to a x64 image which came pre-installed with firefox.

Thanks for the fix @cheehongw! I have some comments on the octokit dependency.

Dev dependencies should be reserved for dependencies that are only used during development and not needed in production. CATcher and WATcher has been so far installing everything for use in production so putting @octokit/core into devDependencies should not break stuff, but I think the best practice is to put it into the dependencies instead.
Also, a better way to resolve the octokit issue would be to upgrade @octokit/rest to ^18.12.0, but this can cause some breaking changes (we need to change some parts of our code).

cheehongw · 2024-04-27T16:02:19Z

Thanks for the review @chunweii

Ok, will move octokit/core to dependencies
Any reason for targeting octokit/[email protected]? I am unable to find anything in the octokit/rest release notes about changing how octokit/core is depended upon.
- Either way and as you mentioned, the task of upgrading octokit/rest has alot of breaking changes. Going from v16 -> v17 alone seems to introduce quite a bit of breaking changes. As such, I think this is better off as a separate PR since it is not related to Angular 13.

cheehongw · 2024-04-27T16:10:47Z

System

OS: Ubuntu 22.04
Browser: Chrome
Browser Version: 123.0.6312.122
Local Angular Version: 13.4.0
node Version: 16.20.2
npm Version: 8.19.4

Actions Run

npm install
npm run ng:serve:web
npm run test
npm run e2e

All success

Tested Phases

Bug Reporting Phase
Team's Response Phase
Tester's Response Phase

No bugs found

chunweii · 2024-04-27T16:15:36Z

Any reason for targeting octokit/[email protected]? I am unable to find anything in the octokit/rest release notes about changing how octokit/core is depended upon.

Either way and as you mentioned, the task of upgrading octokit/rest has alot of breaking changes. Going from v16 -> v17 alone seems to introduce quite a bit of breaking changes. As such, I think this is better off as a separate PR since it is not related to Angular 13.

This old version of octokit (v16) does not support the use of ESM imports (I don't see it in the documentations), which imo makes the type checking better.

Furthermore, one of the dependencies in octokit/rest is causing the problem of installing an incompatible version of octokit/core. I tried installing v18 and there were no issues with installation afterwards (but I could also have tried 17, or some other version)

But I agree we should not upgrade octokit in this PR

cheehongw marked this pull request as draft February 20, 2024 03:49

cheehongw force-pushed the angular-13 branch from 437c1b7 to 6f99254 Compare March 4, 2024 04:53

cheehongw requested a review from vigneshsankariyer1234567890 March 4, 2024 05:05

cheehongw marked this pull request as ready for review March 4, 2024 05:06

vigneshsankariyer1234567890 reviewed Mar 4, 2024

View reviewed changes

src/app/shared/issue-tables/issue-paginator.ts Show resolved Hide resolved

vigneshsankariyer1234567890 reviewed Mar 4, 2024

View reviewed changes

cheehongw force-pushed the angular-13 branch from 6f99254 to d841253 Compare April 12, 2024 06:55

cheehongw added 5 commits April 12, 2024 14:55

Update to @angular/cli@13 and @angular/core@13

42b82c4

Update to @angular/material@13

6dc336e

Update [email protected] from @12.0.1

30cb199

We have upgraded to Angular 13. Lets bump ngx-markdown to v13 to support the newer Angular version.

Fix TS error from decrement operation

6506bc3

This line gave me typescript erorr TS2362.

cheehongw force-pushed the angular-13 branch from d841253 to f59ba8f Compare April 12, 2024 06:55

cheehongw added 4 commits April 26, 2024 20:39

Merge branch 'master' into angular-13

2c009dc

Update karma config to run tests with --code-coverage

4ae7224

Upgrade eslint dependencies for Angular 13

9b3f5f5

vigneshsankariyer1234567890 approved these changes Apr 27, 2024

View reviewed changes

Merge branch 'master' into angular-13

72c5e3a

cheehongw added 2 commits April 27, 2024 21:33

Use node 16 in github CI

70e27b7

Add @octokit/core dependency

075bcb4

cheehongw force-pushed the angular-13 branch from 16fdc2d to 075bcb4 Compare April 27, 2024 14:16

Use macos-x64 instead of macos-arm in runner

31b14a4

cheehongw added 2 commits April 27, 2024 23:07

install firefox on macos-latest

bff197c

Install firefox manually on macos-latest

53d8be0

Update .nvmrc to specify node 16

0f87a2d

Move octokit/core under dependencies

52ff77c

cheehongw merged commit ed71498 into CATcher-org:master Apr 27, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Angular 13 #1249

Upgrade to Angular 13 #1249

cheehongw commented Feb 20, 2024 •

edited

Loading

cheehongw commented Mar 4, 2024 •

edited

Loading

vigneshsankariyer1234567890 left a comment

vigneshsankariyer1234567890 left a comment

luminousleek commented Apr 27, 2024

luminousleek commented Apr 27, 2024

cheehongw commented Apr 27, 2024 •

edited

Loading

cheehongw commented Apr 27, 2024

chunweii commented Apr 27, 2024

cheehongw commented Apr 27, 2024

cheehongw commented Apr 27, 2024

chunweii commented Apr 27, 2024

Upgrade to Angular 13 #1249

Upgrade to Angular 13 #1249

Conversation

cheehongw commented Feb 20, 2024 • edited Loading

Summary:

Changes Made:

Proposed Commit Message:

cheehongw commented Mar 4, 2024 • edited Loading

vigneshsankariyer1234567890 left a comment

Choose a reason for hiding this comment

vigneshsankariyer1234567890 left a comment

Choose a reason for hiding this comment

luminousleek commented Apr 27, 2024

System

Actions Run

Tested Phases

luminousleek commented Apr 27, 2024

cheehongw commented Apr 27, 2024 • edited Loading

cheehongw commented Apr 27, 2024

System

Actions Run

Tested Phases

chunweii commented Apr 27, 2024

cheehongw commented Apr 27, 2024

cheehongw commented Apr 27, 2024

System

Actions Run

Tested Phases

chunweii commented Apr 27, 2024

cheehongw commented Feb 20, 2024 •

edited

Loading

cheehongw commented Mar 4, 2024 •

edited

Loading

cheehongw commented Apr 27, 2024 •

edited

Loading