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

feat(bazel): support ts_library targets as entry-points for ng_package #32610

Conversation

@devversion
Copy link
Member

commented Sep 11, 2019

Within an Angular package, it can happen that there are
entry-points which do not contain features that belong into
an @NgModule or need metadata files to be generated.

For example: the cdk, cdk/testing and cdk/coercion
entry-points. Besides other entry-points in the cdk
package, those entry-points do not need metadata to
be generated and no not use the ng_module rule.

Currently the "ng_package" rule properly picks up such
entry-points and builds bundles, does downleveling etc.
The only thing it misses is that no package.json files
are generated for the entry-point. This means that consumers
will not be able to use these entry-points built with "ts_library"
(except accessing the individual bundlings explicitly).

The "ng_package" rule should follow the full APF specification
for such entry-points. Partially building bundles and doing the
downleveling is confusing and a breaking issue.


The motifivation of supporting this (besides making the
rule behavior consistent; the incomplete output is not
acceptable), is that using the "ng_module" rule does
not make sense to be used for non-Angular entry-points.

Especially since it depends on Angular packages to
be specified as Bazel action inputs just to compile
vanilla TypeScript with @angular/compiler-cli.

@devversion devversion requested review from angular/dev-infra-framework as code owners Sep 11, 2019
@ngbot ngbot bot added this to the needsTriage milestone Sep 11, 2019
@googlebot googlebot added the cla: yes label Sep 11, 2019
devversion added a commit to devversion/material2 that referenced this pull request Sep 11, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
@devversion devversion force-pushed the devversion:feat/bazel-ng-package-allow-ts-library-targets branch from c79abd1 to f11fb6c Sep 11, 2019
devversion added a commit to devversion/material2 that referenced this pull request Sep 11, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
@devversion devversion force-pushed the devversion:feat/bazel-ng-package-allow-ts-library-targets branch 4 times, most recently from f7a70a8 to 77db4c2 Sep 11, 2019
@devversion devversion force-pushed the devversion:feat/bazel-ng-package-allow-ts-library-targets branch 2 times, most recently from bb9e25d to 2928cdc Sep 11, 2019
@josephperrott josephperrott requested a review from alexeagle Sep 11, 2019
devversion added a commit to devversion/material2 that referenced this pull request Sep 12, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
Copy link
Contributor

left a comment

nice job as always

Copy link
Member

left a comment

LGTM

devversion added 2 commits Sep 11, 2019
Within an Angular package, it can happen that there are
entry-points which do not contain features that belong into
an `@NgModule` or need metadata files to be generated.

For example: the `cdk`, `cdk/testing` and `cdk/coercion`
entry-points. Besides other entry-points in the `cdk`
package, those entry-points do not need metadata to
be generated and no not use the `ng_module` rule.

Currently the "ng_package" rule properly picks up such
entry-points and builds bundles, does downleveling etc.
The only thing it misses is that no `package.json` files
are generated for the entry-point. This means that consumers
will not be able to use these entry-points built with "ts_library"
(except accessing the individual bundlings explicitly).

The "ng_package" rule should follow the full APF specification
for such entry-points. Partially building bundles and doing the
downleveling is confusing and a breaking issue.

The motifivation of supporting this (besides making the
rule behavior consistent; the incomplete output is not
acceptable), is that using the "ng_module" rule does
not make sense to be used for non-Angular entry-points.

Especially since it depends on Angular packages to
be specified as Bazel action inputs just to compile
vanilla TypeScript with `@angular/compiler-cli`.
…_package

Add debug messages
@devversion devversion force-pushed the devversion:feat/bazel-ng-package-allow-ts-library-targets branch from 2928cdc to dd5a220 Sep 13, 2019
devversion added a commit to devversion/material2 that referenced this pull request Sep 13, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
@kara kara closed this in 217db9b Sep 13, 2019
devversion added a commit to devversion/material2 that referenced this pull request Sep 17, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
devversion added a commit to devversion/material2 that referenced this pull request Sep 17, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
devversion added a commit to devversion/material2 that referenced this pull request Sep 17, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
devversion added a commit to devversion/material2 that referenced this pull request Sep 17, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
devversion added a commit to devversion/material2 that referenced this pull request Sep 17, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
devversion added a commit to devversion/material2 that referenced this pull request Sep 18, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
jelbourn added a commit to angular/components that referenced this pull request Sep 18, 2019
We can't use "ts_library" for entry-points which should be part
of a "ng_package". This is because "ng_package" currently only
partially includes "ts_library" targets in the package (i.e. missing
`package.json` file for secondary entry-points).

Until we figure out what best practice is, or if angular/angular#32610
is merged, we just use "ng_module" to work around this issue.
arnehoek added a commit to arnehoek/angular that referenced this pull request Sep 26, 2019
angular#32610)

Within an Angular package, it can happen that there are
entry-points which do not contain features that belong into
an `@NgModule` or need metadata files to be generated.

For example: the `cdk`, `cdk/testing` and `cdk/coercion`
entry-points. Besides other entry-points in the `cdk`
package, those entry-points do not need metadata to
be generated and no not use the `ng_module` rule.

Currently the "ng_package" rule properly picks up such
entry-points and builds bundles, does downleveling etc.
The only thing it misses is that no `package.json` files
are generated for the entry-point. This means that consumers
will not be able to use these entry-points built with "ts_library"
(except accessing the individual bundlings explicitly).

The "ng_package" rule should follow the full APF specification
for such entry-points. Partially building bundles and doing the
downleveling is confusing and a breaking issue.

The motifivation of supporting this (besides making the
rule behavior consistent; the incomplete output is not
acceptable), is that using the "ng_module" rule does
not make sense to be used for non-Angular entry-points.

Especially since it depends on Angular packages to
be specified as Bazel action inputs just to compile
vanilla TypeScript with `@angular/compiler-cli`.

PR Close angular#32610
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

commented Oct 14, 2019

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Oct 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.