Skip to content

Commit

Permalink
feat(typescript): add JSNamedModuleInfo provider to ts_library outputs
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gregmagolan committed Oct 7, 2019
1 parent 77e2d4a commit 14df712
Show file tree
Hide file tree
Showing 29 changed files with 485 additions and 368 deletions.
9 changes: 4 additions & 5 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,10 @@ yarn_install(
name = "npm",
# The @npm//:node_modules_filegroup generated by manual_build_file_contents
# is used in the //packages/typescript/test/reference_types_directive:tsconfig_types
# test. For now we're still supporting node_modules as a filegroup tho this
# may change in the future. The default generated //:node_modules target is a
# node_module_library rule which provides NodeModuleInfo and NodeModuleSources
# but that rule is not yet in the public API and we have not yet dropped support
# for filegroup based node_modules target.
# test. For now we're still supporting node_modules as a filegroup tho this may
# change in the future. The default generated //:node_modules target is a node_module_library
# rule which provides NpmPackageInfo but that rule is not yet in the public API and we
# have not yet dropped support for filegroup based node_modules target.
manual_build_file_contents = """
filegroup(
name = "node_modules_filegroup",
Expand Down
129 changes: 113 additions & 16 deletions examples/angular/patches/@angular+bazel+9.0.0-next.8.patch
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
diff --git a/node_modules/@angular/bazel/src/esm5.bzl b/node_modules/@angular/bazel/src/esm5.bzl
index 667f601..7907c12 100755
--- a/node_modules/@angular/bazel/src/esm5.bzl
+++ b/node_modules/@angular/bazel/src/esm5.bzl
@@ -30,7 +30,7 @@ ESM5Info = provider(
diff --git a/node_modules/@angular/bazel/src/external.bzl b/node_modules/@angular/bazel/src/external.bzl
index 4959f22..43f1c96 100755
--- a/node_modules/@angular/bazel/src/external.bzl
+++ b/node_modules/@angular/bazel/src/external.bzl
@@ -14,17 +14,17 @@ load(
_ts_providers_dict_to_struct = "ts_providers_dict_to_struct",
)
load(
- "@build_bazel_rules_nodejs//internal/common:node_module_info.bzl",
- _NodeModuleSources = "NodeModuleSources",
- _collect_node_modules_aspect = "collect_node_modules_aspect",
+ "@build_bazel_rules_nodejs//internal/common:npm_package_info.bzl",
+ _NpmPackageInfo = "NpmPackageInfo",
+ _node_modules_aspect = "node_modules_aspect",
)
load(
"@npm_bazel_typescript//internal:ts_config.bzl",
_TsConfigInfo = "TsConfigInfo",
)

def _map_closure_path(file):
- result = file.short_path[:-len(".closure.js")]
+ result = file.short_path[:-len(".mjs")]
-NodeModuleSources = _NodeModuleSources
-collect_node_modules_aspect = _collect_node_modules_aspect
+NpmPackageInfo = _NpmPackageInfo
+node_modules_aspect = _node_modules_aspect

# short_path is meant to be used when accessing runfiles in a binary, where
# the CWD is inside the current repo. Therefore files in external repo have a
tsc_wrapped_tsconfig = _tsc_wrapped_tsconfig
COMMON_ATTRIBUTES = _COMMON_ATTRIBUTES
diff --git a/node_modules/@angular/bazel/src/modify_tsconfig.js b/node_modules/@angular/bazel/src/modify_tsconfig.js
index 9d3c491..3c99746 100755
--- a/node_modules/@angular/bazel/src/modify_tsconfig.js
Expand All @@ -25,14 +39,23 @@ index 9d3c491..3c99746 100755
fs.writeFileSync(output, JSON.stringify(data));
}
diff --git a/node_modules/@angular/bazel/src/ng_module.bzl b/node_modules/@angular/bazel/src/ng_module.bzl
index 9b88fbb..68217d0 100755
index 9b88fbb..54bf2c7 100755
--- a/node_modules/@angular/bazel/src/ng_module.bzl
+++ b/node_modules/@angular/bazel/src/ng_module.bzl
@@ -20,6 +20,7 @@ load(
@@ -13,13 +13,14 @@ load(
"DEFAULT_NG_COMPILER",
"DEFAULT_NG_XI18N",
"DEPS_ASPECTS",
- "NodeModuleSources",
+ "NpmPackageInfo",
"TsConfigInfo",
- "collect_node_modules_aspect",
+ "node_modules_aspect",
"compile_ts",
"ts_providers_dict_to_struct",
"tsc_wrapped_tsconfig",
)
+load("@build_bazel_rules_nodejs//:providers.bzl", "transitive_js_ecma_script_module_info")
+load("@build_bazel_rules_nodejs//:providers.bzl", "js_ecma_script_module_info", "js_named_module_info")

_FLAT_DTS_FILE_SUFFIX = ".bundle.d.ts"
_R3_SYMBOLS_DTS_FILE = "src/r3_symbols.d.ts"
Expand All @@ -54,22 +77,96 @@ index 9b88fbb..68217d0 100755
bundle_index_typings = ctx.actions.declare_file("%s.d.ts" % flat_module_out)
declaration_files.append(bundle_index_typings)
if is_legacy_ngc:
@@ -603,7 +604,17 @@ def ng_module_impl(ctx, ts_compile_actions):
@@ -517,11 +518,11 @@ def _compile_action(ctx, inputs, outputs, dts_bundles_out, messages_out, tsconfi
file_inputs += ctx.attr.tsconfig[TsConfigInfo].deps

# Also include files from npm fine grained deps as action_inputs.
- # These deps are identified by the NodeModuleSources provider.
+ # These deps are identified by the NpmPackageInfo provider.
for d in ctx.attr.deps:
- if NodeModuleSources in d:
+ if NpmPackageInfo in d:
# Note: we can't avoid calling .to_list() on sources
- file_inputs.extend(_filter_ts_inputs(d[NodeModuleSources].sources.to_list()))
+ file_inputs.extend(_filter_ts_inputs(d[NpmPackageInfo].sources.to_list()))

# Collect the inputs and summary files from our deps
action_inputs = depset(
@@ -603,9 +604,23 @@ def ng_module_impl(ctx, ts_compile_actions):
return providers

def _ng_module_impl(ctx):
- return ts_providers_dict_to_struct(ng_module_impl(ctx, compile_ts))
-
-local_deps_aspects = [collect_node_modules_aspect, _collect_summaries_aspect]
+ ts_providers = ng_module_impl(ctx, compile_ts)
+
+ # Add in new JS providers
+ ts_providers["providers"].extend([
+ transitive_js_ecma_script_module_info(
+ js_named_module_info(
+ sources = ts_providers["typescript"]["es5_sources"],
+ deps = ctx.attr.deps,
+ ),
+ js_ecma_script_module_info(
+ sources = ts_providers["typescript"]["es6_sources"],
+ deps = ctx.attr.deps,
+ ),
+ ])
+
+ return ts_providers_dict_to_struct(ts_providers)
+
+local_deps_aspects = [node_modules_aspect, _collect_summaries_aspect]

# Workaround skydoc bug which assumes DEPS_ASPECTS is a str type
[local_deps_aspects.append(a) for a in DEPS_ASPECTS]
diff --git a/node_modules/@angular/bazel/src/ng_package/ng_package.bzl b/node_modules/@angular/bazel/src/ng_package/ng_package.bzl
index 48d7619..ea635bb 100755
--- a/node_modules/@angular/bazel/src/ng_package/ng_package.bzl
+++ b/node_modules/@angular/bazel/src/ng_package/ng_package.bzl
@@ -13,9 +13,8 @@ It packages your library following the Angular Package Format, see the
specification of this format at https://goo.gl/jB3GVv
"""

-load("@build_bazel_rules_nodejs//internal/common:collect_es6_sources.bzl", "collect_es6_sources")
-load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleSources")
-load("@build_bazel_rules_nodejs//internal/common:sources_aspect.bzl", "sources_aspect")
+load("@build_bazel_rules_nodejs//:providers.bzl", "JSEcmaScriptModuleInfo", "JSNamedModuleInfo")
+load("@build_bazel_rules_nodejs//internal/common:npm_package_info.bzl", "NpmPackageInfo")
load(
"@build_bazel_rules_nodejs//internal/rollup:rollup_bundle.bzl",
"ROLLUP_ATTRS",
@@ -207,7 +206,12 @@ def _filter_js_inputs(all_inputs):
def _ng_package_impl(ctx):
npm_package_directory = ctx.actions.declare_directory("%s.ng_pkg" % ctx.label.name)

- esm_2015_files = _filter_out_generated_files(collect_es6_sources(ctx), "js")
+ esm_2015_files = []
+ for dep in ctx.attr.deps:
+ if JSEcmaScriptModuleInfo in dep:
+ esm_2015_files += dep[JSEcmaScriptModuleInfo].sources.to_list()
+
+ esm_2015_files = _filter_out_generated_files(esm_2015_files, "js")
esm5_sources = _filter_out_generated_files(flatten_esm5(ctx), "js")

# These accumulators match the directory names where the files live in the
@@ -347,9 +351,9 @@ def _ng_package_impl(ctx):
node_modules_files = _filter_js_inputs(ctx.files.node_modules)

# Also include files from npm fine grained deps as inputs.
- # These deps are identified by the NodeModuleSources provider.
+ # These deps are identified by the NpmPackageInfo provider.
for d in ctx.attr.deps:
- if NodeModuleSources in d:
+ if NpmPackageInfo in d:
node_modules_files += _filter_js_inputs(d.files)
esm5_rollup_inputs = depset(node_modules_files, transitive = [esm5_sources])

@@ -469,7 +473,7 @@ def _ng_package_impl(ctx):
files = depset([package_dir]),
)]

