-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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] Switch to Angular CLI #10624
Conversation
b38d226
to
dd166b2
Compare
@wmarques Can you fix conflicts? |
@DanielFran I think it will be for the v7, I want to test this well and to check if we don't lost performances |
@@ -60,6 +60,8 @@ | |||
"zone.js": "0.10.2" | |||
}, | |||
"devDependencies": { | |||
"@angular-builders/custom-webpack": "^8.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"^8.2.0" => "8.2.0"
@@ -60,6 +60,8 @@ | |||
"zone.js": "0.10.2" | |||
}, | |||
"devDependencies": { | |||
"@angular-builders/custom-webpack": "^8.2.0", | |||
"@angular-devkit/build-angular": "^0.803.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"^0.803.3" => "0.803.3"
@@ -18,4 +18,4 @@ | |||
-%> | |||
import 'core-js/proposals/reflect-metadata'; | |||
import 'zone.js/dist/zone'; | |||
require('../manifest.webapp'); | |||
//require('../manifest.webapp'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so you can remove this line ?
This PR needs a rebase to fix conflicts. |
Yep I'll work on it during the month |
dd166b2
to
91cfa25
Compare
@wmarques : what is the state of your PR ? The original ticket is opened for too long (8 months) |
@@ -18,5 +18,5 @@ | |||
-%> | |||
{ | |||
"extends": "./tsconfig.json", | |||
"files": ["<%= MAIN_SRC_DIR %>app/app.main.ts"] | |||
"files": ["<%= MAIN_SRC_DIR %>app/app.main.ts","<%= MAIN_SRC_DIR %>app/polyfills.ts"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"<%= MAIN_SRC_DIR %>app/app.main.ts","<%= MAIN_SRC_DIR %>app/polyfills.ts"
=>
"<%= MAIN_SRC_DIR %>app/app.main.ts", "<%= MAIN_SRC_DIR %>app/polyfills.ts"
😇
ff4494d
to
f1e6829
Compare
867d129
to
fc52ded
Compare
fc52ded
to
a096e40
Compare
a096e40
to
5a22159
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should webpack.(common|prod|dev).js
be removed?
Thanks alot @mshima for your review ! I'll do this tomorrow in the afternoon, hope we can merge this asap then |
Saw yesterday that this is almost ready. |
@wmarques do you want some help finishing this PR? |
No thanks I'll apply your comments tomorrow, I was quite busy today 🙁 |
@wmarques Can you fix the conflicts. |
@DanielFran Working on it ! |
I'll fix this tomorrow, sorry I won't have time tonight 😢 |
f261ff7
to
3410c88
Compare
@wmarques : if the CI is green and if you feel confident, go ahead and merge this. It will avoid some conflict and we can still do post review |
3410c88
to
c8eedb6
Compare
c8eedb6
to
805e5cc
Compare
Finally ! Thanks guys, that was quite a journey 😄 |
The final result LGTM, thanks for your hard work @wmarques ! |
Great work! I also think the doc and the generated README.md should be updated. |
@gmarziou you're right I'll work on it tonight |
Fixes #10539
As discussed, given the opportunities offered by the Angular CLI, we can now extend their config with ours so IMO we should move into Angular CLI.
This offers some advantages:
We could even go further with the Angular CLI schematics, for example we could do an Entity schematic to generate an entity page.
Still Todo:
ng test
Jest support