feat(router): route initializer #36084

kristofdegrave · 2020-03-16T11:44:41Z

This is an initial POC to provide an initialization hook for lazy loaded Modules. Implementation is Open for Discussion!

Additional Docs will be added when a consent about the solution is made.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

A lazy route is now just loaded without being block by initialization logic

Issue Number: #17606

What is the new behavior?

A lazy route is now loaded with an option being block by initialization logic. If all initialization logic has run, the route gets loaded

Does this PR introduce a breaking change?

Yes
No

Other information

It introduces a new injection toke ROUTE_INITIALIZER.

googlebot · 2020-03-16T11:44:45Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

kristofdegrave · 2020-03-16T11:49:08Z

@googlebot I signed it!

Add an injection token ROUTE_INITIALIZE to provide one or more initialization functions. These function are injected when a route is loaded. If any of these functions returns a Promise, initialization does not complete until the Promise is resolved. If the initialization is completed the LoadedRouterConfig will be returned.

Did some code refactoring after feedback from frederikprijck.

googlebot · 2020-03-16T12:33:18Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

steve-todorov · 2020-06-14T17:30:05Z

Hey @atscott, have you had a chance to checkout this implementation yet? :)

atscott

Does this provide value beyond what CanActivate at the forChild root could do?

RouterModule.forChild([
  {path: '', canActivate: [MyInitializerGuard], children: [...]}
]);

The canActivate there would allow for asynchronous initialization before attempting to activate any child routes in the module. If you think of canActivate conceptually like "can this route activate yet?" rather than "is it allowed to activate at all?" it makes a bit more sense to do this.

atscott · 2020-08-10T21:13:50Z

packages/router/src/router_config_loader.ts

      if (this.onLoadEndListener) {
        this.onLoadEndListener(route);
      }

      const module = factory.create(parentInjector);
+      const routeInits: (() => any)[] = module.injector.get(ROUTE_INITIALIZER, []);


This type should be consistent with the injection token: Array<() => void> here and in runInitializers

atscott · 2020-08-10T21:14:18Z

packages/router/src/router_config_loader.ts

    }));
  }

+  private runInitializers(routeInits: (() => any)[] = []) {


The default value [] isn't necessary here.

atscott · 2020-08-10T21:20:51Z

packages/router/src/router_config_loader.ts

    }));
  }

