Skip to content
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.

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

[BUG] nx-serverless addUniversal option #25

Open
2 tasks done
KingDarBoja opened this issue Jun 14, 2020 · 12 comments
Open
2 tasks done

[BUG] nx-serverless addUniversal option #25

KingDarBoja opened this issue Jun 14, 2020 · 12 comments

Comments

@KingDarBoja
Copy link

KingDarBoja commented Jun 14, 2020

Describe the bug
The current docs for the nx-serverless schematic are wrong at the addUniversal option. It does accept a boolean instead of yes as stated.

To Reproduce
Steps to reproduce the behavior:

  1. Create a new nx-workspace
  2. Run nx add @flowaccount/nx-serverless with the --addUniversal=yes option as stated on the docs.
  3. See error
Cannot parse arguments. See below for the reasons.
    Argument --addUniversal could not be parsed using value "yes".Valid type(s) is: boolean

Expected behavior
No error should show up. The documentation need small changes to reflect the correct value.

Screenshots

image

Check which provider is affected:

  • AWS
  • [] Azure
  • [] Google Cloud Platform

Check which framework is affected:

  • Angular
  • [] Nodejs
  • [] Serverless
  • [] Lambda
  • [] Infrastructure as a code

Additional context
Running on Windows 10 using git bash.

@KingDarBoja
Copy link
Author

KingDarBoja commented Jun 14, 2020

@wickstargazer I found another bug while testing the differences between starting a empty workspace and using an preset (Angular), should I open another issue? It is pretty much about the nx add command.

For this issue, I picked the Angular preset but if you pick the empty one, add the @nrwl/angular and create an Angular app, the command to run the nx-serverless schematic is different.

nx generate @flowaccount/nx-serverless:add-universal --project=universal-sls-nx --provider=aws --addUniversal=true

Also, the install fails even though I tested both using Node 14.4.0 with nvm.

nx generate @flowaccount/nx-serverless:add-universal --project=universal-sls-nx --provider=aws --addUniversal=true
    updating server.ts to support serverless-express and production mode.
- Installing packages...
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
warning @nguniversal/builders > browser-sync > [email protected]: Chokidar 2 will break on node v14+. Upgrade to chokidar 3 with 15x less dependencies.
warning @nguniversal/builders > guess-parser > @wessberg/ts-evaluator > jsdom > [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
error @nguniversal/[email protected]: The engine "node" is incompatible with this module. Expected version ">=10.13.0 <13.0.0". Got "14.4.0"
error Found incompatible module.
× Package install failed, see above.
The Schematic workflow failed. See above.

However, the schematic modified the files successfully.
The schematic modified the files but had to call npm install again to properly ran the app as some node modules weren't installed.

@wickstargazer
Copy link
Member

hmm you can keep it in the same issue, since the first one is very minor ... would you like to help investigate?
I have seen that before .. but not sure where its coming from. Maybe an e2e test would show the error.

@KingDarBoja
Copy link
Author

@wickstargazer No problem, I can take a look at it.

My purpose is to check if this plugin can be used as replacement of ng-toolkit as me being one of the current maintainers (the only one since the author is too busy) been looking at the new changes at Angular Universal v9.

Yesterday I tested this plugin over a sample Angular project (had to do some extra steps like installing @nrwl/workspace as devDependency) and tested running the universal commands build:ssr / serve:ssr using either ng or nx without success as the build step and serve works but the page isn't being rendered.

I will give a shot using a sample nx workspace and see how it goes.

@wickstargazer
Copy link
Member

The universal serve-ssr for local is not supposed to be used as this lib helps deploy it to serverless

I had some issues with hotload though but that can be fixed. To browse locally use the offline command, it helps runs the serverless up.

If you want to use the serve-ssr you will have to edit the server.ts and uncomment the call to app() function so it starts the express server

With serverless the express is wrapped by a proxy in handler.ts

We can somehow accomplish both if we want bu have to make some changes

Let me know if you need some help :)

@KingDarBoja
Copy link
Author

Thanks for the response, I will try as soon as possible. This plugin could indeed save some headache and extra schematics library as this is more up-to-date but lacking the other two main serverless configs that ng-toolkit has. Once again, I have been idle and haven't update the library so probably those two need some revamp as well as my main test environment was AWS.

@wickstargazer
Copy link
Member

Can you show me the config because this plugin is suppose to deploy it into lambda and i am using it in production

@KingDarBoja
Copy link
Author

KingDarBoja commented Jun 15, 2020

@wickstargazer I haven't tested the deployment step at the moment, I will try later as busy with some job stuff at the moment.

Regarding using build:ssr / serve:ssr on non-nx workspaces (aka a single Angular App), I have the following config on my package.json:

