Consolidate all initialization in a single Service

[ybnd]

References

Add references/links to any related issues or PRs. These may include:

• Fixes #1683
• More robust approach to address #1668

Description

This PR moves most app initialization to a new InitService in order to

• make sure route-listening initialization methods are called as early as possible
  • After #1627, AppComponent does not receive the earliest Router events, which lead to issues with route-dependent themes
• have just one APP_INITIALIZER provider
• keep all initialization logic close together and more neatly separated between server/browser
  • before, initialization was split between AppModule, *AppModule, *DSpaceStateTransfer and AppComponent
  • AppComponent used @Optional to skip some browser-specific steps on the server

Some extra context

• A side effect of injecting Router into an APP_INITIALIZER dependency was that now APP_BASE_HREF must be fully resolved by the time InitService.init() is called, otherwise the app crashes during the SSR→CSR transition. To address ths we moved the extendEnvironmentWithAppConfig() call to a "pre-initialization step" to ensure it is called before init().

• The providers required for InitService are closely linked to it and a bit finnicky, so we thought it best not to duplicate them in ServerAppModule/BrowserAppModule. Instead, they are defined once in InitService.providers().

Going forward, when adding new initialization logic:

• Check if it can actually be done in *InitService
  • Services that (indirectly) depend on ApplicationRef can only be injected after APP_INITIALIZER, so they cannot be used in InitService (e.g. NgbModal)
  • Anything that needs to interact with a component template can't be moved from that component
• Add a new method for this particular initialization step. If it can be reused between the server and browser implementations, add it to InitService.
• Call this method from ServerInitService and/or BrowserInitService

Instructions for Reviewers

Confirm that the app keeps working in exactly the same way as before this PR.

Confirm that #1668 doesn't reappear.

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!

• [x] 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.
• [x] My PR passes TSLint validation using yarn run lint
• [x] My PR doesn't introduce circular dependencies
• [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• [x] 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[bot]]

This pull request introduces 3 alerts when merging 4403555c29f8b0a48dfb3671f85b7b93cd017d1b into bfbe38974ba8eb40a93c412b2960381b6ace3303 - view on LGTM.com

new alerts:

• 3 for Unused variable, import, function or class