add import of missing Angular interfaces (squashed version)

[saschaszott]

Description

This PR is a replacement of #2870 where all commits were squashed into one commit as suggested by @alanorth .

It is synced with the latest changes in main branch.

[saschaszott]

Should I open an issue for the adoption of an additional lint rule as suggested by @ybnd in #2870 (comment) ?

[saschaszott]

One test is failing, but it is not related to the proposed changes:

1) Edit Item > Edit Metadata tab
       should pass accessibility tests:
     AssertionError: 1 accessibility violation was detected: expected 1 to equal 0
      at Context.eval (webpack://dspace-angular/./node_modules/cypress-axe/dist/index.js:100:0)

[tdonohue]

@saschaszott : We've had random failures from that "Edit Item" e2e test. It's not specific to this PR. I've restarted the tests, as usually it passes after rerunning the tests.

[alanorth]

Thanks @saschaszott. I tested this by doing an item submission and searching with the sandbox.dspace.org instance as a backend. All working fine. Passing all tests too. Happy to merge this as it's 1 APPROVAL pull request only requires a single approval to merge .

I'm curious what the issue is here, though? If these interfaces were missing and everything was already working, what problem is this solving? 😄

[saschaszott]

I'm curious what the issue is here, though? If these interfaces were missing and everything was already working, what problem is this solving? 😄

https://www.reddit.com/r/Angular2/comments/ge3evp/comment/fpmzpah/

[ybnd]

@alanorth everything will still work if we don't implement those interfaces, but this way the intent is a lot more clear and it can serve as a sort of spell-check too

https://v17.angular.io/guide/styleguide#implement-lifecycle-hook-interfaces

same time, sorry

[saschaszott]

@alanorth , to enforce the import of Angular lifecycle interfaces a lint rule should be added as suggested by @ybnd in #2870 (comment)

[alanorth]

Awesome, thanks for the explanations. This sounds reasonable to me, but I will let someone familiar with Angular give the final approval.

[tdonohue]

@ybnd : Since you seem to agree with this direction... I wanted to see if you're interested in reviewing this? If you approve of this & have tested it yourself, then I think this can be merged. I do agree with @alanorth that'd it'd be nice to have another "Angular expert" at a different institution from @saschaszott to approve this. But, in my opinion, you'd qualify for that, @ybnd if you have time :)

In general, I agree with the direction here (and thank you to @saschaszott !). I just have not had time to get to myself yet (as I have a lot on my plate). So, it's only waiting for someone to have time to give it a code review.

[ybnd]

The changes that you've made so far are all good, but it looks that there's a few more files that should be updated.

Can you add "@angular-eslint/use-lifecycle-interface": "error" to .eslintrc.json and run yarn lint --fix?

This should fix the remaining problems and cover any upcoming ones as well.

[saschaszott]

@ybnd , @tdonohue : yarn lint --quiet is now reporting 21 new problems due to missing imports of Angular lifecycle interfaces. I will create one new commit that fixes these problems.

[saschaszott]

@ybnd , this PR is ready for review.

[tdonohue]

Merging with one approval. Looks good to me too. Thanks again @saschaszott !

[dspace-bot]

Successfully created backport PR for dspace-8_x:

• [Port dspace-8_x] add import of missing Angular interfaces (squashed version) #3242