build(bazel): add rule for cdk npm package #10509
Conversation
jelbourn
requested review from
andrewseguin,
crisbeto,
devversion,
josephperrott and
tinayuangao
as code owners
googlebot
added
the
cla: yes
src/cdk/rollup_globals.bzl
Outdated
| # Base rollup globals for dependencies and the root entry-point. | ||
| CDK_ROLLUP_GLOBALS = { | ||
| 'tslib': 'tslib', | ||
| 'moment': 'moment', |
There was a problem hiding this comment.
Is moment needed for cdk? I think it should only be required for material-moment-adapter
| "@angular/bazel": "6.0.0-beta.8", | ||
| "@angular/compiler-cli": "6.0.0-beta.8", | ||
| "@angular/bazel": "6.0.0-rc.0", | ||
| "@angular/compiler-cli": "6.0.0-rc.0", | ||
| "@angular/http": "6.0.0-beta.8", |
There was a problem hiding this comment.
maybe you want to update all the @angular/* in lock-step?
There was a problem hiding this comment.
Can't update the rest due to rxjs 6.0, which we don't have working yet
| @@ -1,4 +1,22 @@ | |||
| # List of all @angular/material components / subpackages. | |||
| CDK_PACKAGES = [ | |||
There was a problem hiding this comment.
do you want to try a bit harder to glob these, so adding a new subdirectory will automatically do the right thing?
There was a problem hiding this comment.
Suggestions? glob can only be used where a list of files is expected, and I don't want to repeat the glob in multiple places. AFAICT, there aren't any skylark APIs that deal with the file system.
src/cdk/keycodes/BUILD.bazel
Outdated
| # Note: this will need to ng_module if any Angular-specific stuff is added to coercion. | ||
| ts_library( | ||
|
|
||
| # TODO(jelbourn): change this back to a ts_library once ng_package supports |
There was a problem hiding this comment.
remove the TODO - you need ng_module because you want to produce flat metadata and bundle index which are done by ngc
| @@ -67,7 +67,7 @@ export class StepperSelectionEvent { | |||
| moduleId: module.id, | |||
| selector: 'cdk-step', | |||
| exportAs: 'cdkStep', | |||
| templateUrl: 'step.html', | |||
| template: '<ng-template><ng-content></ng-content></ng-template>', | |||
There was a problem hiding this comment.
this wasn't necessary, right? the resource inlining has landed
There was a problem hiding this comment.
Wasn't necessary, just cleaned it up incidentally since the template was trivial.
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. |
Also:
ng_moduleas a workaround