Skip to content

Commit

Permalink
refactor: update to rules_js 1.0.0-rc.4
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan committed Aug 2, 2022
1 parent 8577e7e commit 5dfa5f4
Show file tree
Hide file tree
Showing 13 changed files with 82 additions and 67 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
matrix:
folder:
- '.'
- 'e2e/bzlmod'
# - 'e2e/bzlmod' # FIXME
- 'e2e/workspace'
- 'e2e/worker'

Expand Down Expand Up @@ -57,7 +57,7 @@ jobs:
- uses: actions/setup-go@v3
if: ${{ hashFiles(format('{0}/test.sh', matrix.folder)) != '' }}
with:
go-version: '1.17.0'
go-version: '1.17.0'
- run: go install github.com/bazelbuild/buildtools/buildozer@latest
if: ${{ hashFiles(format('{0}/test.sh', matrix.folder)) != '' }}
- name: run ./test.sh
Expand Down
2 changes: 1 addition & 1 deletion docs/repositories.md

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

2 changes: 1 addition & 1 deletion docs/rules.md

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

2 changes: 0 additions & 2 deletions examples/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ npm_link_package(
src = "//examples/deps_lib",
root_package = "examples",
visibility = ["//examples:__subpackages__"],
# TODO: propagate this dependency from //examples/deps_lib: https://github.com/aspect-build/rules_ts/issues/96
deps = {":.aspect_rules_js/node_modules/date-fns/2.29.1": ""},
)

