feat(typescript): add JSNamedModuleInfo provider to ts_library outputs #1215

gregmagolan · 2019-09-30T19:28:26Z

All rules that require devmode named sources are updated to use JSNamedModuleInfo. The rules affected are karma, protractor, ts_devserver, ts_proto_library (in labs) & npm_package.

Some related non-breaking changes:

node_module_library scripts attribute renamed to named_module_srcs. node_module_library is currently not part of the public API and in user code the npm targets are generated by generate_build_file.ts by yarn_install & npm_install.

BREAKING CHANGES:

The following breaking changes are from internal details so they should not affect most users. Some some downstream projects, however, such as Angular rely on these internal details and will need to be updated accordingly when updating to the next release.

sources_aspect from /internal/node/node.bzl and /internal/common/sources_aspect.bzl is removed; its functionality was duplicate to what JSNamedModuleInfo providers
NodeModuleSources is removed and its sources field is moved to NpmPackageInfo; sources in the removed scripts field are provided by the JSNamedModuleInfo provider which node_module_library now provides
collect_node_modules_aspect renamed to just node_modules_aspect

alexeagle · 2019-09-30T21:26:51Z

internal/common/node_module_info.bzl

-    doc = "Provides sources for npm dependencies installed with yarn_install and npm_install rules",
+# `node_module_library` targets via the `node_modules_aspect` below.
+NodeModuleInfo = provider(
+    doc = "Provides information about npm dependencies",


FYI to remove js_library I think I'd like to produce this provider from npm_package too. Would be nice to include package name (could add in later PR)

I had sources as direct and transitive_sources as direct + transitive but changed to just sources which are direct + transitive similar to what we decided for the JS providers. Does that sounds like the right approach for this?

+1 for adding package (or module_name) or later PR.

I'm not sure. In JS tooling we sometimes want a strict-deps policy that requires distinguishing direct and transitive sources. I would bet we'll need direct for something sometime

Hmmmm. I like the idea of being able to differentiate as well even in the new JS providers. It would have solved my karma runtime_deps issue I saw earlier today by being able to just access sources of JSNamedModuleInfo instead of having to take all of transitive_sources.

Went with sources and direct_sources which now matches what we have in the JS providers. DeclarationInfo still differs with declarations and transitive_declarations however.

internal/node/npm_package_bin.bzl

internal/common/devmode_js_sources.bzl

internal/node/npm_package_bin.bzl

internal/npm_install/test/golden/@angular/core/BUILD.bazel.golden

alexeagle · 2019-10-01T02:02:42Z

packages/protractor/src/protractor_web_test.bzl

-        elif hasattr(d, "files"):
-            files = depset(transitive = [files, d.files])
+    files_depsets = [depset(ctx.files.srcs)]
+    for dep in ctx.attr.deps:


does this want to be a helper in providers.bzl? seems like I've seen the same impl many times?

just two places (karma & protractor). those two rules are due for a cleaning so will note that down to look at.

buildsize · 2019-10-07T21:59:58Z

File name	Previous Size	New Size	Change
release.tar.gz	1.06 MB	1.06 MB	-1002 bytes (0%)

alexeagle

looks close, will give it one more look later

examples/angular/patches/@angular+bazel+9.0.0-next.8.patch

alexeagle · 2019-10-07T22:43:51Z

internal/common/npm_package_info.bzl

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""NpmPackageInfo providers and apsect to collect node_modules from deps.


seems like it shouldn't be in internal/ - maybe providers.bzl or next to it?

providers.bzl seems like the right place as I don't think we want to introduce too many load points. where do you think for node_modules_aspect?

kick this downstream to later PR

alexeagle · 2019-10-07T22:44:17Z

internal/common/npm_package_info.bzl

+# `node_module_library` rule as well as other targets that have direct or transitive deps on
+# `node_module_library` targets via the `node_modules_aspect` below.
+NpmPackageInfo = provider(
+    doc = "Provides information about npm dependencies",


will need a bunch more doc here as it should be a public API

internal/node/node.bzl

alexeagle · 2019-10-07T22:47:06Z

internal/npm_install/test/golden/@angular/core/BUILD.bazel.golden

@@ -677,7 +677,7 @@ node_module_library(
 node_module_library(
    name = "core__contents",
    srcs = [":core__files"],
-    scripts = [
+    named_module_srcs = [


kinda breaking for users, huh?
maybe we can paste a buildozer script into the release notes?

I don't think anyone is using node_module_library directly as it is still private API and their generated build files will be re-generated on update

All rules that require devmode named sources are updated to use JSNamedModuleInfo. The rules affected are karma, protractor, ts_devserver, ts_proto_library (in labs) & npm_package. Some related non-breaking changes: * node_module_library scripts attribute renamed to named_module_srcs. node_module_library is currently not part of the public API and in user code the npm targets are generated by generate_build_file.ts by yarn_install & npm_install. BREAKING CHANGES: The following breaking changes are from internal details so they should not affect most users. Some some downstream projects, however, such as Angular rely on these internal details and will need to be updated accordingly when updating to the next release. * sources_aspect from /internal/node/node.bzl and /internal/common/sources_aspect.bzl is removed; its functionality was duplicate to what JSNamedModuleInfo providers * NodeModuleSources is removed and its sources field is moved to NpmPackageInfo; sources in the removed scripts field are provided by the JSNamedModuleInfo provider which node_module_library now provides * collect_node_modules_aspect renamed to just node_modules_aspect fix: nodejs_binary doesn't need declaration files

gregmagolan requested a review from alexeagle September 30, 2019 19:28

gregmagolan requested a review from soldair as a code owner September 30, 2019 19:28

googlebot added the cla: yes label Sep 30, 2019

gregmagolan force-pushed the js_named_module_info branch 2 times, most recently from b12ed7a to 4fb81dd Compare September 30, 2019 19:47

gregmagolan mentioned this pull request Sep 30, 2019

feat(typescript): add JSModuleInfo provider to ts_library outputs #1040

Closed

gregmagolan force-pushed the js_named_module_info branch 2 times, most recently from 2202a02 to 21e4e48 Compare September 30, 2019 21:07

alexeagle reviewed Sep 30, 2019

View reviewed changes

gregmagolan force-pushed the js_named_module_info branch 4 times, most recently from 61f5516 to e6fed92 Compare October 1, 2019 00:08

alexeagle reviewed Oct 1, 2019

View reviewed changes

gregmagolan force-pushed the js_named_module_info branch 16 times, most recently from 7d8f36c to 8b43e6c Compare October 4, 2019 22:01

gregmagolan force-pushed the js_named_module_info branch 18 times, most recently from 17947b3 to 14df712 Compare October 7, 2019 21:56

alexeagle reviewed Oct 7, 2019

View reviewed changes

gregmagolan force-pushed the js_named_module_info branch from e7b6788 to 35fff2b Compare October 8, 2019 16:34

alexeagle approved these changes Oct 9, 2019

View reviewed changes

alexeagle merged commit bb1f9b4 into bazel-contrib:master Oct 9, 2019

This was referenced Oct 10, 2019

node_sources in aspect is misnamed #76

Closed

Shared javascript provider #57

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(typescript): add JSNamedModuleInfo provider to ts_library outputs #1215

feat(typescript): add JSNamedModuleInfo provider to ts_library outputs #1215

gregmagolan commented Sep 30, 2019 •

edited

Loading

alexeagle Sep 30, 2019

gregmagolan Oct 1, 2019

alexeagle Oct 1, 2019

gregmagolan Oct 1, 2019 •

edited

Loading

gregmagolan Oct 7, 2019

alexeagle Oct 1, 2019

gregmagolan Oct 7, 2019

buildsize bot commented Oct 7, 2019 •

edited

Loading

alexeagle left a comment

alexeagle Oct 7, 2019

gregmagolan Oct 8, 2019 •

edited

Loading

gregmagolan Oct 8, 2019

alexeagle Oct 7, 2019

alexeagle Oct 7, 2019

gregmagolan Oct 8, 2019

feat(typescript): add JSNamedModuleInfo provider to ts_library outputs #1215

feat(typescript): add JSNamedModuleInfo provider to ts_library outputs #1215

Conversation

gregmagolan commented Sep 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregmagolan Oct 1, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buildsize bot commented Oct 7, 2019 • edited Loading

alexeagle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregmagolan Oct 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregmagolan commented Sep 30, 2019 •

edited

Loading

gregmagolan Oct 1, 2019 •

edited

Loading

buildsize bot commented Oct 7, 2019 •

edited

Loading

gregmagolan Oct 8, 2019 •

edited

Loading