feature(universal): add support for Angular Universal #2318

devCrossNet · 2016-09-23T21:21:18Z

add the possibility to scaffold a universal project with --universal

googlebot · 2016-09-23T21:21:23Z

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.

hansl · 2016-09-23T23:06:26Z

Hi @devCrossNet, could you rebase this PR? It's obviously on top of previous commits, so there is some problem with it. Thanks!

googlebot · 2016-09-24T09:37:05Z

CLAs look good, thanks!

devCrossNet · 2016-09-24T09:40:06Z

@hansl I rebased the branch

intellix · 2016-09-24T11:23:40Z

packages/angular-cli/blueprints/directive/index.js

@@ -14,7 +14,7 @@ module.exports = {

  beforeInstall: function() {


Missing your "space before ()" notation :)

ritz078 · 2016-09-26T07:08:55Z

@devCrossNet guidelines to update an existing project to universal will be helpful.

devCrossNet · 2016-09-26T11:52:58Z

@ritz078 @intellix added :-)

devCrossNet · 2016-09-29T14:52:34Z

AoT is currently not working with universal.

kuldeepkeshwar · 2016-10-02T01:23:51Z

@devCrossNet what about lazy routing support ?

hansl

Some first comments.

hansl · 2016-09-30T21:11:31Z

README.md

@@ -265,7 +266,21 @@ You can modify the these scripts in `package.json` to run whatever tool you pref

 **The `--mobile` flag has been disabled temporarily. Sorry for the inconvenience.**

-~~Angular-CLI includes support for offline applications via the `--` flag on `ng new`. Support is experimental, please see the angular/mobile-toolkit project and https://mobile.angular.io/ for documentation on how to make use of this functionality.~~
+~~Angular-CLI includes support for offline applications via the `--` flag on `ng new`. Support is experimental, 


Is that line necessary at all? If it is, the flag is empty.

I'll change this to --mobile

hansl · 2016-09-30T21:35:55Z

packages/angular-cli/blueprints/ng2/files/__path__/typings.d.ts

@@ -3,3 +3,7 @@
 // https://www.typescriptlang.org/docs/handbook/writing-declaration-files.html

 declare var System: any;
