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

angular-cli + universal compliance #56

Closed
bliitzkrieg opened this issue Aug 9, 2017 · 11 comments · Fixed by #64
Closed

angular-cli + universal compliance #56

bliitzkrieg opened this issue Aug 9, 2017 · 11 comments · Fixed by #64

Comments

@bliitzkrieg
Copy link

I'm submitting a ... (check one with "x")

[ X ] bug report => check the README and search github for a similar issue or PR before submitting
[ ] support request => check the README and search github for a similar issue or PR before submitting
[ ] feature request

Current behavior

Angular Universal with the angular CLI (https://github.com/angular/angular-cli/wiki/stories-universal-rendering) is throwing an unexpected token import error

C:\Users\Luca\dev\myproject\node_modules\@ngx-meta\core\src\meta.loader.js:1
(function (exports, require, module, __filename, __dirname) { import { PageTitlePositioning } from './models/page-title-positioning';

SyntaxError: Unexpected token import
    at createScript (vm.js:53:10)
    at Object.runInThisContext (vm.js:95:10)
    at Module._compile (module.js:543:28)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)
    at Function.Module._load (module.js:439:3)
    at Module.require (module.js:498:17)
    at require (internal/module.js:20:19)
    at Object.Sk2Z (C:\Users\Luca\dev\myproject\dist-server\main.bundle.js:1:7520)

Expected/desired behavior
Should compile

Minimal reproduction of the problem with instructions

What is the motivation / use case for changing the behavior?
N/A

Please tell us about your environment:
Angular CLI using Express - The setup is a brand new Angular CLI project and the changes in universal guide linked above.

  • Angular version: 2.0.X
    4.3.3

  • Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]
    All

  • Language: [all | TypeScript X.X | ES6/7 | ES5]
    ES6

  • Node (for AoT issues): node --version =
    v7.10.0

@bliitzkrieg bliitzkrieg changed the title Angular CLI Universal " Unexpected token import" error [Support] Angular CLI Universal " Unexpected token import" error Aug 9, 2017
@bliitzkrieg bliitzkrieg changed the title [Support] Angular CLI Universal " Unexpected token import" error [support] Angular CLI Universal " Unexpected token import" error Aug 9, 2017
@bliitzkrieg bliitzkrieg changed the title [support] Angular CLI Universal " Unexpected token import" error Angular CLI Universal " Unexpected token import" error Aug 9, 2017
@bliitzkrieg
Copy link
Author

bliitzkrieg commented Aug 9, 2017

I believe ngrx-meta needs to be in a CommonJS format - See angular/angular-cli#7200 for more information

@fulls1z3
Copy link
Owner

@bliitzkrieg I've read the issue, and it seems like that's no way connected with the ngx-meta. Also, @hansl suggests a fix (not a solution) which uses https://www.npmjs.com/package/import-export. or disable AoT.

Furthermore, I believe using CLI with Angular Universal does not sound like a good practice - because CLI limits access to build configuration.

Using the CommonJS format has side effects on tree-shaking.

I can suggest you having a look at the boilerplate of ng-seed/universal.

@fulls1z3 fulls1z3 changed the title Angular CLI Universal " Unexpected token import" error [question] angular-cli universal Unexpected token import error Aug 10, 2017
@bliitzkrieg
Copy link
Author

bliitzkrieg commented Aug 10, 2017

Damn that's unfortunate.

Ideally the CLI should be the starting point for all angular projects and if custom configuration is required, that's when you eject from it and customize to your requirements. I assume the angular team has plans to integrate universal considering they have a universal guide on the CLI wiki (added 14 days ago) so there is going to be a lot of issues being raised for ngx-meta as that gains more traction. I've already seen 6+ issues all with the same problem being raised in popular angular 2 libraries.

Thanks for the reply though!

@fulls1z3
Copy link
Owner

@bliitzkrieg thanks for the information, but not only the popular angular libs are suffering, also key features such as lazy-loading, etc. are suffering too. Without those key features, CLI + Universal wouldn't be so practical.

Moreover, It's the official Angular documentation who recommends the es2015 module format for AoT and tree-shaking - not the library authors.

Also, CLI is unable to keep the pace with Angular itself in most cases. That's why we can't rely on CLI and
provide a custom boilerplate/build configuration for Universal. Hence, this issue is something that CLI team has to fix.

@bliitzkrieg
Copy link
Author

I can agree with that. I hope they get to a point where its streamlined with an opinionated approach (in the vain of NextJS) as SSR is such a great feature and a requirement for so many applications.

@hansl
Copy link

hansl commented Aug 10, 2017

The real fix is for ngx (and all libraries that want to be Universal compatible) to use the proper flags when building. @robwormald should contact you about it.

This is not something we can fix in the CLI easily, unfortunately.

@fulls1z3
Copy link
Owner

@hansl thanks for getting in touch and suggesting a solution, I really appreciate it 👍

Actually I'm aware the great job you and your team are doing with the CLI and can imagine how difficult is it to come up with a solution which works for everyone. Though job!

I'm looking forward to the information from @robwormald, while tracking further development and fixes with the CLI + Universal. Until then, I'll consider the official Angular documentation as a primary information source (ex: https://angular.io/guide/aot-compiler#aot-quickstart-source-code) and try to keep as close with it.

@fulls1z3 fulls1z3 reopened this Aug 10, 2017
@fulls1z3 fulls1z3 self-assigned this Aug 10, 2017
@fulls1z3 fulls1z3 changed the title [question] angular-cli universal Unexpected token import error [question] angular-cli + universal compliance Aug 10, 2017
@fulls1z3 fulls1z3 changed the title [question] angular-cli + universal compliance [enhancement] angular-cli + universal compliance Aug 10, 2017
@hansl
Copy link

hansl commented Aug 10, 2017

@fulls1z3 that guide is for applications, not libraries. Building a library is covered by another guide. https://www.youtube.com/watch?v=unICbsPGFIA is a good video tutorial. Rob will have more information.

@hansl
Copy link

hansl commented Aug 10, 2017

There's also this document which contains a lot of information (but isn't necessarily an easy read): https://docs.google.com/document/d/1CZC2rcpxffTDfRDs6p1cfbmKNLA6x5O-NtkJglDaBVs/preview

@fulls1z3
Copy link
Owner

@hansl thanks, I think this document will help 👍

fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build(npm): update npm scripts, deps

* build(webpack): remove webpack test config

* refactor: migrate test config to `jest`

* refactor: centralize tsconfig

* docs: add jest badge to README

* refactor(core): refactor `jest` migration

* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
@fulls1z3
Copy link
Owner

fulls1z3 commented Sep 3, 2017

@hansl thanks again for the documentation, watching the ng-conf video and reading the document that you shared as well as analyzing the repo that @filipesilva deployed angular-quickstart-lib gave me enough information about how to maintain the build system for angular libraries. If you'll be able to have a quick look at the repo, it would be awesome to share ideas.

@bliitzkrieg please try to get the latest version of ngx-meta from this repository, and try to copy the contents of dist folder to node_modules\@ngx-meta\core and see if these changes solve this problem. You would need to perform the following actions to build @ngx-meta:

git clone https://github.com/fulls1z3/ngx-meta
cd ngx-meta
yarn
npm run build

Actually I was able to get this repo running with the new builds without a problem, but it would be better to get your feedback also. According to the result, I'll close this issue and deploy the changes on the next release which's planned this week.

@fulls1z3 fulls1z3 reopened this Sep 3, 2017
@fulls1z3 fulls1z3 modified the milestones: v0.4.0, v0.2.0 Sep 3, 2017
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 3, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* build(npm): update npm scripts, deps

Fixes #56
fulls1z3 pushed a commit that referenced this issue Sep 4, 2017
* build: update ignorers

* build(webpack): remove webpack prod config

* build(gulp): remove gulp tasks

* build(rollup): add rollup tasks

* refactor(core): export enum

* build(npm): update npm scripts, deps

Fixes #56
@fulls1z3 fulls1z3 changed the title [enhancement] angular-cli + universal compliance angular-cli + universal compliance Sep 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants