Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Angular and Its Dependencies #2085

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Conversation

hudson-newey
Copy link
Member

@hudson-newey hudson-newey commented Oct 17, 2023

Upgrade Angular and Its Dependencies

We have several outdated dependencies, this pull request updates all the dependencies

You can see all the breaking changes due to our migration from ngx 14 -> 16 here

Although, a lot of tests were modified due to breaking changes in ng-mocks (see below)

Changes

  • Added tests for time.component.ts
  • Fixed bug in time.component.ts where errors weren't shown if a propper time wasn't input first
  • Commented out empty TODO tests (empty tests now causes a Karma error)
  • When fetching an element mocked with ng-mock, we can no longer fetch it with it's mocked instance, you now have to fetch it using the component which was mocked. This caused tests to break after upgrading
  • ng-mock MockRender now requires an empty object as the second parameter
  • Ran formatter on all files (this was automatically done by the ng update command)
  • We now use the "declare" keyword in certain places when overriding abstract class methods
  • Importing ngx-datatable styles is now done through the @use keyword instead of @import
  • Using the ~ as an alias for node_modules is no longer supported for scss imports. node_modules was added as a default import location for scss imports, and the use of the "~" was removed
  • Changed builder to es-build 🥳
  • Angular testing now recognises .spec.ts as the file extension for test files. This assertion was therefore removed from our test.ts
  • Stronger type checking for ng-template's required me to update the Typeahead input template output context to include the search term & term formatter
  • The collapsed property is no longer exposed in ngx-boostrap DOM elements. This required tests to be changed to query classess not DOM object properties
  • Added "work in progress" menu item predicate to add better semantic meaning as to why only admins can view a menu item
  • Officially dropped support for kaios, and opera mini browsers (as they do not support ES6 and are mobile browsers with small market share)
  • RouterLinkWithHerf is no longer implemented in Angular and all functionality has been transfered to RouterLink. Therefore, I've changed all instances of RouterLinkWithHerf with RouterLink (ensuring that I don't break strong route)
  • In cards.component.spec.ts, the CardsComponent was being mocked twice each test, causing errors
  • TypeScript return type of NodeJS setInterval was renamed to NodeJS.Timeout
  • "plugin:@angular-eslint/ng-cli-compat" es-lint extensions are now deprecated (has been merged with @angular-eslint/recommended)
  • eslint plugin "plugin:@angular-eslint/ng-cli-compat--formatting-add-on" was deprecated and merged with @angular-eslint/recommended
  • Upgraded to NodeJS version 18 (LTS) (in docker container and dev environment)

Problems

None

Issues

Fixes: #2077

Visual Changes

None

Final Checklist

  • Assign reviewers if you have permission
  • Assign labels if you have permission
  • Link issues related to PR
  • Ensure project linter is not producing any warnings (npm run lint)
  • Ensure build is passing on all browsers (npm run test:all)
  • Ensure CI build is passing
  • Ensure docker container is passing (docs)

@hudson-newey hudson-newey self-assigned this Oct 17, 2023
@hudson-newey hudson-newey added dependencies Pull requests that update a dependency file architecture Architectural changes to the software labels Oct 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2023

Size Change: -570 kB (-17%) 👏

Total Size: 2.88 MB

Filename Size Change
dist/workbench-client/browser/index.html 4.1 kB +268 B (+7%) 🔍
dist/workbench-client/browser/main.****************.js 0 B -1.15 MB (removed) 🏆
dist/workbench-client/browser/polyfills.****************.js 0 B -12.5 kB (removed) 🏆
dist/workbench-client/browser/runtime.****************.js 0 B -715 B (removed) 🏆
dist/workbench-client/browser/styles.****************.css 0 B -37.4 kB (removed) 🏆
dist/workbench-client/server/main.js 1.77 MB -479 kB (-21%) 🎉
dist/workbench-client/browser/chunk-7SGCG2G3.js 1.08 kB +1.08 kB (new file) 🆕
dist/workbench-client/browser/main.********.js 1.06 MB +1.06 MB (new file) 🆕
dist/workbench-client/browser/polyfills.********.js 12.2 kB +12.2 kB (new file) 🆕
dist/workbench-client/browser/styles.********.css 32.3 kB +32.3 kB (new file) 🆕
dist/workbench-client/server/776.js 4.2 kB +4.2 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size
dist/workbench-client/browser/assets/environment.json 555 B
dist/workbench-client/browser/manifest.json 150 B

compressed-size-action

@github-actions
Copy link
Contributor

github-actions bot commented Oct 17, 2023

Unit Test Results

         6 files           6 suites   10m 45s ⏱️
20 694 tests 20 070 ✔️ 624 💤 0
20 802 runs  20 178 ✔️ 624 💤 0

Results for commit fa2d150.

♻️ This comment has been updated with latest results.

@hudson-newey hudson-newey requested a review from atruskie October 17, 2023 07:23
src/app/app.menus.ts Outdated Show resolved Hide resolved
@@ -111,8 +112,8 @@ export const uploadAnnotationsProjectMenuItem = menuRoute({
icon: ["fas", "file-import"],
label: "Batch Upload Annotations",
parent: projectMenuItem,
// TODO Change to isProjectEditorPredicate
predicate: isAdminPredicate,
// TODO: Once functionality is implemented, this should be changed to isProjectEditorPredicate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it?

you should rely on a capability from the server if it exists...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, this is how the isProjectEditorPredicate predicate works

image

Where canEdit comes from the server capabilities

image

Note: Because this PR is going to be merged before the annotation uploads, I've put this behind the wipPredicate

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point rather is that assuming someone can edit a project means that they can upload annotations is not correct.

From my memory of the servers permissions: only owners can edit projects but anyone with writer can upload annotations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point rather is that assuming someone can edit a project means that they can upload annotations is not correct.

From my memory of the servers permissions: only owners can edit projects but anyone with writer can upload annotations.

Understandable, I've backfilled a new predicate in preparation

image

src/app/directives/url/url-active.directive.ts Outdated Show resolved Hide resolved
src/app/guards/input/input.guard.ts Outdated Show resolved Hide resolved
@hudson-newey hudson-newey force-pushed the dependency-upgrade-2023 branch 6 times, most recently from 0aea7c9 to 2a48f37 Compare October 24, 2023 05:40
Fixes: #2077

Ran ng update

Fixed tests for angular 15

Updated ng express

Updated to angular 16

Removed override

Got client working on ng 16

Updated other libraries

Commented out broken test

Changed to esbuild

Uncommented not working test

Fixed time component bug

Reverted test changes

Updated http-status

Update ng-spectator

Upgraded dependencies

Fixed POSIX compliance for many files

Fixed baw-client tests

Improved typing of baw-client.component.spec.ts

Finished dependency upgrade

Fixed ssr CI

Updated node version in docker container

PR review changes

Fixed is wip predicate

Added isProjectEditorPredicate

Fixed sass transpiler errors

Reverted @vvo/tzdb upgrade

Reverted saas working changes
@hudson-newey hudson-newey force-pushed the dependency-upgrade-2023 branch from 2a48f37 to fa2d150 Compare October 24, 2023 06:25
@hudson-newey
Copy link
Member Author

hudson-newey commented Oct 24, 2023

I've done some testing using staging data and there are no regressions

Given that this PR will impact almost every function of the web client, are you all good for me to merge after CI passes?

@hudson-newey hudson-newey merged commit fe80ba4 into master Oct 30, 2023
25 checks passed
@hudson-newey hudson-newey deleted the dependency-upgrade-2023 branch October 30, 2023 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Architectural changes to the software dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change builder to esbuild
2 participants