package.json
{
  "name": "external-nx-universal",
  "version": "0.0.0",
  "scripts": {
    "ng": "ng",
    "start": "ng serve",
    "build": "ng build",
    "test": "ng test",
    "lint": "ng lint",
    "e2e": "ng e2e",
    "dev:ssr": "ng run external-nx-universal:serve-ssr",
    "serve:ssr": "node dist/server/main.js",
    "build:ssr": "ng build --prod && ng run external-nx-universal:server:production",
    "prerender": "ng run external-nx-universal:prerender"
  },
  "private": true,
  "dependencies": {
    "@angular/animations": "~9.1.11",
    "@angular/common": "~9.1.11",
    "@angular/compiler": "~9.1.11",
    "@angular/core": "~9.1.11",
    "@angular/forms": "~9.1.11",
    "@angular/platform-browser": "~9.1.11",
    "@angular/platform-browser-dynamic": "~9.1.11",
    "@angular/platform-server": "~9.1.11",
    "@angular/router": "~9.1.11",
    "@nguniversal/express-engine": "^9.1.1",
    "aws-serverless-express": "^3.3.2",
    "express": "^4.17.1",
    "rxjs": "~6.5.4",
    "tslib": "^1.10.0",
    "zone.js": "~0.10.2"
  },
  "devDependencies": {
    "@angular-devkit/build-angular": "~0.901.8",
    "@angular/cli": "~9.1.8",
    "@angular/compiler-cli": "~9.1.11",
    "@flowaccount/nx-serverless": "*",
    "@nguniversal/builders": "^9.1.1",
    "@nrwl/cli": "^9.4.0",
    "@nrwl/cypress": "^9.4.0",
    "@nrwl/jest": "^9.4.0",
    "@nrwl/workspace": "^9.4.0",
    "@types/aws-serverless-express": "^3.3.2",
    "@types/express": "^4.17.0",
    "@types/jasmine": "~3.5.0",
    "@types/jasminewd2": "~2.0.3",
    "@types/jest": "25.1.4",
    "@types/node": "^12.11.1",
    "codelyzer": "^5.1.2",
    "jasmine-core": "~3.5.0",
    "jasmine-spec-reporter": "~4.2.1",
    "jest": "25.2.3",
    "karma": "~5.0.0",
    "karma-chrome-launcher": "~3.1.0",
    "karma-coverage-istanbul-reporter": "~2.1.0",
    "karma-jasmine": "~3.0.1",
    "karma-jasmine-html-reporter": "^1.4.2",
    "protractor": "~7.0.0",
    "serverless": "1.58.0",
    "serverless-apigw-binary": "^0.4.4",
    "serverless-offline": "^5.12.0",
    "ts-jest": "25.2.1",
    "ts-node": "~8.3.0",
    "tslint": "~6.1.0",
    "typescript": "~3.8.3"
  }
}

Notice how adding universal schematics (which is called inside the add-universal schematic of this plugin) will add four extra scripts as stated by the official guide.

And not sure which part should I uncomment on server.ts as I don't see anything to uncomment. If running npm run build:ssr and then npm run serve:ssr, I get the following error at localhost:4000:

Node Express server listening on http://localhost:4000
Error: Failed to lookup view "index" in views directory "some\path\external-nx-universal\browser"

But running npm run dev:ssr according to docs does work, so probably I am not using the above commands properly.

server.ts
import 'zone.js/dist/zone-node';

import { ngExpressEngine } from '@nguniversal/express-engine';
import * as express from 'express';
import { join } from 'path';

import { AppServerModule } from './src/main.server';
import { APP_BASE_HREF } from '@angular/common';
import { existsSync } from 'fs';
import { environment } from './src/environments/environment';

// The Express app is exported so that it can be used by serverless Functions.
export function app() {
  const server = express();
  const distFolder = environment.production ? join(process.cwd(), './browser') : join(process.cwd(), 'dist//browser');
  const indexHtml = existsSync(join(distFolder, 'index.original.html')) ? 'index.original.html' : 'index';

  // Our Universal express-engine (found @ https://github.com/angular/universal/tree/master/modules/express-engine)
  server.engine('html', ngExpressEngine({
    bootstrap: AppServerModule,
  }));

  server.set('view engine', 'html');
  server.set('views', distFolder);

  // Example Express Rest API endpoints
  // server.get('/api/**', (req, res) => { });
  // Serve static files from /browser
  server.get('*.*', express.static(distFolder, {
    maxAge: '1y'
  }));

  // All regular routes use the Universal engine
  server.get('*', (req, res) => {
    res.render(indexHtml, { req, providers: [{ provide: APP_BASE_HREF, useValue: req.baseUrl }] });
  });

  return server;
}

