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

refactor: add include_sources and include_transitive_declarations to js_filegroup rules and gather_files_from_js_providers + gather_runfiles helpers #1585

Merged
merged 1 commit into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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