Skip to content
This repository was archived by the owner on Sep 16, 2021. It is now read-only.

Fix for Bazel incompatible changes #378

Closed
wants to merge 1 commit into from
Closed

Fix for Bazel incompatible changes #378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion internal/build_defs.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def _compile_action(ctx, inputs, outputs, tsconfig_file, node_opts, description
# These deps are identified by the NodeModuleInfo provider.
for d in ctx.attr.deps:
if NodeModuleInfo in d:
action_inputs.extend(_filter_ts_inputs(d.files))
action_inputs.extend(_filter_ts_inputs(d.files.to_list()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to_list() has some performance issues and should be avoided (see ref).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is subject to the performance problem either way. The question is whether to_list is done implicitly by passing the depset to an API that expects a list, or done explicitly. I think the bazel team is moving us to the latter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it was just to make the behavior more obvious. Bazel implicitly flattens depsets in some places (which is a performance problem). We want to forbid that. The simplest way forward is to make all flattening explicit in existing files.


if ctx.file.tsconfig:
action_inputs.append(ctx.file.tsconfig)
Expand Down
12 changes: 6 additions & 6 deletions internal/common/compilation.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def compile_ts(
tsickle_externs = [ctx.actions.declare_file(ctx.label.name + ".externs.js")]

dep_declarations = _collect_dep_declarations(ctx, deps)
input_declarations = dep_declarations.transitive + src_declarations
input_declarations = depset(src_declarations, transitive = [dep_declarations.transitive])
type_blacklisted_declarations = dep_declarations.type_blacklisted
if not is_library and not ctx.attr.generate_externs:
type_blacklisted_declarations += srcs_files
Expand All @@ -277,7 +277,7 @@ def compile_ts(
if "TYPESCRIPT_PERF_TRACE_TARGET" in ctx.var:
perf_trace = str(ctx.label) == ctx.var["TYPESCRIPT_PERF_TRACE_TARGET"]

compilation_inputs = input_declarations + srcs_files
compilation_inputs = depset(srcs_files, transitive = [input_declarations])
tsickle_externs_path = tsickle_externs[0] if tsickle_externs else None

# Calculate allowed dependencies for strict deps enforcement.
Expand All @@ -294,7 +294,7 @@ def compile_ts(
srcs_files,
jsx_factory = jsx_factory,
tsickle_externs = tsickle_externs_path,
type_blacklisted_declarations = type_blacklisted_declarations,
type_blacklisted_declarations = type_blacklisted_declarations.to_list(),
allowed_deps = allowed_deps,
)

Expand Down Expand Up @@ -333,7 +333,7 @@ def compile_ts(
replay_params = None

if has_sources:
inputs = compilation_inputs + [ctx.outputs.tsconfig]
inputs = depset([ctx.outputs.tsconfig], transitive = [compilation_inputs])
replay_params = compile_action(
ctx,
inputs,
Expand Down Expand Up @@ -377,7 +377,7 @@ def compile_ts(
ctx.actions.write(output = tsconfig_json_es5, content = json_marshal(
tsconfig_es5,
))
inputs = compilation_inputs + [tsconfig_json_es5]
inputs = depset([tsconfig_json_es5], transitive = [compilation_inputs])
devmode_compile_action(
ctx,
inputs,
Expand All @@ -388,7 +388,7 @@ def compile_ts(

# TODO(martinprobst): Merge the generated .d.ts files, and enforce strict
# deps (do not re-export transitive types from the transitive closure).
transitive_decls = dep_declarations.transitive + src_declarations + gen_declarations
transitive_decls = depset(src_declarations + gen_declarations, transitive = [dep_declarations.transitive])

# both ts_library and ts_declarations generate .closure.js files:
# - for libraries, this is the ES6/production code
Expand Down
2 changes: 1 addition & 1 deletion internal/common/tsconfig.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,6 @@ def create_tsconfig(
return {
"compilerOptions": compiler_options,
"bazelOptions": bazel_options,
"files": [workspace_path + "/" + f.path for f in files],
"files": [workspace_path + "/" + f.path for f in files.to_list()],
"compileOnSave": False,
}
10 changes: 5 additions & 5 deletions internal/protobufjs/ts_proto_library.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ def _run_pbjs(actions, executable, output_name, proto_files, suffix = ".js", wra
# Reference of arguments:
# https://github.com/dcodeIO/ProtoBuf.js/#pbjs-for-javascript
args = actions.args()
args.add(["--target", "static-module"])
args.add(["--wrap", wrap])
args.add_all(["--target", "static-module"])
args.add_all(["--wrap", wrap])
args.add("--strict-long") # Force usage of Long type with int64 fields
args.add(["--out", js_file.path + ".tmpl"])
args.add([f.path for f in proto_files])
args.add_all(["--out", js_file.path + ".tmpl"])
args.add_all(proto_files)

actions.run(
executable = executable._pbjs,
Expand Down Expand Up @@ -55,7 +55,7 @@ def _run_pbts(actions, executable, js_file):
# Reference of arguments:
# https://github.com/dcodeIO/ProtoBuf.js/#pbts-for-typescript
args = actions.args()
args.add(["--out", ts_file.path])
args.add_all(["--out", ts_file.path])
args.add(js_file.path)

actions.run(
Expand Down
9 changes: 5 additions & 4 deletions package.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ def rules_typescript_dev_dependencies():
_maybe(
http_archive,
name = "io_bazel",
urls = ["https://github.com/bazelbuild/bazel/releases/download/0.17.1/bazel-0.17.1-dist.zip"],
urls = ["https://github.com/bazelbuild/bazel/releases/download/0.21.0/bazel-0.21.0-dist.zip"],
sha256 = "6ccb831e683179e0cfb351cb11ea297b4db48f9eab987601c038aa0f83037db4",
)

#############################################
Expand All @@ -118,9 +119,9 @@ def rules_typescript_dev_dependencies():

http_archive(
name = "io_bazel_skydoc",
url = "https://github.com/bazelbuild/skydoc/archive/9bbdf62c03b5c3fed231604f78d3976f47753d79.zip", # 2018-11-20
strip_prefix = "skydoc-9bbdf62c03b5c3fed231604f78d3976f47753d79",
sha256 = "07ae937026cb56000fb268d4986b220e868c1bdfe6aac27ecada4b4b3dae246f",
url = "https://github.com/bazelbuild/skydoc/archive/82fdbfe797c6591d8732df0c0389a2b1c3e50992.zip", # 2018-12-12
strip_prefix = "skydoc-82fdbfe797c6591d8732df0c0389a2b1c3e50992",
sha256 = "75fd965a71ca1f0d0406d0d0fb0964d24090146a853f58b432761a1a6c6b47b9",
)

def _maybe(repo_rule, name, **kwargs):
Expand Down