# This macro expands to a npm_link_package for each third-party package in package.json
Expand Down
9 changes: 5 additions & 4 deletions examples/deps_lib/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ ts_config(
ts_project(
name = "deps_ts",
srcs = ["index.ts"],
data = [
# Also propagate the date-fns npm package downstream so it is an runtime dependency
# of any linked npm_package that this target is used by.
"//examples:node_modules/date-fns",
],
declaration = True,
# TODO: fix worker mode flaky bug when there are multiple ts_project targets in a package
supports_workers = False,
Expand All @@ -23,10 +28,6 @@ npm_package(
name = "deps_lib",
srcs = [
":deps_ts",
# TODO: This seems undesirable to have to specify the transitive closure of ts_project
# targets. However, if npm_package is meant to use DefaultInfo then this _is_ technically
# the correct behavior.
"//examples/deps_lib:b",
],
package = "@myorg/deps_lib",
visibility = ["//examples:__subpackages__"],
Expand Down
1 change: 0 additions & 1 deletion ts/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ bzl_library(
"@aspect_rules_js//js:defs",
"@bazel_skylib//lib:partial",
"@bazel_skylib//rules:build_test",
"@rules_nodejs//nodejs/private:bzl", # keep
],
)

Expand Down
4 changes: 2 additions & 2 deletions ts/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ def ts_project(
native.filegroup(
name = typecheck_target_name,
srcs = [tsc_target_name],
# This causes the DeclarationInfo to be produced, which in turn triggers the tsc action to typecheck
# This causes the declarations to be produced, which in turn triggers the tsc action to typecheck
output_group = "types",
**common_kwargs
)
Expand All @@ -416,7 +416,7 @@ def ts_project(
js_library(
name = name,
# Include the tsc target in srcs to pick-up both the direct & transitive declaration outputs so
# that this js_library can be a valid dep for downstream ts_project or other DeclarationInfo-aware rules.
# that this js_library can be a valid dep for downstream ts_project or other rules_js derivative rules.
srcs = js_outs + map_outs + [tsc_target_name],
deps = deps,
**common_kwargs
Expand Down
5 changes: 3 additions & 2 deletions ts/private/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ bzl_library(
":ts_validate_options",
"@aspect_bazel_lib//lib:copy_to_bin",
"@aspect_bazel_lib//lib:utils",
"@aspect_rules_js//js:libs",
"@aspect_rules_js//js:providers",
"@bazel_skylib//lib:dicts",
"@rules_nodejs//nodejs/private/providers:bzl", # keep
],
)

Expand All @@ -40,7 +41,7 @@ bzl_library(
name = "ts_lib",
srcs = ["ts_lib.bzl"],
visibility = ["//ts:__subpackages__"],
deps = ["@rules_nodejs//nodejs:bzl"], # keep
deps = ["@aspect_rules_js//js:providers"],
)

bzl_library(
Expand Down
8 changes: 4 additions & 4 deletions ts/private/npm_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ def _http_archive_version_impl(rctx):
elif "dependencies" in p.keys() and "typescript" in p["dependencies"]:
ts = p["dependencies"]["typescript"]
else:
fail("key `typescript` not found in either dependencies or devDependencies of %s" % json_path)
fail("key 'typescript' not found in either dependencies or devDependencies of %s" % json_path)
if any([not seg.isdigit() for seg in ts.split(".")]):
fail("""typescript version in package.json must be exactly specified, not a semver range: %s.
You can supply an exact `ts_version` attribute to `rules_ts_dependencies` to bypass this check.""" % ts)
You can supply an exact 'ts_version' attribute to 'rules_ts_dependencies' to bypass this check.""" % ts)
version = ts

if rctx.attr.integrity:
Expand All @@ -38,7 +38,7 @@ def _http_archive_version_impl(rctx):
integrity = TS_VERSIONS[version]
else:
fail("""typescript version {} is not mirrored in rules_ts, is this a real version?
If so, you must manually set `ts_integrity`.
If so, you must manually set 'ts_integrity'.
See documentation on rules_ts_dependencies.""".format(version))

rctx.download_and_extract(
Expand Down Expand Up @@ -70,7 +70,7 @@ http_archive_version = repository_rule(
# buildifier: disable=function-docstring
def npm_dependencies(ts_version_from = None, ts_version = None, ts_integrity = None):
if (ts_version and ts_version_from) or (not ts_version_from and not ts_version):
fail("""Exactly one of `ts_version` or `ts_version_from` must be set.""")
fail("""Exactly one of 'ts_version' or 'ts_version_from' must be set.""")

maybe(
http_archive,
Expand Down
11 changes: 4 additions & 7 deletions ts/private/ts_lib.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"Utilities functions for selecting and filtering ts and other files"

load("@rules_nodejs//nodejs:providers.bzl", "DeclarationInfo")
load("@aspect_rules_js//js:providers.bzl", "JsInfo")
load("@aspect_rules_js//js:libs.bzl", "js_lib_helpers")

ValidOptionsInfo = provider(
doc = "Internal: whether the validator ran successfully",
Expand All @@ -12,7 +13,7 @@ ValidOptionsInfo = provider(

# Targets in deps must provide one or the other of these
DEPS_PROVIDERS = [
[DeclarationInfo],
[JsInfo],
[ValidOptionsInfo],
]

Expand All @@ -21,11 +22,7 @@ STD_ATTRS = {
"args": attr.string_list(
doc = "https://www.typescriptlang.org/docs/handbook/compiler-options.html",
),
"data": attr.label_list(
doc = "Runtime dependencies to include in binaries/tests that depend on this TS code",
default = [],
allow_files = True,
),
"data": js_lib_helpers.JS_LIBRARY_DATA_ATTR,
"declaration_dir": attr.string(
doc = "https://www.typescriptlang.org/tsconfig#declarationDir",
),
Expand Down
89 changes: 54 additions & 35 deletions ts/private/ts_project.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_files_to_bin_actions")
load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("@rules_nodejs//nodejs:providers.bzl", "DeclarationInfo", "declaration_info", "js_module_info")
load("@aspect_rules_js//js:providers.bzl", "js_info")
load("@aspect_rules_js//js:libs.bzl", "js_lib_helpers")
load(":ts_lib.bzl", "COMPILER_OPTION_ATTRS", "OUTPUT_ATTRS", "STD_ATTRS", "ValidOptionsInfo", _lib = "lib")
load(":ts_config.bzl", "TsConfigInfo")
load(":ts_validate_options.bzl", _validate_lib = "lib")
Expand Down Expand Up @@ -84,17 +85,17 @@ def _ts_project_impl(ctx):
"--extendedDiagnostics",
])

deps_depsets = []
inputs = ctx.files.srcs[:]
for dep in ctx.attr.deps:
if TsConfigInfo in dep:
deps_depsets.append(dep[TsConfigInfo].deps)
if DeclarationInfo in dep:
deps_depsets.append(dep[DeclarationInfo].transitive_declarations)
if ValidOptionsInfo in dep:
inputs.append(dep[ValidOptionsInfo].marker)

inputs.extend(depset(transitive = deps_depsets).to_list())
inputs.extend(js_lib_helpers.gather_files_from_js_providers(
targets = ctx.attr.srcs + ctx.attr.deps,
include_transitive_sources = True,
include_declarations = True,
include_npm_linked_packages = True,
))

# Gather TsConfig info from both the direct (tsconfig) and indirect (extends) attribute
tsconfig_inputs = copy_files_to_bin_actions(ctx, _validate_lib.tsconfig_inputs(ctx))
Expand Down Expand Up @@ -123,7 +124,7 @@ def _ts_project_impl(ctx):
ctx.outputs.buildinfo_out.short_path,
])
outputs.append(ctx.outputs.buildinfo_out)
runtime_outputs = json_outs + js_outs + map_outs
output_sources = json_outs + js_outs + map_outs
typings_srcs = [s for s in ctx.files.srcs if s.path.endswith(".d.ts")]

if len(js_outs) + len(typings_outs) < 1:
Expand Down Expand Up @@ -153,22 +154,28 @@ This might be because
- `srcs` has elements producing non-ts outputs
""" % label
else:
no_outs_msg = "ts_project target %s with custom transpiler needs `declaration = True`." % label
no_outs_msg = "ts_project target %s with custom transpiler needs 'declaration = True'." % label
fail(no_outs_msg + """
This is an error because Bazel does not run actions unless their outputs are needed for the requested targets to build.
""")

typings_outputs = typings_outs + typing_maps_outs + typings_srcs
output_declarations = typings_outs + typing_maps_outs + typings_srcs

# Default outputs (DefaultInfo files) is what you see on the command-line for a built
# library, and determines what files are used by a simple non-provider-aware downstream
# library. Only the JavaScript outputs are intended for use in non-TS-aware dependents.
if ctx.attr.transpile:
default_outputs_depset = depset(runtime_outputs) if len(runtime_outputs) else depset(typings_outputs)
# Special case case where there are no source outputs and we don't have a custom
# transpiler so we add output_declarations to the default outputs
default_outputs = output_sources[:] if len(output_sources) else output_declarations[:]
else:
# We must avoid tsc writing any JS files in this case, as tsc was only run for typings, and some other
# action will try to write the JS files. We must avoid collisions where two actions write the same file.
arguments.add("--emitDeclarationOnly")

# We don't produce any DefaultInfo outputs in this case, because we avoid running the tsc action
# unless the DeclarationInfo is requested.
default_outputs_depset = depset([])
# unless the output_declarations are requested.
default_outputs = []

if len(outputs) > 0:
ctx.actions.run(
Expand All @@ -187,30 +194,49 @@ This is an error because Bazel does not run actions unless their outputs are nee
},
)

transitive_sources = js_lib_helpers.gather_transitive_sources(output_sources, ctx.attr.srcs + ctx.attr.deps)

transitive_declarations = js_lib_helpers.gather_transitive_declarations(output_declarations, ctx.attr.srcs + ctx.attr.deps)

npm_linked_packages = js_lib_helpers.gather_npm_linked_packages(
srcs = ctx.attr.srcs,
deps = ctx.attr.deps,
)

npm_package_stores = js_lib_helpers.gather_npm_package_stores(
targets = ctx.attr.data,
)

runfiles = js_lib_helpers.gather_runfiles(
ctx = ctx,
sources = output_sources,
data = ctx.attr.data,
deps = ctx.attr.srcs + ctx.attr.deps,
)

providers = [
# DefaultInfo is what you see on the command-line for a built library,
# and determines what files are used by a simple non-provider-aware
# downstream library.
# Only the JavaScript outputs are intended for use in non-TS-aware
# dependents.
DefaultInfo(
files = default_outputs_depset,
runfiles = ctx.runfiles(
transitive_files = depset(ctx.files.data, transitive = [
default_outputs_depset,
]),
collect_default = True,
),
files = depset(default_outputs),
runfiles = runfiles,
),
js_module_info(
sources = depset(runtime_outputs),
deps = ctx.attr.deps,
js_info(
declarations = output_declarations,
npm_linked_packages = npm_linked_packages.direct,
npm_package_stores = npm_package_stores.direct,
sources = output_sources,
transitive_declarations = transitive_declarations,
transitive_npm_linked_packages = npm_linked_packages.transitive,
transitive_npm_package_stores = npm_package_stores.transitive,
transitive_sources = transitive_sources,
),
TsConfigInfo(deps = depset(tsconfig_inputs, transitive = [
dep[TsConfigInfo].deps
for dep in ctx.attr.deps
if TsConfigInfo in dep
])),
OutputGroupInfo(
types = depset(output_declarations),
),
coverage_common.instrumented_files_info(
ctx,
source_attributes = ["srcs"],
Expand All @@ -219,13 +245,6 @@ This is an error because Bazel does not run actions unless their outputs are nee
),
]

# Only provide DeclarationInfo if there are some typings.
# Improves error messaging if a ts_project is missing declaration = True
typings_in_deps = [d for d in ctx.attr.deps if DeclarationInfo in d]
if len(typings_outputs) or len(typings_in_deps):
providers.append(declaration_info(depset(typings_outputs), typings_in_deps))
providers.append(OutputGroupInfo(types = depset(typings_outputs)))

return providers

ts_project = struct(
Expand Down
8 changes: 4 additions & 4 deletions ts/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ LATEST_VERSION = TS_VERSIONS.keys()[-1]
def rules_ts_dependencies(ts_version_from = None, ts_version = None, ts_integrity = None):
"""Dependencies needed by users of rules_ts.
To skip fetching the typescript package, define repository called 'npm_typescript' before calling this.
To skip fetching the typescript package, define repository called `npm_typescript` before calling this.
Args:
ts_version_from: label of a json file (typically `package.json`) which declares an exact typescript version
Expand Down Expand Up @@ -55,9 +55,9 @@ def rules_ts_dependencies(ts_version_from = None, ts_version = None, ts_integrit
maybe(
http_archive,
name = "aspect_rules_js",
sha256 = "b82da82edf64ba7e07e568193d645fc09b0a4ec92e0d82bd4e53d1a0e28ff681",
strip_prefix = "rules_js-1.0.0-rc.3",
url = "https://github.com/aspect-build/rules_js/archive/refs/tags/v1.0.0-rc.3.tar.gz",
sha256 = "33363f905dd4bb691160351709804f40efd642d56b808704e1c0d4b263bed442",
strip_prefix = "rules_js-c3f764367e5b9e0067e2ff15b4382ec2f080d96b",
url = "https://github.com/aspect-build/rules_js/archive/c3f764367e5b9e0067e2ff15b4382ec2f080d96b.tar.gz",
)

maybe(
Expand Down
4 changes: 2 additions & 2 deletions ts/test/transpiler_tests.bzl
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
"Unit tests for starlark API of ts_project with custom transpiler"

load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
load("@rules_nodejs//nodejs:providers.bzl", "DeclarationInfo")
load("@aspect_rules_js//js:providers.bzl", "JsInfo")

def _impl0(ctx):
env = unittest.begin(ctx)

decls = []
for decl in ctx.attr.lib[DeclarationInfo].declarations.to_list():
for decl in ctx.attr.lib[JsInfo].declarations:
decls.append(decl.basename)
asserts.equals(env, ctx.attr.expected_declarations, sorted(decls))

Expand Down

0 comments on commit 5dfa5f4

Please sign in to comment.