Skip to content

Commit

Permalink
refactor: add include_sources and include_transitive_declarations to …
Browse files Browse the repository at this point in the history
…js_filegroup rules and gather_files_from_js_providers + gather_runfiles helpers (#1585)
  • Loading branch information
gregmagolan authored Apr 3, 2024
1 parent b5c5c19 commit 0f3b38f
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 15 deletions.
8 changes: 5 additions & 3 deletions docs/js_filegroup.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 15 additions & 4 deletions js/private/js_filegroup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ This helper rule is used by the `js_run_binary` macro.
def _js_filegroup_impl(ctx):
return DefaultInfo(files = _gather_files_from_js_providers(
targets = ctx.attr.srcs,
include_sources = ctx.attr.include_sources,
include_transitive_sources = ctx.attr.include_transitive_sources,
include_declarations = ctx.attr.include_declarations,
include_transitive_declarations = ctx.attr.include_transitive_declarations,
include_npm_linked_packages = ctx.attr.include_npm_linked_packages,
))

# TODO(2.0): Add an include_direct_sources & include_direct_declarations & include_direct_npm_linked_packages and
# rename include_declarations to include_transitive declarations & include_npm_linked_packages to include_transitive_npm_linked_packages.
js_filegroup = rule(
doc = _DOC,
implementation = _js_filegroup_impl,
Expand All @@ -25,14 +25,25 @@ js_filegroup = rule(
doc = """List of targets to gather files from.""",
allow_files = True,
),
"include_sources": attr.bool(
doc = """When True, `sources` from `JsInfo` providers in `srcs` targets are included in the default outputs of the target.""",
default = True,
),
"include_transitive_sources": attr.bool(
doc = """When True, `transitive_sources` from `JsInfo` providers in `srcs` targets are included in the default outputs of the target.""",
default = True,
),
"include_declarations": attr.bool(
doc = """When True, `declarations` and `transitive_declarations` from `JsInfo` providers in srcs targets are included in the default outputs of the target.
doc = """When True, `declarations` from `JsInfo` providers in srcs targets are included in the default outputs of the target.
Defaults to False since declarations are generally not needed at runtime and introducing them could slow down developer round trip
time due to having to generate typings on source file changes.""",
default = False,
),
"include_transitive_declarations": attr.bool(
doc = """When True, `transitive_declarations` from `JsInfo` providers in srcs targets are included in the default outputs of the target.
Defaults to false since declarations are generally not needed at runtime and introducing them could slow down developer round trip
Defaults to False since declarations are generally not needed at runtime and introducing them could slow down developer round trip
time due to having to generate typings on source file changes.""",
default = False,
),
Expand Down
45 changes: 37 additions & 8 deletions js/private/js_helpers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,19 @@ this option is not needed.

return copy_file_to_bin_action(ctx, file)

def gather_runfiles(ctx, sources, data, deps, data_files = [], copy_data_files_to_bin = False, no_copy_to_bin = [], include_transitive_sources = True, include_declarations = False, include_npm_linked_packages = True):
def gather_runfiles(
ctx,
sources,
data,
deps,
data_files = [],
copy_data_files_to_bin = False,
no_copy_to_bin = [],
include_sources = True,
include_transitive_sources = True,
include_declarations = False,
include_transitive_declarations = False,
include_npm_linked_packages = True):
"""Creates a runfiles object containing files in `sources`, default outputs from `data` and transitive runfiles from `data` & `deps`.
As a defense in depth against `data` & `deps` targets not supplying all required runfiles, also
Expand Down Expand Up @@ -210,10 +222,14 @@ def gather_runfiles(ctx, sources, data, deps, data_files = [], copy_data_files_t
file such as a file in an external repository. In most cases, this option is not needed.
See `copy_data_files_to_bin` docstring for more info.
include_sources: see js_filegroup documentation
include_transitive_sources: see js_filegroup documentation
include_declarations: see js_filegroup documentation
include_transitive_declarations: see js_filegroup documentation
include_npm_linked_packages: see js_filegroup documentation
Returns:
Expand All @@ -231,12 +247,13 @@ def gather_runfiles(ctx, sources, data, deps, data_files = [], copy_data_files_t
for target in data
])

# Gather the transitive sources & transitive npm linked packages from the JsInfo &
# NpmPackageStoreInfo providers of data & deps targets.
# Gather files from JsInfo providers of data & deps
transitive_files_depsets.append(gather_files_from_js_providers(
targets = data + deps,
include_sources = include_sources,
include_transitive_sources = include_transitive_sources,
include_declarations = include_declarations,
include_transitive_declarations = include_transitive_declarations,
include_npm_linked_packages = include_npm_linked_packages,
))

Expand Down Expand Up @@ -296,32 +313,44 @@ def envs_for_log_level(log_level):

def gather_files_from_js_providers(
targets,
include_sources,
include_transitive_sources,
include_declarations,
include_transitive_declarations,
include_npm_linked_packages):
"""Gathers files from JsInfo and NpmPackageStoreInfo providers.
Args:
targets: list of target to gather from
include_sources: see js_filegroup documentation
include_transitive_sources: see js_filegroup documentation
include_declarations: see js_filegroup documentation
include_transitive_declarations: see js_filegroup documentation
include_npm_linked_packages: see js_filegroup documentation
Returns:
A depset of files
"""
files_depsets = [
target[JsInfo].sources
for target in targets
if JsInfo in target and hasattr(target[JsInfo], "sources")
]
files_depsets = []
if include_sources:
files_depsets = [
target[JsInfo].sources
for target in targets
if JsInfo in target and hasattr(target[JsInfo], "sources")
]
if include_transitive_sources:
files_depsets.extend([
target[JsInfo].transitive_sources
for target in targets
if JsInfo in target and hasattr(target[JsInfo], "transitive_sources")
])
if include_declarations:
files_depsets.extend([
target[JsInfo].declarations
for target in targets
if JsInfo in target and hasattr(target[JsInfo], "declarations")
])
if include_transitive_declarations:
files_depsets.extend([
target[JsInfo].transitive_declarations
for target in targets
Expand Down
2 changes: 2 additions & 0 deletions js/private/js_run_devserver.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ def _js_run_devserver_impl(ctx):

transitive_runfiles = [_gather_files_from_js_providers(
targets = ctx.attr.data,
include_sources = True,
include_transitive_sources = ctx.attr.include_transitive_sources,
include_declarations = ctx.attr.include_declarations,
include_transitive_declarations = ctx.attr.include_declarations,
include_npm_linked_packages = ctx.attr.include_npm_linked_packages,
)]

Expand Down
2 changes: 2 additions & 0 deletions js/private/test/create_launcher/custom_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ def _custom_test_impl(ctx):
files = ctx.files.data,
transitive_files = js_lib_helpers.gather_files_from_js_providers(
targets = ctx.attr.data,
include_sources = True,
include_transitive_sources = ctx.attr.include_transitive_sources,
include_declarations = ctx.attr.include_declarations,
include_transitive_declarations = ctx.attr.include_declarations,
include_npm_linked_packages = ctx.attr.include_npm_linked_packages,
),
).merge(launcher.runfiles).merge_all([
Expand Down

0 comments on commit 0f3b38f

Please sign in to comment.