local_deps_aspects = [collect_node_modules_aspect, _collect_summaries_aspect]
-DEPS_ASPECTS = [esm5_outputs_aspect, sources_aspect]
+DEPS_ASPECTS = [esm5_outputs_aspect]

# Workaround skydoc bug which assumes ROLLUP_DEPS_ASPECTS is a str type
[DEPS_ASPECTS.append(a) for a in ROLLUP_DEPS_ASPECTS]
23 changes: 12 additions & 11 deletions internal/common/devmode_js_sources.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
Outputs a manifest file with the sources listed.
"""

load(":sources_aspect.bzl", "sources_aspect")
load("@build_bazel_rules_nodejs//:providers.bzl", "JSNamedModuleInfo")

# Avoid using non-normalized paths (workspace/../other_workspace/path)
def _to_manifest_path(ctx, file):
Expand All @@ -27,26 +27,27 @@ def _to_manifest_path(ctx, file):
return ctx.workspace_name + "/" + file.short_path

def _devmode_js_sources_impl(ctx):
files = depset()

for d in ctx.attr.deps:
if hasattr(d, "node_sources"):
files = depset(transitive = [files, d.node_sources])
elif hasattr(d, "files"):
files = depset(transitive = [files, d.files])
sources_depsets = []
for dep in ctx.attr.deps:
if JSNamedModuleInfo in dep:
sources_depsets.append(dep[JSNamedModuleInfo].sources)
if hasattr(dep, "files"):
sources_depsets.append(dep.files)
sources = depset(transitive = sources_depsets)

ctx.actions.write(ctx.outputs.manifest, "".join([
_to_manifest_path(ctx, f) + "\n"
for f in files.to_list()
for f in sources.to_list()
if f.path.endswith(".js") or f.path.endswith(".mjs")
]))
return [DefaultInfo(files = files)]

return [DefaultInfo(files = sources)]

devmode_js_sources = rule(
implementation = _devmode_js_sources_impl,
attrs = {
"deps": attr.label_list(
allow_files = True,
aspects = [sources_aspect],
),
},
outputs = {
Expand Down
74 changes: 0 additions & 74 deletions internal/common/node_module_info.bzl

This file was deleted.

52 changes: 52 additions & 0 deletions internal/common/npm_package_info.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright 2017 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""NpmPackageInfo providers and apsect to collect node_modules from deps.
"""

# NpmPackageInfo provider is provided by targets that are npm dependencies by the
# `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",
fields = {
"direct_sources": "Depset of direct source files in this npm package",
"sources": "Depset of direct & transitive source files in this npm package and in its dependencies",
"workspace": "The workspace name that this npm package is provided from",
},
)

def _node_modules_aspect_impl(target, ctx):
providers = []

# provide NpmPackageInfo if it is not already provided there are NpmPackageInfo deps
if not NpmPackageInfo in target:
sources_depsets = []
workspace = None
if hasattr(ctx.rule.attr, "deps"):
for dep in ctx.rule.attr.deps:
if NpmPackageInfo in dep:
if workspace and dep[NpmPackageInfo].workspace != workspace:
fail("All npm dependencies need to come from a single workspace. Found '%s' and '%s'." % (workspace, dep[NpmPackageInfo].workspace))
workspace = dep[NpmPackageInfo].workspace
sources_depsets.append(dep[NpmPackageInfo].sources)
if workspace:
providers.extend([NpmPackageInfo(direct_sources = depset(), sources = depset(transitive = sources_depsets), workspace = workspace)])

return providers

node_modules_aspect = aspect(
_node_modules_aspect_impl,
attr_aspects = ["deps"],
)
Loading

0 comments on commit 14df712

Please sign in to comment.