feat(@angular/cli): add ability to build bundle for node #6913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

hansl merged 1 commit into angular:master from FrozenPandaz:serverbuild

Jul 20, 2017

Contributor

FrozenPandaz commented Jul 7, 2017 •

edited

Loading

This adds the ability to create a commonjs bundle which contains the application module/ngfactory which can be used in node environments

Here's an example of how a project using this feature would look https://github.com/FrozenPandaz/u-cli-app

googlebot added the cla: yes label

FrozenPandaz force-pushed the serverbuild branch 3 times, most recently from 31c8e93 to 160ca2c Compare

July 10, 2017 23:57

hansl previously requested changes

View reviewed changes

Contributor

hansl left a comment

1 question for now

tests/e2e/assets/webpack/test-server-app/app/injectable.ts

@@ @@ -4,5 +4,5 @@ import {DOCUMENT} from '@angular/platform-browser'; @@
               @Injectable()
               export class MyInjectable {
-                constructor(public viewContainer: ViewContainerRef, @Inject(DOCUMENT) public doc) {}

Contributor

hansl Jul 11, 2017

?

Contributor Author

FrozenPandaz Jul 11, 2017

VCR is not an ordinary injectable

It can only be injected from a component and it's value is the VCR of the specific instance of the component.

A service/ injectable is not able to inject a VCR

Contributor

hansl Jul 11, 2017

This is to test the downgrading of a non-annotated Angular class. I think the test needs to happen, I don't care much with which class. Does this test makes your code fail? How so?

Contributor Author

FrozenPandaz Jul 19, 2017

This test is done in the normal webpack tests.

This test makes my tests fail because I am actually executing the application and it fails to bootstrap because the service is not able to find the VCR.

Is it fine to remove it from this set of tests because the functionality is covered in the other webpack tests?

FrozenPandaz force-pushed the serverbuild branch 5 times, most recently from df8d278 to 2602917 Compare

July 11, 2017 04:11

hansl reviewed

View reviewed changes

tests/e2e/tests/packages/webpack/server.ts Outdated

                              + 'type: undefined, decorators.*Inject.*args: .*DOCUMENT.*'))
                   .then(() => expectFileToMatch('dist/app.main.js',
                     new RegExp('AppComponent.ctorParameters = .*MyInjectable'))
                   .then(() => expectFileToMatch('dist/app.main.js',
                     /AppModule \*\/\].*\.testProp = \'testing\'/))
                   .then(() => expectFileToMatch('dist/app.main.js',
-                    /renderModuleFactory \*\/\].*\/\* AppModuleNgFactory \*\/\]/))
+                      /renderModuleFactory \*\/\].*\/\* AppModuleNgFactory \*\/\]/))
+                  .then(() => exec(normalize('rm'),

Contributor

hansl Jul 11, 2017

Considering there's no path, normalize('rm') is a noop.

hansl reviewed

View reviewed changes

packages/@angular/cli/lib/config/schema.json

@@ @@ -83,6 +83,11 @@ @@
                           "type": "string",
                           "description": "Base url for the application being built."
                         },
+                        "platform": {
+                          "type": "string",

Contributor

hansl Jul 12, 2017

This should be an enum, not a generic stsring.

Contributor Author

FrozenPandaz Jul 18, 2017

This is done.

FrozenPandaz force-pushed the serverbuild branch 2 times, most recently from eb675f2 to 04d605e Compare

July 14, 2017 06:14

hansl reviewed

View reviewed changes

packages/@angular/cli/tasks/eject.ts Outdated

@@ @@ -436,6 +436,9 @@ export default Task.extend({ @@
                   if (project.root === path.resolve(outputPath)) {
                     throw new SilentError ('Output path MUST not be project root directory!');
                   }
+                  if (appConfig.platform && appConfig.platform === 'server') {

Contributor

hansl Jul 17, 2017

appConfig.platform is redundant here.

hansl reviewed

View reviewed changes

packages/@ngtools/webpack/src/loader.ts Outdated

                       if (!plugin.skipCodeGeneration) {
                         return Promise.resolve()
                           .then(() => _removeDecorators(refactor))
-                          .then(() => _refactorBootstrap(plugin, refactor));
+                          .then(() => _refactorBootstrap(plugin, refactor))
+                          .then(() => _stuff(plugin, refactor))

Contributor

hansl Jul 17, 2017

_stuff? I'm sure you can do better ;)

FrozenPandaz force-pushed the serverbuild branch 4 times, most recently from d6e695d to ed7f5f0 Compare

July 18, 2017 03:07

alxhub reviewed

View reviewed changes

packages/@ngtools/webpack/src/loader.ts Outdated

+                    });
+                    const dirName = path.normalize(path.dirname(refactor.fileName));
+                    let exportStatement = `export const moduleMap = ${JSON.stringify(map)};`;

Member

alxhub Jul 18, 2017

Change this to not use JSON.stringify and instead properly build up the export map without string replacement hacks.

Member

alxhub Jul 18, 2017

Also, let's rename the exported symbol to LAZY_MODULE_FACTORY_MAP.

alxhub reviewed

View reviewed changes

packages/@ngtools/webpack/src/loader.ts Outdated

+                        return {
+                          loadChildrenString: routeKey,
+                          path: plugin.lazyRoutes[lazyRouteKey],
+                          name: routeKey.split('#')[1]

Member

alxhub Jul 18, 2017

Add validation that the loadChildren format is correct.

FrozenPandaz force-pushed the serverbuild branch 2 times, most recently from e127c52 to a2d4ddb Compare

July 19, 2017 01:22

alxhub dismissed hansl’s stale review

July 19, 2017 18:43

Jason has addressed the issues

alxhub requested changes

View reviewed changes

packages/@ngtools/webpack/src/loader.ts Outdated

+                        refactor.prependBefore(node, `import * as __lazy_${index}__ from './${relativePath}'`);
+                        return `"${routeKey}": __lazy_${index}__.${moduleName}`;
+                      })
+                      .join();

Member

alxhub Jul 19, 2017

Wait, how are these separated. Shouldn't it be joined with a comma or something?

Member

alxhub Jul 19, 2017

Never mind, apparently comma is the default for join().

alxhub reviewed

View reviewed changes

packages/@ngtools/webpack/src/loader.ts Outdated

+                        const modulePath = plugin.lazyRoutes[lazyRouteKey];
+                        const relativePath = path.relative(dirName, modulePath).replace(/\\/g, '/');
+                        refactor.prependBefore(node, `import * as __lazy_${index}__ from './${relativePath}'`);

Member

alxhub Jul 19, 2017

It's weird for .map to have side effects. Can we move this out to a separate .forEach iteration through the keys?

packages/@ngtools/webpack/src/loader.ts Outdated

+                        refactor.prependBefore(node, `import * as __lazy_${index}__ from './${relativePath}'`);
+                        return `"${routeKey}": __lazy_${index}__.${moduleName}`;
+                      })
+                      .join();

Member

alxhub Jul 19, 2017

Never mind, apparently comma is the default for join().

FrozenPandaz force-pushed the serverbuild branch 2 times, most recently from eafa684 to b6ae689 Compare

July 20, 2017 18:53

alxhub previously approved these changes

View reviewed changes


          feat(@angular/cli): add ability to build bundle for node and export l…

f089a33

…azy route map

FrozenPandaz dismissed alxhub’s stale review via

f089a33

July 20, 2017 19:12

FrozenPandaz force-pushed the serverbuild branch from b6ae689 to f089a33 Compare

July 20, 2017 19:12

Contributor

hansl commented Jul 20, 2017

Just confirmed locally that the e2e tests are flakes and everything is green. Cheers.

hansl merged commit 6f23636 into angular:master

intellix mentioned this pull request

feat(@angular/cli): add platform: server #5547

Closed

Contributor

intellix commented Jul 22, 2017 •

edited

Loading

Was just playing with this and found an issue with a particular export pattern in barrels that is broken: intellix/universal-cli@8ed967d#diff-a44e09a3c12667be300e8cb75d793c5dR1

Worked inside @app/user/index.ts:

export * from './user.module';
export * from './user.service';

Didn't work inside app/user/index.ts:

export { UserModule } from './user.module';
export { UserService } from './user.service';

Build error:

Date: 2017-07-22T06:19:07.696Z
Hash: f089fcd9ebdfb15dd64e
Time: 2950ms
chunk {main} main.bundle.js (main) 20.8 kB [entry] [rendered]
chunk {polyfills} polyfills.bundle.js (polyfills) 6.35 kB [entry] [rendered]
chunk {styles} styles.bundle.js (styles) 13.7 kB [entry] [rendered]


ERROR in ./src/app/user/index.ts
Module parse failed: /Users/dominic/Sites/test/universal/node_modules/@ngtools/webpack/src/index.js!/Users/dominic/Sites/test/universal/src/app/user/index.ts Duplicate export 'LAZY_MODULE_MAP' (5:11)
You may need an appropriate loader to handle this file type.
| export var LAZY_MODULE_MAP = { "./lazy/lazy.module#LazyModule": __lazy_0__.LazyModule };
| export { UserService } from './user.service';
| export var LAZY_MODULE_MAP = { "./lazy/lazy.module#LazyModule": __lazy_0__.LazyModule };
| //# sourceMappingURL=index.js.map
@ ./src/app/app.module.ts 12:0-36
@ ./src/main.server.ts
@ multi ./src/main.server.ts

FrozenPandaz deleted the serverbuild branch

July 23, 2017 18:13

Contributor Author

FrozenPandaz commented Jul 23, 2017

@intellix Thank you for testing this feature and for the replication for the issue.

I have prepared a fix for this issue but would you do the honors of logging it as an issue in this GitHub repo?

Thank you

intellix mentioned this pull request

Duplicate LAZY_ROUTE_MAP exports with platform server in 1.3.0-rc.0 #7116

Closed

somombo mentioned this pull request

Integrating angular2 Universal with angular CLI project #1050

Closed

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

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

angular-automatic-lock-bot bot locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet