build(cdk): add bazel ng_module rules #8370
Conversation
jelbourn
added
area: dev-infra
jelbourn
requested review from
andrewseguin,
crisbeto,
devversion,
josephperrott and
mmalerba
as code owners
googlebot
added
the
cla: yes
jelbourn
force-pushed
the
bazel
branch
3 times, most recently
from
fef0808 to
f705b63
Compare
| @@ -0,0 +1,35 @@ | |||
| package(default_visibility = ["//visibility:public"]) | |||
There was a problem hiding this comment.
maybe we should use //:__subpackages__ as the default visibility everywhere, to avoid external repos depending on our rules
There was a problem hiding this comment.
If I try to use that here I get:
ERROR: /usr/local/google/home/jelbourn/.cache/bazel/_bazel_jelbourn/2010aad45cf3f55bb7127948064a6254/external/build_bazel_rules_typescript/internal/tsc_wrapped/BUILD.bazel:19:1: Target '//:node_modules' is not visible from target '@build_bazel_rules_typescript//internal/tsc_wrapped:tsc_bin'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: /usr/local/google/home/jelbourn/.cache/bazel/_bazel_jelbourn/2010aad45cf3f55bb7127948064a6254/external/build_bazel_rules_typescript/internal/tsc_wrapped/BUILD.bazel:19:1: Target '//:node_modules' is not visible from target '@build_bazel_rules_typescript//internal/tsc_wrapped:tsc_bin'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: Analysis of target '//src/cdk/observers:observers' failed; build aborted: Analysis of target '@build_bazel_rules_typescript//internal/tsc_wrapped:tsc_bin' failed; build aborted
INFO: Elapsed time: 0.833s
FAILED: Build did NOT complete successfully (1 packages loaded)
For the rest of the rules I'm not too concerned about it right now
| # Reduces the number of files as inputs to nodejs_binary: | ||
| # bazel query "deps(:node_modules)" | wc -l | ||
| # This won't scale in the general case. | ||
| # TODO(alexeagle): figure out what to do |
There was a problem hiding this comment.
sorry, I have an idea how to fix this if anyone has some time to investigate...
There was a problem hiding this comment.
Doesn't strike me as that bad- you want to be explicit about your dependencies anyway
WORKSPACE
Outdated
| @@ -0,0 +1,38 @@ | |||
| workspace(name = "angular_material_src") | |||
| load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") | |||
There was a problem hiding this comment.
I don't think we should do this anymore, you can use the built-in git_repository.
This one is the jgit version which supports sso:// git schemes, not needed here
There was a problem hiding this comment.
Done, didn't know that was a thing.
angular.tsconfig.json
Outdated
| "exclude": [ | ||
| "node_modules/@angular/bazel/**", | ||
| "node_modules/@angular/compiler-cli/**", | ||
| // Workaround bug introduced by 079d884 |
There was a problem hiding this comment.
don't need this workaround now, see alexeagle/angular-bazel-example at head
| "@angular/compiler-cli": "^5.0.0", | ||
| "@angular/http": "^5.0.0", | ||
| "@angular/platform-browser-dynamic": "^5.0.0", | ||
| "@angular/platform-server": "^5.0.0", | ||
| "@angular/router": "^5.0.0", | ||
| "@angular/upgrade": "^5.0.1", |
There was a problem hiding this comment.
any particular reason you added upgrade?
There was a problem hiding this comment.
Without it the router fails the postinstall ngc:
> ngc -p angular.tsconfig.json
node_modules/@angular/router/upgrade/src/upgrade.d.ts(9,31): error TS2307: Cannot find module '@angular/upgrade/static'.
| "//src/cdk/keycodes", | ||
| "//src/cdk/platform", | ||
| ], | ||
| tsconfig = ":tsconfig-build.json", |
There was a problem hiding this comment.
note, you might want to try using the same tsconfig for all the rules, and have it be the one the editor uses too
There was a problem hiding this comment.
Impossible presently because we need to specify a different flatModuleId for each package, but would be nice long term.
| # Add TypeScript rules | ||
| local_repository( | ||
| name = "build_bazel_rules_typescript", | ||
| path = "node_modules/@bazel/typescript", |
There was a problem hiding this comment.
We should explicitly specify this as a dependency: https://www.npmjs.com/package/@bazel/typescript
jelbourn
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Part of #8369
This is setup for eventually switching everything to bazel. It does not affect our real build yet.
cc @devversion