docs(toh-6-aot): add aot to toh-6 #2496

thelgevold · 2016-09-28T03:44:43Z

Tor says: "... AoT CB is ready to go"

thelgevold · 2016-09-29T03:01:13Z

This is still pending angular/in-memory-web-api#36, but here is my first stab at supporting both AoT and JiT in the same code base. We can improve on this, but this is at least a starting point.

AoT
I have added a new project with no source code under toh-6-aot. I went in this direction to allow both compilation modes to be active at the same time (with e2e tests).

There is no source code in the new project, but there is a new index.html file pointing to the generated bundle. The bundle is generated from the source in the original toh-6 folder. Trigger a build by running npm run build:toh-aot from toh-6-aot/ts

JiT
The JiT code still lives in its original location, but I have added two sets of main.ts and tsconfigs over there. Trigger a JiT build from toh-6/ts by running npm run build:toh-jit.

@wardbell @johnpapa @brandonroberts @Foxandxss @filipesilva

cc: @robwormald

thelgevold · 2016-09-30T03:58:35Z

I made a few tweaks based on discussions with @wardbell

Instead of adding a new tsconfig.json I have made a few tweaks to the shared config
This also includes an upgrade of in-mem-web-api

The remaining part for this cookbook is to add a section to the prose.

filipesilva · 2016-10-03T10:53:03Z

@thelgevold I've reviewed your files and have a few questions:

you mentioned that after talking with Ward, you had opted to tweaking the shared tsconfg instead of adding a new one. In this PR I still see 3 tsconfig total, can you elaborate?
can this approach handle lazy loaded modules?

I'm also thinking of generalizing this approach to run AoT over all of our examples in a separate e2e CI instance.

thelgevold · 2016-10-04T00:03:27Z

@filipesilva The third tsconfig is obsolete now, so I have deleted it.
Currently this does not address lazy loading of modules. I was thinking we could address that in a subsequent revision.

filipesilva · 2016-10-04T00:19:31Z

Addressing lazy loading separately is fine by me, just wanted to confirm.

thelgevold · 2016-10-04T04:54:20Z

@wardbell I have updated the prose and removed the WIP part of the title.
This PR is ready for review.

brandonroberts · 2016-10-04T13:19:50Z

public/docs/_examples/toh-6/ts/app/app.module.ts

@@ -12,7 +12,7 @@ import { HttpModule }    from '@angular/http';

 // #enddocregion v1
 // Imports for loading & configuring the in-memory web api
-import { InMemoryWebApiModule } from 'angular-in-memory-web-api';
+import { InMemoryWebApiModule } from 'angular-in-memory-web-api/in-memory-web-api.module';


Is there a reason we have to import this way instead of just importing from angular-in-memory-web-api?

For some reason we get an issue with rollup not finding the module if we rely on just the barrel. If I give it the fully qualified name like this it works. It's a bit odd, but seems to be a rollup thing.

Ok. It should be noted as a caveat to rollup if this is not the way it has to be done normally unless we figure out a way to use the barrel export.

I wonder if this is happening because the code is already converted to JavaScript by the time it enters the rollup world. I guess the barrel is transpiled as well, but maybe Rollup isn't considering the convention of using barrels...

brandonroberts · 2016-10-04T13:21:40Z

public/docs/ts/latest/cookbook/aot-compiler.jade

+
+  Tour of Heroes has an external dependency on the `angular-in-memory-web-api` library.
+
+  We can't expect all libraries to be released with AoT in mind, but `angular-in-memory-web-api` has already been AoT compiled and released with the necessary dependencies to integrate it with another AoT compiled application.


I think we should expect this since one non-AoT provided library prevents AoT altogether. Maybe that is addressed in a separate cookbook on creating a third party library.

Hm..Does this mean that any third part angular lib has to be AoT-ed for AoT to work? We are integrating rxjs which is non-AoT, but that is not Angular....

I mean in the context of the Angular world. Any third-party library that provides an NgModule/Component would need to be AoT compiled for the host application to be able to use it with AoT.

