Update Angular2Spa template to Angular 2.0/TypeScript 2.0, based on @MarkPieszak's PR #321

SteveSandersonMS · 2016-09-19T14:46:48Z

Following on from @MarkPieszak's PR #319, I've simplified the template further as far as I think makes sense at the moment.

Putting this out as a PR for feedback before merging and publishing.

…reference (no longer needed).

hheexx · 2016-09-19T14:57:29Z

why is @types/node not added to devDependencies with other @types ?

SteveSandersonMS · 2016-09-19T14:59:53Z

@MarkPieszak, do you have any feedback on the simplifications/changes I've made here? Your PR was extremely helpful. On top of that, the main things I wanted to change were:

Making HMR work
Removing files/folders and NPM dependencies that we can avoid the need for
Adjusting the filename/folder conventions to more closely match those emitted by angular-cli
Simplifying boot code (just because few developers are going to understand this at first glance)
Simplifying webpack config (eliminating dev/prod override files). I have plans to simplify this further, but will do that when I get time to update all the templates, not just the Angular one.

I also attempted to get the Angular 2 compiler (ngc), or at least Webpack 2 tree shaking, to work with this, but completely failed. It doesn't seem like ngc and arbitrary Webpack configs work together right now, so that will have to wait, unless someone else has a way to make it work! There are a bunch of experimental webpack-ngc-loader type things on GitHub right now, but it's not clear that any of them really work or will be supported in the long run.

@hheexx - check the later commits. It's in dependencies because it's needed at runtime if you're doing server-side prerendering.

MarkPieszak · 2016-09-19T15:04:01Z

templates/Angular2Spa/ClientApp/boot-server.ts

    });

+    return requestZone.run<Promise<string>>(() => platform.serializeModule(AppModule))


Much cleaner I like it 👍

MarkPieszak · 2016-09-19T15:07:40Z

templates/Angular2Spa/ClientApp/app/app.module.ts

+        HomeComponent
+    ],
+    imports: [
+        UniversalModule,


Can we leave a Note that UniversalModule should be the first Import? (Just want to prevent common errors that can creep up from this)

Also, we should point out that BrowserModule HttpModule JsonpModule are all automatically included here.
(The same goes for the Node versions on the main.server.ts side, NodeModule / NodeHttpModule etc)
Things can error out if they include those. We're working on looking into fixing that.

Patrick also thought it'd be nice to keep the main files separate since that allows someone to have specific platform logic injected for either browser/server, but it's up to you. It's definitely cleaner/simpler this way!

Please keep server and client main files separate.
For me they diverged very early in development. And I am pretty sure it will for most users.

Added the comment about UniversalModule (and BrowserModule/HttpModule/JsonpModule)

main files separate

I'd hope that the boot files serve that purpose well enough (this is where you configure DI, after all), and am reluctant to add more duplicate files. Having one root @NgModule makes things much clearer, especially given how much tedious config goes into that file. We can monitor this and see if people often report issues that are best solved by having separate root modules.

Can we have logic in main file based on platform? Then we don't need 2 files.

if (server) {
declarations: [......]
} else if (client) {
declarations: [.....]
}

?

@hheexx Yes, just import { isNode, isBrowser } from 'angular2-universal'; and then do if (isNode) { ... } etc.

That is sweet. I would add it as a comment so people know.

hheexx · 2016-09-19T15:07:49Z

Oh.. I see they are removed in later commit.

MarkPieszak · 2016-09-19T15:19:16Z

templates/Angular2Spa/ClientApp/boot-server.ts

-    const _config = Object.assign({
-      get cancel() { return cancel; },
-      cancelHandler() { return Zone.current.get('cancel') }
-    }, platformConfig);


Only thing I'm worried about is the Zone canceling itself if errors/issues occur.
Hence these cancelHandler's being added to the zone.
Let me look into it!

OK, please let me know if you think something needs to be added for this.

MarkPieszak · 2016-09-19T15:19:44Z

templates/Angular2Spa/ClientApp/main.browser.ts

-
-    // * NOTE: Needs to be your first import (!)
-    UniversalModule, 
-    // * ^ BrowserModule, HttpModule, and JsonpModule are included here


These notes would be nice to keep to avoid issues if people think they need to add them in.

MarkPieszak · 2016-09-19T15:28:13Z

@SteveSandersonMS
Added a few comments above just now, otherwise I like it!!
Cleaned it up nicely 😀
I was focused on getting it working this weekend didn't have time to gut out unnecessary code, apologies.

One thing recommended by the Angular team is DOMContentLoaded. You don't want angular bootstrapping prior to that, it can cause sporadic/bizarre issues. Can we leave that in there?

document.addEventListener('DOMContentLoaded', () => { 
    platformRef.bootstrapModule(MainModule);
});

Another thing I thought might be beneficial is leaving those comments about UniversalModule. It DepInjects several modules (for you), and if they're included on top of it, it can error things out. (We're looking into it fixing this or at least providing better error messaging for it)