+<% if(universal) { %>
+/// <reference path="../node_modules/@types/node/index.d.ts" />


Instead, include "types": [ "node" ] in the tsconfig. I don't want any relative /// references.

hansl · 2016-09-30T21:39:11Z

packages/angular-cli/utilities/find-parent-module.ts

-    } else if (files.length > 1) {
-      throw new SilentError(`Multiple module files found: ${pathToCheck.replace(sourceRoot, '')}`);
+    if (files.length > 0) {
+      return files.map((file) => {


Single line, without the return. return files.map(file => path.join(pathToCheck, file));

hansl · 2016-09-30T21:41:01Z

packages/angular-cli/blueprints/ng2/files/angular-cli.json

-      "main": "main.ts",
+      "index": "index.html",<% if(!universal) { %>
+      "main": "main.ts",<% } %><% if(universal) { %>
+      "main": "client.ts",


Why a new client file instead of keeping/overwriting main? Is there a reason you don't want main as the output of your app in universal? I understand the semantic differences, I'm just wondering if there's a technical reason.

I did not want to change so much in the CLI code. We can talk about a strategy to rename files. Because I need the main.ts file as client.ts in the universal context. This was the easiest solution for me to not implement "renaming files".

TheLarkInn · 2016-10-04T14:21:41Z

packages/angular-cli/models/webpack-build-node.ts

+          test: /\.js$/,
+          loader: 'source-map-loader',
+          exclude: [
+            path.resolve(projectRoot, 'node_modules/rxjs'),


Exclude all node modules sourcemaps, its faster, more reliable, especially as more packages get added, like angularfire material etc. Users won't be able to turn these off, so we need to ignore them all.

I'll do that. Thanks.

TheLarkInn · 2016-10-04T14:23:02Z

packages/angular-cli/models/webpack-build-node.ts

+    module: {
+      rules: [
+        {
+          test: /\.js$/,


This used to be a preLoader, (or ideally?), if soo add enforce: 'pre'

yes, it was a preloader. but the preloader config attribute is not available anymore?! I got an error. I will use enforce: 'pre' thank you!

TheLarkInn · 2016-10-04T16:05:59Z

packages/angular-cli/tasks/build-webpack.ts

+
+        const fileName = path.resolve(outputDir, appConfig.index);
+        const data = fs.readFileSync(fileName, 'utf8').replace(
+          '<script src="http://localhost:35729/livereload.js?snipver=1"></script>',


what and why

I tried to replace it with webpack in the webpack-build-prod config. but I did not get it to work. I used the string-replacement plugin. I'll try it again. I just want to remove the live-reload-script in production.

TheLarkInn · 2016-10-04T16:08:02Z

packages/angular-cli/tasks/server/universal-dev-server.ts

+        console.log(stderr.toString());
+      });
+
+    this.webpackCompiler.plugin('compile', () => {});


this technically should hang and break compiling because compile requires a callback which is passed in, in addition to the compiler instance. Why is this and the rest of these events being done.

I just copied code from webpack-dev-server, removed unused code in the invalid-plugin and the left over was an empty function. I'll try to remove this. https://github.com/webpack/webpack-dev-server/blob/master/lib/Server.js#L30

TheLarkInn · 2016-10-04T16:08:46Z

packages/angular-cli/tasks/server/universal-dev-server.ts

+  private demon: any;
+  private liveServer: any;
+
+  constructor(private webpackCompiler: any, private webpackDevServerConfiguration: any) {


What is the purpose for not using webpack-dev-server. Additionally, if you are wanting to use an express server there is webpack-dev-middleware as well which has a lot of the api configuration to hook webpack into your existing express server.

If the concern is trying to hide the webpack-dev-middleware from the user, I'd rather see that being used in server.ts and then packaged/blackboxed universal module which runs the dev-middleware

I am not familiar with the webpack-dev-middleware. Is it possible to reload the whole server code when somethings change? Because in universal the server.ts file is part of the business logic and for DX it must reload the server code when the user changes something. I'll have a look at it. maybe we can chat or something?

filipesilva · 2016-10-04T19:09:18Z

package.json

@@ -20,6 +20,7 @@
    "test:cli": "node tests/runner",
    "test:inspect": "node --inspect --debug-brk tests/runner",
    "test:packages": "node scripts/run-packages-spec.js",
+    "test:universal": "UNIVERSAL=true node tests/e2e_runner.js",


These tests are not being ran in the CI atm, you might have to fiddle with travis.yml to see them run.

I know, but I was not sure if you want to have the universal tests in the master branch. But now as we have a universal branch, I'll enable the test.

xzilja · 2016-10-14T09:09:24Z

Hello, I believe myself and few other people have been following this feature for a while. Wanted to check in if there is any update on progress for universal support + if you guys need help with anything?

devCrossNet · 2016-10-14T09:51:19Z

@iljadaderko I am on it. I hope I can get the requested changes (most of them) in this weekend. after that, I have to wait for another review.

kuldeepkeshwar · 2016-10-14T14:22:44Z

@devCrossNet any idea how will you be handling lazy routes (code split points) on server ?

add the possibility to scaffold a universal project with --universal

devCrossNet · 2016-10-14T14:45:11Z

@kuldeepkeshwar I don't know. I have to talk about it with the team

Brocco · 2016-10-14T19:36:56Z

Closing this PR to master, as this is being tracked/managed in a separate branch.

hongbo-miao · 2016-10-14T19:39:16Z

@Brocco can you put the new link here so we can track? thanks

CanKattwinkel · 2016-10-14T19:40:50Z

https://github.com/angular/angular-cli/tree/universal

chrillewoodz · 2016-10-17T12:32:36Z

Where can we track this? I need this available asap.

hongbo-miao · 2016-10-17T19:21:07Z

@chrillewoodz I guess this one now: #2749

angular-automatic-lock-bot · 2019-09-10T04:42:01Z

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 Sep 23, 2016

Brocco added the pr_action: rebase label Sep 24, 2016

devCrossNet force-pushed the universal-webpack branch from 0baf833 to 5989dd2 Compare September 24, 2016 09:37

googlebot added cla: yes and removed cla: no labels Sep 24, 2016

intellix reviewed Sep 24, 2016

View reviewed changes

devCrossNet mentioned this pull request Sep 24, 2016

spec out universal plugin to angular CLI angular/universal#260

Closed

Brocco added pr_action: review and removed pr_action: rebase labels Sep 25, 2016

devCrossNet force-pushed the universal-webpack branch from 5989dd2 to 34ab9ad Compare September 26, 2016 11:51

devCrossNet force-pushed the universal-webpack branch 3 times, most recently from d5fac08 to f2aa290 Compare September 27, 2016 18:22

MarkPieszak mentioned this pull request Sep 27, 2016

Integrating angular2 Universal with angular CLI project #1050

Closed

devCrossNet force-pushed the universal-webpack branch 2 times, most recently from 0413923 to 9a2a0df Compare September 29, 2016 14:51

devCrossNet force-pushed the universal-webpack branch from 9a2a0df to c23b869 Compare September 30, 2016 18:24

devCrossNet changed the title ~~(feat): Universal Angular~~ feature(universal): add support for Angular Universal Oct 1, 2016

hansl suggested changes Oct 3, 2016

View reviewed changes

devCrossNet force-pushed the universal-webpack branch 3 times, most recently from 7639230 to 611f337 Compare October 3, 2016 17:33

devCrossNet force-pushed the universal-webpack branch from 611f337 to 516d576 Compare October 3, 2016 19:15

jeffbcross mentioned this pull request Oct 3, 2016

Universal Version going along with CLI 1.0.0-beta.16? angular/universal#569

Closed

TheLarkInn reviewed Oct 4, 2016

View reviewed changes

devCrossNet changed the base branch from master to universal October 4, 2016 18:41

filipesilva reviewed Oct 4, 2016

View reviewed changes

devCrossNet force-pushed the universal-webpack branch from 516d576 to 98fe28f Compare October 14, 2016 14:06

feature(universal): add support for Angular Universal

b155283

add the possibility to scaffold a universal project with --universal

devCrossNet force-pushed the universal-webpack branch from 98fe28f to b155283 Compare October 14, 2016 14:27

Brocco closed this Oct 14, 2016

devCrossNet mentioned this pull request Oct 17, 2016

feature(universal): add support for Angular Universal #2749

Merged

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

		@@ -14,7 +14,7 @@ module.exports = {

		beforeInstall: function() {

feature(universal): add support for Angular Universal #2318

feature(universal): add support for Angular Universal #2318

Conversation

devCrossNet commented Sep 23, 2016

googlebot commented Sep 23, 2016

hansl commented Sep 23, 2016

googlebot commented Sep 24, 2016

devCrossNet commented Sep 24, 2016

Choose a reason for hiding this comment

ritz078 commented Sep 26, 2016

devCrossNet commented Sep 26, 2016

devCrossNet commented Sep 29, 2016

kuldeepkeshwar commented Oct 2, 2016

hansl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devCrossNet Oct 3, 2016 • 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xzilja commented Oct 14, 2016

devCrossNet commented Oct 14, 2016

kuldeepkeshwar commented Oct 14, 2016

devCrossNet commented Oct 14, 2016

Brocco commented Oct 14, 2016

hongbo-miao commented Oct 14, 2016

CanKattwinkel commented Oct 14, 2016 • edited

chrillewoodz commented Oct 17, 2016

hongbo-miao commented Oct 17, 2016

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

devCrossNet Oct 3, 2016 •

edited

CanKattwinkel commented Oct 14, 2016 •

edited