+  private runInitializers(routeInits: (() => any)[] = []) {
+    return from(Promise.all(routeInits.map(routeInit => routeInit()).filter(isPromise)));


This would be easier to understand if you extracted some variables. I see what you're doing here and I do like how clever it is, but it might more immediately understandable if it were deconstructed a bit.

Suggested change

return from(Promise.all(routeInits.map(routeInit => routeInit()).filter(isPromise)));

const initResults = routeInits.map(routeInit => routeInit());

const promiseInits = initResults.filter(isPromise);

return from(Promise.all(promiseInits));

atscott · 2020-08-10T21:25:29Z

packages/router/src/router_config_loader.ts

-          flatten(module.injector.get(ROUTES)).map(standardizeConfig), module);
+      return this.runInitializers(routeInits)
+          .pipe(
+              map(() => new LoadedRouterConfig(


Is this the right operator here? While from(Promise.all) would only emit once, this code shouldn't rely on the implementation details of the runInitializers function. I think this should instead be last or even finalize/finally. What happens when an initializer rejects isn't really discussed here or tested (please add a test or two for it).

atscott · 2020-08-10T21:50:09Z

packages/router/src/router_config_loader.ts

 /**
 * The [DI token](guide/glossary/#di-token) for a router configuration.
 * @see `ROUTES`
 * @publicApi
 */
 export const ROUTES = new InjectionToken<Route[][]>('ROUTES');

+/**
+ * An injection token that allows you to provide one or more initialization functions.
+ * These function are injected when a route is loaded. If any of these functions returns


Suggested change

* These function are injected when a route is loaded. If any of these functions returns

* These function are injected when a module is loaded. If any of these functions returns

These initializers have more to do with the modules being lazy loaded than they do with a "Route". This documentation is a bit misleading and seems to imply the initializers are linked to Routes and not the act of lazy loading the module they're provided in. It's maybe a subtle distinction, but I think it's important.

That said, maybe the token token name needs some more bikeshedding

atscott · 2020-08-10T21:50:26Z

packages/router/src/router_config_loader.ts

+ * a Promise, initialization does not complete until the Promise is resolved.
+ *
+ * You can, for example, create a factory function that loads language data
+ * or an external configuration, and provide that function to the `ROUTER_INITIALIZER` token.


Suggested change

* or an external configuration, and provide that function to the `ROUTER_INITIALIZER` token.

* or an external configuration, and provide that function to the `ROUTE_INITIALIZER` token.

atscott · 2020-08-10T21:58:00Z

packages/router/test/router_config_loader.spec.ts

@@ -0,0 +1,183 @@
+/**


These tests are good, but it would also be nice to have coverage in integration.spec.ts to verify correct behavior during route activation.

atscott · 2020-08-10T22:01:23Z

packages/router/src/router_config_loader.ts

 import {Compiler, InjectionToken, Injector, NgModuleFactory, NgModuleFactoryLoader} from '@angular/core';
+import {isPromise} from '@angular/core/src/util/lang';


This should be imported from @angular/core like what's done in utils/collection.ts

atscott · 2020-08-10T22:02:01Z

packages/router/src/router_config_loader.ts

@@ -6,19 +6,37 @@
 * found in the LICENSE file at https://angular.io/license
 */

+// TODO(i): switch to fromPromise once it's expored in rxjs


let's remove this comment

kristofdegrave · 2020-08-24T07:19:21Z

@atscott I think the canActivate hook will be to late. The loading of the module depends on the loading strategy and can be preloaded. The case I need it for is to make sure that I can fetch configuration for example before the the module and it's dependencies are loaded, because they can depend on that configuration.
I will look into the review you gave asap.

atscott · 2020-08-24T14:26:00Z

@atscott I think the canActivate hook will be to late. The loading of the module depends on the loading strategy and can be preloaded. The case I need it for is to make sure that I can fetch configuration for example before the the module and it's dependencies are loaded, because they can depend on that configuration.

Outside of preloading, CanLoad could be used in this case. With preloading, it seems like you could actually build this functionality into your preloader. I think you’re going to need to provide more concrete examples before we can consider moving this PR forward. It seems like the existing router guards allow for handling the behavior desired. This also does not resolve the linked issue since modules can be loaded without the router.

kristofdegrave · 2020-08-25T07:33:51Z

@atscott never looked this way at it, you could have a point. The desired behavior is the same as the APP_INITIALIZER I want to have, and ideal it shouldn't matter if the module is loaded immediately or trough a lazy route.
I wanted this behavior to prepare my application to move to a MicroFrontend architecture. So every (routed) module would be a micro frontend in this case, with it's own configuration file. So to get this behavior right I should write some logic in the preloader and the canLoad guard that gets the APP_INITIALIZER tokens from the module that needs to get loaded and initialize them, but is this even possible?
The module would need to be loaded before I can get them in those cases? Also I would need something to determine if the initialization has run

atscott · 2020-08-25T18:01:31Z

The module would need to be loaded before I can get them in those cases? Also I would need something to determine if the initialization has run

That's right, I was really just suggesting canLoad because you said canActivate is too late. I guess what you meant was that if the module is preloaded, canActivate still won't run until you try navigating to a route in that module but you would want the initializers to run before that.

Here's an [over-engineered] example of how this could all work:
https://stackblitz.com/edit/angular-ivy-72wgbc?file=src%2Fapp%2Finitializing-router.module.ts
Unfortunately, there's still a "small" hack since _loadedConfig is internal and the preloader doesn't technically have any way to access the module that was loaded. Maybe there's a way to do this that I'm not thinking of... Other than that, I think the would be what you're looking for.

atscott · 2020-12-04T19:48:59Z

Closing this PR per the comments mentioned above, namely:

This does not resolve the linked issue since modules can be loaded without the router.
Some of this behavior can be achieved with the router guards
I think this should be addressed in a more complete way along with Ergonomic component-level code splitting and lazy loading #18093 rather than extending the router API to only partially fix the issues.

angular-automatic-lock-bot · 2021-01-04T16:51:13Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: no label Mar 16, 2020

pullapprove bot requested a review from atscott March 16, 2020 11:44

kristofdegrave added 2 commits March 16, 2020 13:32

refactor(router): some refactorings

f87441b

Did some code refactoring after feedback from frederikprijck.

kristofdegrave force-pushed the feature/route_initializer branch from 9a374ab to f87441b Compare March 16, 2020 12:33

googlebot added cla: yes and removed cla: no labels Mar 16, 2020

AndrewKushnir added area: router feature Issue that requests a new feature labels Mar 18, 2020

ngbot bot added this to the needsTriage milestone Mar 18, 2020

atscott requested changes Aug 10, 2020

View reviewed changes

atscott mentioned this pull request Aug 11, 2020

MODULE_INITIALIZER like APP_INITIALIZER #17606

Closed

atscott closed this Dec 4, 2020

kristofdegrave mentioned this pull request Dec 16, 2020

Pass configuration data to services in a lazy loaded module #34351

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(router): route initializer #36084

feat(router): route initializer #36084

kristofdegrave commented Mar 16, 2020 •

edited

Loading

googlebot commented Mar 16, 2020

kristofdegrave commented Mar 16, 2020

googlebot commented Mar 16, 2020

steve-todorov commented Jun 14, 2020

atscott left a comment

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

atscott Aug 10, 2020

kristofdegrave commented Aug 24, 2020

atscott commented Aug 24, 2020

kristofdegrave commented Aug 25, 2020

atscott commented Aug 25, 2020

atscott commented Dec 4, 2020

angular-automatic-lock-bot bot commented Jan 4, 2021

-    return from(Promise.all(routeInits.map(routeInit => routeInit()).filter(isPromise)));
+    const initResults = routeInits.map(routeInit => routeInit());
+    const promiseInits = initResults.filter(isPromise);
+    return from(Promise.all(promiseInits));

	* These function are injected when a route is loaded. If any of these functions returns
	* These function are injected when a module is loaded. If any of these functions returns

	* or an external configuration, and provide that function to the `ROUTER_INITIALIZER` token.
	* or an external configuration, and provide that function to the `ROUTE_INITIALIZER` token.

		import {Compiler, InjectionToken, Injector, NgModuleFactory, NgModuleFactoryLoader} from '@angular/core';
		import {isPromise} from '@angular/core/src/util/lang';

feat(router): route initializer #36084

feat(router): route initializer #36084

Conversation

kristofdegrave commented Mar 16, 2020 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

googlebot commented Mar 16, 2020

What to do if you already signed the CLA

Individual signers

Corporate signers

kristofdegrave commented Mar 16, 2020

googlebot commented Mar 16, 2020

steve-todorov commented Jun 14, 2020

atscott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristofdegrave commented Aug 24, 2020

atscott commented Aug 24, 2020

kristofdegrave commented Aug 25, 2020

atscott commented Aug 25, 2020

atscott commented Dec 4, 2020

angular-automatic-lock-bot bot commented Jan 4, 2021

kristofdegrave commented Mar 16, 2020 •

edited

Loading