feat(ls-routes): introduce ls-routes #778

Toxicable · 2017-08-14T21:59:43Z

Please check if the PR fulfills these requirements

- [x] The commit message follows our guidelines
- [ ] Tests for the changes have been added (for bug fixes / features)
- [x] Docs have been added / updated (for bug fixes / features)

What modules are related to this pull-request

- [x] ls-routes (new)

What kind of change does this PR introduce?
A tool for traversing a given factory and retrieving the routes.
Useful when used with Universal pre-rendering when you want to prerender each potential static route

This differs from Alex's one, ref https://github.com/alxhub/universal/blob/pwa-tools/modules/pwa-tools/lib/ls-routes/lib.ts
In that it works off a factory bundle rather than the Module src and is only hunting for routes instead of other metadata

PatrickJS · 2017-08-15T02:54:52Z

modules/ng-ls-routes/src/routes-ls.ts

+function resolveLazyChildren(route: Route, injector: Injector): Promise<Route> {
+  if (typeof route.loadChildren === 'function') {
+    //not supported
+    //return route.loadChildren().;


why would this not be supported if it returns a Promise?

The issue I had with this one is that loadChildren is typed as () => Type<any> | NgModuleFactory<any> | Promise<Type<any>> | Observable<Type<any>>
So out of those we could do a instanceof to see if it's a NgModuleFactory
and Coerce the Observable into a Promise
But I'm uncertain how to how to retrieved a NgModuleFactory from a Type

can you use the RouterConfigLoader?
https://github.com/angular/angular/blob/master/packages/router/src/router_config_loader.ts

here's an example of how it's used
https://github.com/angular/angular/blob/master/packages/router/src/router_preloader.ts#L126

That's a Private API since you cant find it here https://github.com/angular/angular/blob/master/tools/public_api_guard/router/router.d.ts
but it does infer a reasonable point

if (t instanceof NgModuleFactory) { return of (t); } else { return fromPromise(this.compiler.compileModuleAsync(t)); }

this indicates to me that the returned function will be a factory if it's in AOT or just a normal type if it's JIT, hence the compiler

Toxicable · 2017-08-15T06:39:15Z

I've added Karma config for testing to the project since it was becoming very tedious to test if specific scenarios were working.
I'm not sure if this is wanted to be in the same commit or if the config I've done is any good so feel free to advise me otherwise.
Also you'll see Travis failing because it cant find @nguniversal/module-map-ngfactory-loader that's because the Tool I'm adding depends on it but it's not actually published to NPM yet

ghost · 2017-08-15T10:58:25Z

modules/ng-ls-routes/src/ls-routes.spec.ts

+
+class FileLoader implements ResourceLoader {
+  get(url: string): Promise<string> {
+    return new Promise((resolve) => {


ResourceLoader#get returns a Promise | string union. You can thus return readFileSync directly instead of the resolved promise.

With fs.ReadFile you have to provide callbacks and what not, it's not as easy as just calling .toString(). I did simplify it a little bit more, I don't think using the sync version could get it anymore concise

huh, readFileSync is used in that code. No promise needed.

oh! sorry, I misread what you said, gotcha

frederikprijck · 2017-08-16T21:01:53Z

modules/ng-ls-routes/src/ls-routes.spec.ts

+  class MockServerModule{}
+  return jitCompiler.compileModuleAsync(MockServerModule);
+}
+function createFactoryAndGetRotues(routeConfig: Route[], moduleMap: {[key: string]: any} = {}) {


typo in createFactoryAndGetRotues

frederikprijck · 2017-08-16T21:09:05Z

modules/ng-ls-routes/src/ls-routes.ts

+  });
+}
+
+function cocerceIntoPromise<T>(mightBePromise: Observable<T> | Promise<T> | T): Promise<T> {


I guess this has to be coerceIntoPromise ?

Toxicable · 2017-08-29T00:11:09Z

now that @nguniversal/module-map-ngfactory-loader is published this is no longer blocked by travis

PatrickJS · 2017-09-26T04:07:22Z

got the tests to pass. If Alex already wrote this then I think this would be a great example of how people can build stuff with Universal. @Toxicable maybe you can turn this into a blog post

Toxicable · 2018-03-03T01:33:41Z

I've updated it but I don't know how to configure the build system.
@CaerusKaru You're going to have to fix that

CaerusKaru · 2018-03-03T02:15:13Z

@Toxicable Do you want me to push to this branch or just walk you through it?

Toxicable · 2018-03-03T02:28:11Z

@CaerusKaru Feel free to push to this branch

Toxicable · 2018-05-20T07:19:03Z

closed in favour of #1002

angular-automatic-lock-bot · 2019-09-06T00:32:57Z

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.}

Toxicable force-pushed the routes-ls branch 2 times, most recently from 78e1e63 to 8334355 Compare August 14, 2017 22:21

PatrickJS reviewed Aug 15, 2017

View reviewed changes

Toxicable force-pushed the routes-ls branch 2 times, most recently from 1eb506b to 3d046c1 Compare August 15, 2017 06:36

Toxicable force-pushed the routes-ls branch 2 times, most recently from c06886b to c58dda3 Compare August 15, 2017 09:22

ghost reviewed Aug 15, 2017

View reviewed changes

Toxicable force-pushed the routes-ls branch 5 times, most recently from 7391eb2 to 2755b92 Compare August 16, 2017 04:05

frederikprijck reviewed Aug 16, 2017

View reviewed changes

Toxicable force-pushed the routes-ls branch from 2755b92 to 4410ce8 Compare August 16, 2017 21:06

frederikprijck reviewed Aug 16, 2017

View reviewed changes

Toxicable force-pushed the routes-ls branch 5 times, most recently from 2e136e6 to 32406c9 Compare August 16, 2017 21:56

Toxicable changed the title ~~[WIP] feat(ls-routes): introduce ls-routes~~ feat(ls-routes): introduce ls-routes Aug 18, 2017

Toxicable force-pushed the routes-ls branch 6 times, most recently from ea09dbb to 93a6a46 Compare August 29, 2017 00:06

Toxicable force-pushed the routes-ls branch 2 times, most recently from 72622e3 to 62c9292 Compare September 22, 2017 10:23

Toxicable added the action: review label Oct 10, 2017

Toxicable requested a review from alxhub February 21, 2018 00:17

CaerusKaru added the cla: yes label Feb 22, 2018

CaerusKaru added action: cleanup and removed action: review labels Mar 1, 2018

Toxicable force-pushed the routes-ls branch from 62c9292 to cb361f7 Compare March 3, 2018 01:27

Toxicable requested review from CaerusKaru and vikerman as code owners March 3, 2018 01:27

Toxicable assigned CaerusKaru Mar 5, 2018

Toxicable mentioned this pull request Mar 6, 2018

Angular ng build prerendered routes like angular universal for static serving #FeatureRequest angular/angular-cli#9128

Closed

Toxicable added this to the 6.0 milestone Mar 7, 2018

Toxicable added the target: minor target: minor This PR is targeted for the next minor release label Mar 8, 2018

Toxicable unassigned CaerusKaru Mar 10, 2018

Toxicable force-pushed the routes-ls branch from 99db794 to d33d44a Compare March 10, 2018 04:48

feat(ls-routes): introduce ls-routes

b237c20

Toxicable force-pushed the routes-ls branch from d33d44a to b237c20 Compare March 10, 2018 05:04

Toxicable added state: WIP and removed action: cleanup labels Mar 10, 2018

Toxicable added the state: blocked label Mar 31, 2018

Toxicable mentioned this pull request May 20, 2018

WIP: feat(common): introduce ls-routes #1002

Closed

3 tasks

Toxicable closed this May 20, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ls-routes): introduce ls-routes #778

feat(ls-routes): introduce ls-routes #778

Toxicable commented Aug 14, 2017 •

edited

PatrickJS Aug 15, 2017

Toxicable Aug 15, 2017

PatrickJS Aug 15, 2017

PatrickJS Aug 15, 2017

Toxicable Aug 15, 2017

Toxicable commented Aug 15, 2017 •

edited

ghost Aug 15, 2017

Toxicable Aug 15, 2017

ghost Aug 15, 2017

Toxicable Aug 15, 2017 •

edited

frederikprijck Aug 16, 2017 •

edited

frederikprijck Aug 16, 2017

Toxicable commented Aug 29, 2017 •

edited

PatrickJS commented Sep 26, 2017 •

edited

Toxicable commented Mar 3, 2018

CaerusKaru commented Mar 3, 2018

Toxicable commented Mar 3, 2018

Toxicable commented May 20, 2018

angular-automatic-lock-bot bot commented Sep 6, 2019

feat(ls-routes): introduce ls-routes #778

feat(ls-routes): introduce ls-routes #778

Conversation

Toxicable commented Aug 14, 2017 • edited

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

Toxicable commented Aug 15, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Toxicable Aug 15, 2017 • edited

Choose a reason for hiding this comment

frederikprijck Aug 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Toxicable commented Aug 29, 2017 • edited

PatrickJS commented Sep 26, 2017 • edited

Toxicable commented Mar 3, 2018

CaerusKaru commented Mar 3, 2018

Toxicable commented Mar 3, 2018

Toxicable commented May 20, 2018

angular-automatic-lock-bot bot commented Sep 6, 2019

Toxicable commented Aug 14, 2017 •

edited

Toxicable commented Aug 15, 2017 •

edited

Toxicable Aug 15, 2017 •

edited

frederikprijck Aug 16, 2017 •

edited

Toxicable commented Aug 29, 2017 •

edited

PatrickJS commented Sep 26, 2017 •

edited