function run() {
  const port = process.env.PORT || 4000;

  // Start up the Node server
  const server = app();
  server.listen(port, () => {
    console.log(`Node Express server listening on http://localhost:${port}`);
  });
}

// Webpack will replace 'require' with '__webpack_require__'
// '__non_webpack_require__' is a proxy to Node 'require'
// The below code is to ensure that the server is run only when not requiring the bundle.
declare const __non_webpack_require__: NodeRequire;
const mainModule = __non_webpack_require__.main;
const moduleFilename = mainModule && mainModule.filename || '';
if (moduleFilename === __filename || moduleFilename.includes('iisnode')) {
  run();
}

export * from './src/main.server';

UPDATE

After checking against the universal-demo (following these steps):

npx @angular/cli@latest new universal-demo --routing
cd universal-demo
ng add @nguniversal/express-engine

I noticed there are two differences between the universal-demo and the one being generated with this plugin. The first one is located at server.ts file:

universal-demo

const distFolder = join(process.cwd(), 'dist/universal-demo/browser');

plugin-universal-demo

const distFolder = environment.production ? join(process.cwd(), './browser') : join(process.cwd(), 'dist//browser');

By replacing the distFolder expression with the one from universal-demo make it works with npm run build:ssr && npm run serve:ssr commands.

Also notice how the distFolder generated from the plugin has two slash (//) meaning that there was an extra directory at the output. This can be noticed as well on the package.json:

universal-demo

{
    "dev:ssr": "ng run universal-demo:serve-ssr",
    "serve:ssr": "node dist/universal-demo/server/main.js",
    "build:ssr": "ng build --prod && ng run universal-demo:server:production",
    "prerender": "ng run universal-demo:prerender"
}

plugin-universal-demo

{
    "dev:ssr": "ng run external-nx-universal:serve-ssr",
    "serve:ssr": "node dist/server/main.js",
    "build:ssr": "ng build --prod && ng run external-nx-universal:server:production",
    "prerender": "ng run external-nx-universal:prerender"
}

NOTE: I think I removed the extra directory on the serve:ssr script as it wasn't correct according to the output paths for both build and serve builders at angular.json file after adding the plugin to external-nx-universal app.

@KingDarBoja
Copy link
Author

KingDarBoja commented Jun 15, 2020

Looks like the correct steps to run any schematic from @flowaccount/nx-serverless requires to do the following:

  • Create a workspace (by either using npx, npm, yarn).
    • If it is a empty workspace, add your Angular or React plugin as devDependency. In my case, I want to create an Angular app, so I ran npm install -D @nrwl/angular.
  • Create your Angular app. Run nx g @nrwl/angular:app universal-sls-nx on your workspace. As example, my app is called demo-sls-uni.
  • Add this plugin as devDependency. Run npm i -D @flowaccount/nx-serverless.
  • Now you can trigger any schematic from this plugin. As example, run nx generate @flowaccount/nx-serverless:add-universal --project=universal-sls-nx --provider=aws --addUniversal=true to add universal support + serverless config.
  • Profit.

@wickstargazer
Copy link
Member

yes that should be the correct step. Otherwise we have to add another option to run angular schematics and app bootstrap before adding universal...
What do you think. Also it was aimed for nx-workspace projects ... did not consider any workspace :/

@KingDarBoja
Copy link
Author

KingDarBoja commented Jun 18, 2020

yes that should be the correct step. Otherwise we have to add another option to run angular schematics and app bootstrap before adding universal...
What do you think. Also it was aimed for nx-workspace projects ... did not consider any workspace :/

I do think it should be documented for two cases:

  • Nx workspace.
  • Non-nx workspace (A common Angular App).

For the first case, it should be somewhere as a important note for the developer / end-user to be sure both @nrwl/workspace and @nrwl/angular are available as devDependencies before running the generate schematics.

For the second case, it should be addressed as well but taking into account any extra step involved. I will test again and see the correct steps for this one as soon as possible.

Cheers!

NOTE ABOUT NX-WORKSPACE WITH ANGULAR PRESET
The documentation also needs update as running nx add @flowaccount/nx-serverless on a nx-workspace with Angular preset yields:

nx add @flowaccount/nx-serverless
Installing packages for tooling via npm.
Installed packages for tooling via npm.
Schematic input does not validate against the Schema: {}
Errors:

  Data path "" should have required property 'project'.

@wickstargazer
Copy link
Member

Hi @KingDarBoja would you like to adjust the plugin to support add? I haven't tested it out further yet, since i moved over to scully. But i would like to fix the add command to be more intuitive.

@KingDarBoja
Copy link
Author

KingDarBoja commented Jul 9, 2020

@wickstargazer Yup, consider it done, I will try to fix it on the weekend if I get the time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants