Upgrade to Angular 13

[ybnd]

References

• Fixes Upgrade to Angular 12 or 13 #1432
• Fixes Upgrade to PostCSS v8 for custom-webpack v11 (and above) #1433
• Fixes problematic Angular compiler setting: set preserveWhitespaces to default value (false) #903

Description

Upgrade to Angular 13

Dependencies

• Major changes

  • webpack v4 → v5
  • postcss v7 → v8
  • Migrated from tslint to eslint

• A few packages don't support Angular 13 (haven't encountered any issues yet)

  • @nicky-lenaers/ngx-scroll-to
  • ngx-sortablejs
  • @ng-dynamic-forms

Fixed SSR failures
Apparently it's the intended behavior of cypress-io/github-action@v2 not to shut down the app after running tests.
This caused the subsequest SSR check to fail sometimes; added an extra CI step to shut down the app to solve this.

Remaining issues / TODO

• Upgrade bootstrap v4 → v5

  • To keep v4 w/ Angular 13 we've had to specifically use sass <1.33 for sass-loader to suppress a bunch of warnings
  • Upgrading to Bootstrap 5 breaks the DSpace theme in many minor ways; the individual issues aren't too problematic, but we'd need to set aside some time to make sure we cover everything.
    Seems better to address this in a separate PR

• With TSLint we used the indent rule to enforce spaces for indentation. ESLint's Typescript plugin has a similar rule which is much more strict, but also broken. Because of this we've left it disabled for now.

  However, the parts of the new rule that do work highlighted some major potential improvements in terms of formatting consistency that we may want to address (e.g. with a formatting tool like Prettier)

Reviewing

• Installation
  • Should work
  • The only dependency warnings should be the ones addressed above
• Unit & e2e tests should all pass (WIP)
• Application
  • Should work in dev and prod mode
  • Should look & behave the same as ~ https://demo7.dspace.org/home

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome). If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  → most of the changes in yarn.lock
• My PR passes TSLint validation using yarn run lint
• My PR doesn't introduce circular dependencies
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

[lgtm-com]

This pull request introduces 1 alert when merging fd76e43 into 3ecb3c2 - view on LGTM.com

new alerts:

• 1 for Unused variable, import, function or class

[tdonohue]

@ybnd and @artlowel : Overall excellent job. I've tested this and found no obvious issues, it looks great. I've also reviewed the code, and overall it also looks good as well -- I noticed despite the size of the PR, it's rather easy to review, as it's mostly a lot of small fixes related to tslint -> eslint migration.

There's only one problem: this PR seems to have accidentally broken CodeCov reports. It's not really a fault of this PR, as it appears to be a bug in Codecov (see my inline comment below). But, I'd like us to find a way to workaround that bug so that Codecov still works (some ideas below).

Beyond that small (but obviously important) thing, I think the rest of this PR looks great!

[tdonohue]

👍 Thanks @ybnd ! This PR looks good to me now. My minor feedback has been addressed as Codecov is now working on this PR again. As noted on Friday, I tested/reviewed this PR and found it works well & the code looks good.

[atarix83]

thanks @ybnd for this importatn PR

we made some test and found out the following problems

The problem with button spacing is due to the preserveWhitespaces that has been disabled.

For the problem in the browse page i'm not sure if it depends on some new PR merged on the main branch that is not yet merged in this one.

Overall it seems all good, I've only one doubt regarding the fact that now Angular 13 by default use RxJS v7.4 but we are still using v6.6.3. If it's too effort to do in this PR we need a follow-up Pr to migrate it

[ybnd]

• Fixed the remaining spacing issues
  • Took a different aproach that I think is going to be beneficial in the long run:
    • Put "things that should be spaced" in a div.space-children-mr to automatically add a margin to the right (except for the last child)
    • Won't have to remember e.g. to add margins for each button
    • Margin via a CSS variable to make this more flexible ~ theming just in case
  • I covered all I could find, but chances are there's still some of these spacing problems buried somewhere
• Started on the upgrades ↑, but it seems like this will take a while (@tdonohue MOVED to Update to ng-dynamic-forms v15 and rxjs v7 #1590 ticket)
  • Upgrading only rxjs to v7 results in ~200 test failures, a good number of which fail due pretty uninformatively within rxjs code, no DSpace in the trace at all -- this will take me a while to figure out
  • ng-dynamic-forms@15 depends on rxjs@7, so it would be best to do both at the same time
  • When upgrading both, as well as other mismatched peer dependencies that come up ~ rxjs@7 reduces the amount of failing tests to 11, so it would definitely be more manageable to upgrade all of these at once.
• Rebased on main for Snyk/LGTM

[atarix83]

I've reviewed it again and the previous issue are fixed.
I agree to move migration of RXJS and ng-dynamic-form together in a follow-up PR