As for AOT Compilation, right now we don't have it fully working, it would be additional code needed in the boot-server, but there's more that needs to be done to get that working on our end.

// eventually it'll be somewhat like this
 return zone.run(() => (_options.precompile ?
    platformRef.serializeModule(ngModule, _data) :
    platformRef.serializeModuleFactory(ngModule, _data) 
  )

Webpack2 does work with Universal, what problems were you getting with it, we can look into it another time.

…t all. Is used by other templates though.

SteveSandersonMS · 2016-09-19T16:04:26Z

One thing recommended by the Angular team is DOMContentLoaded

Thanks for the feedback. Done.

As for AOT Compilation, right now we don't have it fully working, it would be additional code needed in the boot-server, but there's more that needs to be done to get that working on our end.

The issue isn't just making it work at runtime in Univeral. Right now I'm not successful in being able to actually do the AOT compilation at all, because it's not generally compatible with arbitrary webpack configs (e.g., using raw-loader and require to reference template .html files).

Webpack2 does work with Universal, what problems were you getting with it

No, that part works fine. I was meaning to say that the end-to-end story with Webpack 2 tree shaking isn't usable right now because of the inability to precompile Angular 2 templates.

SteveSandersonMS · 2016-09-19T16:43:16Z

Thanks for the feedback! This is now merged into the default branch. I'll publish actual Yeoman templates tomorrow.

MarkPieszak · 2016-09-19T17:35:43Z

Excellent! :)
If we need to circle back and add the cancelHandler stuff later we can take care of it, for now at least this should be good enough! I'm still looking into it how important it is, and if it's possible for us to handle it internally with Universal instead perhaps.

Cheers.

MarkPieszak and others added 11 commits September 19, 2016 09:12

ng2 2.0, Universal 2.0, TS 2.0, Preboot 4.*

ce0d208

Remove now-redundant 'typings' dir and custom-typings.d.ts

1e08548

Add @types/node to avoid intellisense errors for "require" statements

243a9b4

Simplify Angular 2 template where possible

8f550c5

Move more modules to vendor bundle. Remove explicit reflect-metadata …

297b4db

…reference (no longer needed).

Simplify imports in boot-client.ts

a1c1bdb

Make HMR work again

a91b6a6

Move tsconfig into ClientApp dir, since it's not needed at root

85dfdd9

Simplify webpack config. Eliminate dev/prod override files.

4ea7eb1

Remove unnecessary NPM dependencies

07a9c16

Make indentation consistent

358ee22

dnfclas added the cla-required label Sep 19, 2016

Update angular2-universal dependencies (cherry-pick 62dd13b)

49a8536

SteveSandersonMS mentioned this pull request Sep 19, 2016

ng2 2.0, Universal 2.0, TS 2.0, Preboot 4.* #319

Closed

MarkPieszak reviewed Sep 19, 2016

View reviewed changes

Fix trailing whitespace

494c7b5

MarkPieszak reviewed Sep 19, 2016

View reviewed changes

Add comment about UniversalModule

83cfb59

MarkPieszak reviewed Sep 19, 2016

View reviewed changes

SteveSandersonMS added 2 commits September 19, 2016 16:53

Remove style-loader from Angular2Spa vendor bundle as it's not used a…

ca99a23

…t all. Is used by other templates though.

Delay Angular 2 bootstrapping until DOMContentLoaded

41f1f6f

SteveSandersonMS merged commit 41f1f6f into dev Sep 19, 2016

SteveSandersonMS deleted the angular2-final branch September 19, 2016 16:43

snyk-bot mentioned this pull request Dec 19, 2020

[Snyk] Upgrade ts-loader from 0.8.2 to 0.9.5 hixio-mh/JavaScriptServices#4

Open

harunpehlivan mentioned this pull request Jun 30, 2022

[Snyk] Security upgrade selenium-standalone from 5.11.2 to 6.15.0 harunpehlivan/JavaScriptServices#81

Open

		});

		return requestZone.run<Promise<string>>(() => platform.serializeModule(AppModule))

Update Angular2Spa template to Angular 2.0/TypeScript 2.0, based on @MarkPieszak's PR #321

Update Angular2Spa template to Angular 2.0/TypeScript 2.0, based on @MarkPieszak's PR #321

Uh oh!

Conversation

SteveSandersonMS commented Sep 19, 2016

Uh oh!

hheexx commented Sep 19, 2016

Uh oh!

SteveSandersonMS commented Sep 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

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!

hheexx commented Sep 19, 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!

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

Uh oh!

Uh oh!

SteveSandersonMS commented Sep 19, 2016

Uh oh!

SteveSandersonMS commented Sep 19, 2016

Uh oh!

MarkPieszak commented Sep 19, 2016

Uh oh!

Uh oh!

MarkPieszak Sep 19, 2016 •

edited

Loading

SteveSandersonMS Sep 19, 2016 •

edited

Loading

MarkPieszak commented Sep 19, 2016 •

edited

Loading