Third-party libraries that aren't Angular-specific would not need to be AoT compiled.

I think I agree that we should have a separate cookbook talking about creating third party libraries. I removed the sentence that said we shouldn't expect AoT since it seems to be a prerequisite.

wardbell · 2016-10-05T23:49:14Z

I am rebasing and reviewing and will force push so beware

googlebot · 2016-10-06T23:11:43Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-10-06T23:11:43Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-10-07T00:45:51Z

CLAs look good, thanks!

googlebot · 2016-10-07T00:45:51Z

CLAs look good, thanks!

filipesilva · 2016-10-07T11:26:13Z

@thelgevold there were a few setup changes in #2233, which went in yesterday.

When rebasing pay special attention to public/docs/_examples/_boilerplate/tsconfig.json, which is the new master tsconfig for example. It contains the "types": [] property that forces tsc/ngc to ignore any @types present in node_modules.

Since you create new tsconfigs for your examples you must add this property, otherwise you will get a lot of duplicate identifier errors.

thelgevold · 2016-10-07T13:12:48Z

The only item I am not clear on is the No obvious way for readers to get the samples part.
Have to think about improvements here.

thelgevold · 2016-10-08T18:09:14Z

I have fixed the typings issue and renamed the rollup.js file to rollup-config.js

johnpapa · 2016-10-08T18:16:35Z

should it be rollup.config.js. or rollup-config.js ? which works in @wardbell 's case?

thelgevold · 2016-10-08T18:20:55Z

I think either will work. My impression is that on windows it can't be named the same as the binary (rollup)

thelgevold · 2016-10-08T19:40:16Z

I switched to using the AppRouterModule and there were no issues with AoT

johnpapa

Fixed my issues. Thanks!

johnpapa · 2016-10-10T18:11:38Z

We should make sure the cookbook prose and code samples are updated too.

johnpapa · 2016-10-10T22:53:07Z

public/docs/_examples/package.json

@@ -74,6 +74,7 @@
    "raw-loader": "^0.5.1",
    "rimraf": "^2.5.4",
    "rollup-plugin-commonjs": "^4.1.0",
+    "source-map-explorer": "^1.3.2",


do we need this in the examples? its cool but are we using it here?

Yeah, the prose talks about how to use it, so I think it makes sense to install it with the sample

s s s s s

googlebot · 2016-10-12T07:55:58Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-10-12T07:55:58Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

Updates tooling for this purpose and also the prose.

wardbell · 2016-10-12T08:27:00Z

I've extended the gulpfile's e2e facility so it can run the same e2e.specs against the AoT build of an app when ~/aot/index.html is defined.

It's primitive and rigid but it works as seen in the toh-6 in his PR. That means no need for a separate aot project folder (toh-6-aot is gone).

The "trick" is that the AoT version of the index.html is in the /aot folder and to run it you serve from within /aot!

Still don't have a good way to demonstrate this to our readers. The instructions in the prose describe what to do if you've cloned the repo.

googlebot added the CLA: yes label Sep 28, 2016

thelgevold force-pushed the toh-6-aot-tor branch 2 times, most recently from 9404fbc to 3d187c3 Compare September 29, 2016 02:46

thelgevold force-pushed the toh-6-aot-tor branch 3 times, most recently from 9cbfeeb to c7cf95b Compare September 30, 2016 03:19

thelgevold force-pushed the toh-6-aot-tor branch from c7cf95b to b4310d9 Compare October 4, 2016 00:01

thelgevold changed the title ~~WIP: docs(toh-6-aot): add aot to toh-6~~ docs(toh-6-aot): add aot to toh-6 Oct 4, 2016

thelgevold force-pushed the toh-6-aot-tor branch from a92330e to c824c40 Compare October 4, 2016 05:11

brandonroberts reviewed Oct 4, 2016

View reviewed changes

thelgevold force-pushed the toh-6-aot-tor branch from c824c40 to fb20888 Compare October 5, 2016 01:08

