Skip to content

Commit

Permalink
feat(bazel): speed up dev-turnaround by bundling types only when pack…
Browse files Browse the repository at this point in the history
…aging (#45405)

Speeds up the dev-turnaround by only bundling types when packaging. Currently
bundling occurs for all the `ng_module` targets in devmode.

This has various positive benefits:

* Avoidance of this rather slower operation in development
* Makes APF-built packages also handle types for `ts_library` targets consistently.
* Allows us to ensure APF entry-points have `d.ts` _always_ bundled (working with ESM
module resolution in TypeScript -- currently experimental)
* Allows us to remove the secondary `package.json` files from APF (maybe APF v14? - seems
low-impact). This would clean-up the APF even more and fix resolution issues (like in Vite)

PR Close #45405
  • Loading branch information
devversion authored and atscott committed Apr 21, 2022
1 parent 3dee3d1 commit 68597bb
Show file tree
Hide file tree
Showing 54 changed files with 392 additions and 605 deletions.
2 changes: 0 additions & 2 deletions packages/animations/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ ng_package(
name = "npm_package",
srcs = [
"package.json",
"//packages/animations/browser:package.json",
"//packages/animations/browser/testing:package.json",
],
tags = [
"release-with-framework",
Expand Down
4 changes: 0 additions & 4 deletions packages/animations/browser/package.json

This file was deleted.

3 changes: 0 additions & 3 deletions packages/animations/browser/testing/package.json

This file was deleted.

7 changes: 5 additions & 2 deletions packages/bazel/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pkg_npm(
) + [
"//packages/bazel/src:package_assets",
"//packages/bazel/src/ng_module:package_assets",
"//packages/bazel/src/api-extractor:package_assets",
"//packages/bazel/src/types_bundle:package_assets",
"//packages/bazel/src/ng_package:package_assets",
"//packages/bazel/src/ngc-wrapped:package_assets",
"//packages/bazel/third_party/github.com/bazelbuild/bazel/src/main/protobuf:package_assets",
Expand All @@ -20,6 +20,9 @@ pkg_npm(
# users having to define the build setting, so we directly point the flag label to the place where
# we expect `@angular/bazel` being located. Note: This expects the `@npm//` workspace to be used.
"//packages/bazel/src:partial_compilation": "@npm//@angular/bazel/src:partial_compilation",
# Substitutions to account for label changes when `@angular/bazel` is consumed externally.
# NodeJS binaries are pre-built and should be consumed through the NPM bin scripts of the package.
"//packages/bazel/src/types_bundle:types_bundler": "@npm//@angular/bazel/bin:types_bundler",
"//packages/bazel/": "//@angular/bazel/",
"@npm//@bazel/concatjs/internal:": "//@bazel/concatjs/internal:",
},
Expand All @@ -28,9 +31,9 @@ pkg_npm(
# Dependencies on the full npm_package cause long re-builds.
visibility = ["//integration:__subpackages__"],
deps = [
"//packages/bazel/src/api-extractor:lib",
"//packages/bazel/src/ng_package:lib",
"//packages/bazel/src/ngc-wrapped:ngc_lib",
"//packages/bazel/src/types_bundle:lib",
],
)
# END-DEV-ONLY
2 changes: 2 additions & 0 deletions packages/bazel/index.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ Users should not load files under "/src"

load("//packages/bazel/src/ng_package:ng_package.bzl", _ng_package = "ng_package_macro")
load("//packages/bazel/src/ng_module:ng_module.bzl", _ng_module = "ng_module_macro")
load("//packages/bazel/src/types_bundle:index.bzl", _types_bundle = "types_bundle")

ng_module = _ng_module
ng_package = _ng_package
types_bundle = _types_bundle

# DO NOT ADD PUBLIC API without including in the documentation generation
# Run `yarn bazel build //packages/bazel/docs` to verify
2 changes: 1 addition & 1 deletion packages/bazel/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
"node": "^14.15.0 || >=16.10.0"
},
"bin": {
"api-extractor": "./src/api-extractor/index.js",
"ngc-wrapped": "./src/ngc-wrapped/index.js",
"packager": "./src/ng_package/packager.js",
"types_bundler": "./src/types_bundle/index.js",
"xi18n": "./src/ngc-wrapped/extract_i18n.js"
},
"typings": "./src/ngc-wrapped/index.d.ts",
Expand Down
102 changes: 0 additions & 102 deletions packages/bazel/src/api-extractor/index.ts

This file was deleted.

70 changes: 5 additions & 65 deletions packages/bazel/src/ng_module/ng_module.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ load(
"//packages/bazel/src:external.bzl",
"COMMON_ATTRIBUTES",
"COMMON_OUTPUTS",
"DEFAULT_API_EXTRACTOR",
"DEFAULT_NG_COMPILER",
"DEFAULT_NG_XI18N",
"DEPS_ASPECTS",
Expand All @@ -30,8 +29,6 @@ load(
# compilation which includes this provider.
NgPerfInfo = provider(fields = ["enable_perf_logging"])

_FLAT_DTS_FILE_SUFFIX = ".bundle.d.ts"

def is_perf_requested(ctx):
return ctx.attr.perf_flag != None and ctx.attr.perf_flag[NgPerfInfo].enable_perf_logging == True

Expand Down Expand Up @@ -72,20 +69,6 @@ def _flat_module_out_file(ctx):
return ctx.attr.flat_module_out_file
return "%s_public_index" % ctx.label.name

def _should_produce_dts_bundle(ctx):
"""Should we produce dts bundles.
We only produce flatten dts outs when we expect the ng_module is meant to be published,
based on the value of the bundle_dts attribute.
Args:
ctx: skylark rule execution context
Returns:
true when we should produce bundled dts.
"""
return getattr(ctx.attr, "bundle_dts", False)

def _should_produce_flat_module_outs(ctx):
"""Should we produce flat module outputs.
Expand Down Expand Up @@ -154,14 +137,6 @@ def _expected_outs(ctx):

declaration_files += [ctx.actions.declare_file(basename + ext) for ext in declarations]

dts_bundle = None
if _should_produce_dts_bundle(ctx):
# We need to add a suffix to bundle as it might collide with the flat module dts.
# The flat module dts out contains several other exports
# https://github.com/angular/angular/blob/84406e4d6d93b28b23efbb1701bc5ae1084da67b/packages/compiler-cli/src/metadata/index_writer.ts#L18
# the file name will be like 'core.bundle.d.ts'
dts_bundle = ctx.actions.declare_file(ctx.label.name + _FLAT_DTS_FILE_SUFFIX)

# We do this just when producing a flat module index for a publishable ng_module
if _should_produce_flat_module_outs(ctx):
flat_module_out_name = _flat_module_out_file(ctx)
Expand Down Expand Up @@ -193,7 +168,6 @@ def _expected_outs(ctx):
devmode_js = devmode_js_files,
declarations = declaration_files,
transpilation_infos = transpilation_infos,
dts_bundle = dts_bundle,
bundle_index_typings = bundle_index_typings,
dev_perf_files = dev_perf_files,
prod_perf_files = prod_perf_files,
Expand Down Expand Up @@ -300,7 +274,6 @@ def ngc_compile_action(
node_opts,
locale = None,
i18n_args = [],
dts_bundle_out = None,
target_flavor = "prodmode"):
"""Helper function to create the ngc action.
Expand All @@ -316,7 +289,6 @@ def ngc_compile_action(
node_opts: list of strings, extra nodejs options.
locale: i18n locale, or None
i18n_args: additional command-line arguments to ngc
dts_bundle_out: produced flattened dts file
target_flavor: Whether prodmode or devmode output is being built.
Returns:
Expand Down Expand Up @@ -364,28 +336,6 @@ def ngc_compile_action(
},
)

if dts_bundle_out != None:
# combine the inputs and outputs and filter .d.ts and json files
filter_inputs = [f for f in inputs.to_list() + outputs if f.path.endswith(".d.ts") or f.path.endswith(".json")]

if _should_produce_flat_module_outs(ctx):
dts_entry_point = "%s.d.ts" % _flat_module_out_file(ctx)
else:
dts_entry_point = ctx.attr.entry_point.label.name.replace(".ts", ".d.ts")

ctx.actions.run(
progress_message = "Bundling DTS (%s) %s" % (target_flavor, str(ctx.label)),
mnemonic = "APIExtractor",
executable = ctx.executable.api_extractor,
inputs = filter_inputs,
outputs = [dts_bundle_out],
arguments = [
tsconfig_file.path,
"/".join([ctx.bin_dir.path, ctx.label.package, dts_entry_point]),
dts_bundle_out.path,
],
)

if not locale and not ctx.attr.no_i18n:
return struct(
label = label,
Expand All @@ -410,7 +360,6 @@ def _compile_action(
ctx,
inputs,
outputs,
dts_bundle_out,
tsconfig_file,
node_opts,
target_flavor):
Expand Down Expand Up @@ -444,16 +393,16 @@ def _compile_action(
# Collect the inputs and summary files from our deps
action_inputs = depset(file_inputs)

return ngc_compile_action(ctx, ctx.label, action_inputs, outputs, tsconfig_file, node_opts, None, [], dts_bundle_out, target_flavor)
return ngc_compile_action(ctx, ctx.label, action_inputs, outputs, tsconfig_file, node_opts, None, [], target_flavor)

def _prodmode_compile_action(ctx, inputs, outputs, tsconfig_file, node_opts):
outs = _expected_outs(ctx)
return _compile_action(ctx, inputs, outputs + outs.closure_js + outs.prod_perf_files, None, tsconfig_file, node_opts, "prodmode")
return _compile_action(ctx, inputs, outputs + outs.closure_js + outs.prod_perf_files, tsconfig_file, node_opts, "prodmode")

def _devmode_compile_action(ctx, inputs, outputs, tsconfig_file, node_opts):
outs = _expected_outs(ctx)
compile_action_outputs = outputs + outs.devmode_js + outs.declarations + outs.dev_perf_files
_compile_action(ctx, inputs, compile_action_outputs, outs.dts_bundle, tsconfig_file, node_opts, "devmode")
_compile_action(ctx, inputs, compile_action_outputs, tsconfig_file, node_opts, "devmode")

# Note: We need to define `label` and `srcs_files` as `tsc_wrapped` passes
# them and Starlark would otherwise error at runtime.
Expand Down Expand Up @@ -496,9 +445,6 @@ def ng_module_impl(ctx, ts_compile_actions):
flat_module_out_prodmode_file = outs.flat_module_out_prodmode_file,
)

if outs.dts_bundle != None:
providers["dts_bundle"] = outs.dts_bundle

return providers

def _ng_module_impl(ctx):
Expand Down Expand Up @@ -572,12 +518,12 @@ NG_MODULE_ATTRIBUTES = {
""",
default = Label(DEFAULT_NG_COMPILER),
executable = True,
cfg = "host",
cfg = "exec",
),
"ng_xi18n": attr.label(
default = Label(DEFAULT_NG_XI18N),
executable = True,
cfg = "host",
cfg = "exec",
),
"_partial_compilation_flag": attr.label(
default = "//packages/bazel/src:partial_compilation",
Expand Down Expand Up @@ -691,12 +637,6 @@ NG_MODULE_RULE_ATTRS = dict(dict(COMMON_ATTRIBUTES, **NG_MODULE_ATTRIBUTES), **{
# See the flatModuleOutFile documentation in
# https://github.com/angular/angular/blob/master/packages/compiler-cli/src/transformers/api.ts
"flat_module_out_file": attr.string(),
"bundle_dts": attr.bool(default = False),
"api_extractor": attr.label(
default = Label(DEFAULT_API_EXTRACTOR),
executable = True,
cfg = "host",
),
# Should the rule generate ngfactory and ngsummary shim files?
"generate_ve_shims": attr.bool(default = False),
})
Expand Down

0 comments on commit 68597bb

Please sign in to comment.