-
Notifications
You must be signed in to change notification settings - Fork 2.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
add bazel schematics #1
Conversation
3086556
to
2371cf1
Compare
4b2916c
to
727e854
Compare
…by angular schematics
727e854
to
1f37752
Compare
src/bazel/webpack.bzl
Outdated
"srcs": attr.label_list(allow_files=True, aspects=[_collect_es5_sources]), | ||
"config": attr.label(allow_single_file=True), | ||
"mode": attr.string(default="dev"), | ||
"_webpack": attr.label(default=Label("@build_bazel_rules_nrwl//:webpack"), executable=True, cfg="host") |
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.
the workspace name is supposed to be a reverse-DNS form
https://docs.bazel.build/versions/master/be/functions.html#workspace
on the other hand, for angular I'm planning to use the NPM namespace (even though this could conflict in theory) so my workspace name will be just @angular
- perhaps you'd like this to be @nrwl
@@ -0,0 +1,107 @@ | |||
module.exports = function(config) { |
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.
Add a warning comment about how the testing rule will change? (ppl might come browse this repo to find out what we are planning)
src/bazel/karma.conf.js
Outdated
fixWebpackSourcePaths: true | ||
}, | ||
|
||
client:{ |
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.
you didn't run a formatter on this file?1?1111
src/bazel/karma.conf.js
Outdated
browsers: ['Chrome'], | ||
|
||
// globals: { | ||
// appPath: `${config.bin}/${config.app}` |
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.
remove comments?
@@ -0,0 +1,30 @@ | |||
var templateUrlRegex = /templateUrl\s*:(\s*['"`](.*?)['"`]\s*([,}]))/gm; |
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.
this file only used with karma, right? seems like there should be a karma subdirectory
src/schematics/app/index.ts
Outdated
function addBootstrap(path: string): Rule { | ||
return (host: Tree) => { | ||
const modulePath = `${path}/app/app.module.ts`; | ||
const moduleSource = host.read(modulePath) !.toString('utf-8'); |
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.
strange space there before non-null assertion - are you using a formatter?
load("@build_bazel_rules_angular//:defs.bzl", "ng_external_libraries") | ||
|
||
filegroup(name = "node_modules", srcs = glob([ | ||
# should not be whitelisted |
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.
can you change this block to
glob(["node_modules/**/*.js", "node_modules/**/*.json", "node_modules/**/*.d.ts"])
without breaking anything?
"node_modules/@angular/platform-browser/animations*", | ||
"node_modules/@angular/platform-browser/animations/**", | ||
|
||
# Alex E? |
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.
this should go away soon when we do bootstrap codegen in the scripts.postinstall
@@ -0,0 +1,28 @@ | |||
# <%= className %> | |||
|
|||
This project was generated with [Angular CLI](https://github.com/angular/angular-cli) version <%= version %>. |
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.
it needs to say EXPERIMENTAL BAZEL I think
@@ -0,0 +1,5 @@ | |||
build --strategy=AngularTemplateCompile=standalone |
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.
can you keep the comments in this file so users have a clue what to change
1f37752
to
66d1441
Compare
- removes the `differentialLoading` build option - differential loading is always enabled for prod builds BEFORE (without ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 13.834 s ± 1.731 s [User: 11.817 s, System: 1.352 s] Range (min … max): 11.947 s … 16.015 s 10 runs AFTER (with ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 18.711 s ± 1.310 s [User: 12.172 s, System: 1.394 s] Range (min … max): 17.232 s … 20.770 s 10 runs
- removes the `differentialLoading` build option - differential loading is always enabled for prod builds BEFORE (without ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 13.834 s ± 1.731 s [User: 11.817 s, System: 1.352 s] Range (min … max): 11.947 s … 16.015 s 10 runs AFTER (with ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 18.711 s ± 1.310 s [User: 12.172 s, System: 1.394 s] Range (min … max): 17.232 s … 20.770 s 10 runs
- removes the `differentialLoading` build option - differential loading is always enabled for prod builds BEFORE (without ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 13.834 s ± 1.731 s [User: 11.817 s, System: 1.352 s] Range (min … max): 11.947 s … 16.015 s 10 runs AFTER (with ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 18.711 s ± 1.310 s [User: 12.172 s, System: 1.394 s] Range (min … max): 17.232 s … 20.770 s 10 runs
- removes the `differentialLoading` build option - differential loading is always enabled for prod builds BEFORE (without ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 13.834 s ± 1.731 s [User: 11.817 s, System: 1.352 s] Range (min … max): 11.947 s … 16.015 s 10 runs AFTER (with ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 18.711 s ± 1.310 s [User: 12.172 s, System: 1.394 s] Range (min … max): 17.232 s … 20.770 s 10 runs
- removes the `differentialLoading` build option - differential loading is always enabled for prod builds BEFORE (without ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 13.834 s ± 1.731 s [User: 11.817 s, System: 1.352 s] Range (min … max): 11.947 s … 16.015 s 10 runs AFTER (with ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 18.711 s ± 1.310 s [User: 12.172 s, System: 1.394 s] Range (min … max): 17.232 s … 20.770 s 10 runs
- removes the `differentialLoading` build option - differential loading is always enabled for prod builds BEFORE (without ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 13.834 s ± 1.731 s [User: 11.817 s, System: 1.352 s] Range (min … max): 11.947 s … 16.015 s 10 runs AFTER (with ESM): Benchmark #1: nx build demo --prod Time (mean ± σ): 18.711 s ± 1.310 s [User: 12.172 s, System: 1.394 s] Range (min … max): 17.232 s … 20.770 s 10 runs
Nx Cloud ReportWe didn't find any information for the current pull request with the commit cbf0411. Check the Getting started section to configure the app. Sent with 💌 from NxCloud. |
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
No description provided.