wardbell force-pushed the toh-6-aot-tor branch 2 times, most recently from b7b71cb to f6c21a7 Compare October 5, 2016 23:48

wardbell force-pushed the toh-6-aot-tor branch 3 times, most recently from 93c55b9 to 187797c Compare October 6, 2016 22:30

googlebot added CLA: no and removed CLA: yes labels Oct 6, 2016

wardbell force-pushed the toh-6-aot-tor branch from d33808a to f7b3c50 Compare October 7, 2016 00:45

thelgevold force-pushed the toh-6-aot-tor branch 3 times, most recently from 754056b to b701379 Compare October 8, 2016 18:07

thelgevold force-pushed the toh-6-aot-tor branch 2 times, most recently from 5cc6145 to 6fa0a05 Compare October 8, 2016 19:12

johnpapa reviewed Oct 8, 2016

View reviewed changes

Foxandxss mentioned this pull request Oct 10, 2016

Bug in Windows: rollup.js config file. #2507

Closed

johnpapa reviewed Oct 10, 2016

View reviewed changes

thelgevold added 2 commits October 11, 2016 16:46

docs(toh-6-aot): add aot to toh-6

cff8876

s s s s s

docs(toh-6-aot): add aot to toh-6

7c2ef49

wardbell force-pushed the toh-6-aot-tor branch from 3403364 to 7aeee03 Compare October 12, 2016 07:55

googlebot added CLA: no and removed CLA: yes labels Oct 12, 2016

wardbell force-pushed the toh-6-aot-tor branch from 7aeee03 to 003973b Compare October 12, 2016 08:19

docs(toh-6-aot): make aot e2e tests run in the same project

d1c3c9c

Updates tooling for this purpose and also the prose.

wardbell force-pushed the toh-6-aot-tor branch from 003973b to d1c3c9c Compare October 12, 2016 08:23

wardbell merged commit cdfe957 into angular:master Oct 12, 2016

wardbell deleted the toh-6-aot-tor branch October 12, 2016 08:44


		Tour of Heroes has an external dependency on the `angular-in-memory-web-api` library.

		We can't expect all libraries to be released with AoT in mind, but `angular-in-memory-web-api` has already been AoT compiled and released with the necessary dependencies to integrate it with another AoT compiled application.

docs(toh-6-aot): add aot to toh-6 #2496

docs(toh-6-aot): add aot to toh-6 #2496

Uh oh!

Conversation

thelgevold commented Sep 28, 2016 • edited by wardbell Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thelgevold commented Sep 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thelgevold commented Sep 30, 2016

Uh oh!

filipesilva commented Oct 3, 2016

Uh oh!

thelgevold commented Oct 4, 2016

Uh oh!

filipesilva commented Oct 4, 2016

Uh oh!

thelgevold commented Oct 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wardbell commented Oct 5, 2016

Uh oh!

googlebot commented Oct 6, 2016

Uh oh!

googlebot commented Oct 6, 2016

Uh oh!

googlebot commented Oct 7, 2016

Uh oh!

googlebot commented Oct 7, 2016

Uh oh!

filipesilva commented Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thelgevold commented Oct 7, 2016

Uh oh!

thelgevold commented Oct 8, 2016

Uh oh!

johnpapa commented Oct 8, 2016

Uh oh!

thelgevold commented Oct 8, 2016

Uh oh!

thelgevold commented Oct 8, 2016

Uh oh!

johnpapa left a comment

Choose a reason for hiding this comment

Uh oh!

johnpapa commented Oct 10, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

googlebot commented Oct 12, 2016

Uh oh!

googlebot commented Oct 12, 2016

Uh oh!

wardbell commented Oct 12, 2016

Uh oh!

Uh oh!

thelgevold commented Sep 28, 2016 •

edited by wardbell

Loading

thelgevold commented Sep 29, 2016 •

edited

Loading

filipesilva commented Oct 7, 2016 •

edited

Loading