Skip to content

Commit

Permalink
refactor: simplify json output file declaration
Browse files Browse the repository at this point in the history
Never predeclare these outputs, since in most cases we can't
  • Loading branch information
Alex Eagle authored and alexeagle committed Jul 6, 2020
1 parent c9a9e0e commit 6c81478
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 49 deletions.
22 changes: 13 additions & 9 deletions packages/typescript/internal/ts_project.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ _ATTRS = {
_OUTPUTS = {
"buildinfo_out": attr.output(),
"js_outs": attr.output_list(),
"json_outs": attr.output_list(),
"map_outs": attr.output_list(),
"typing_maps_outs": attr.output_list(),
"typings_outs": attr.output_list(),
Expand Down Expand Up @@ -98,12 +97,20 @@ def _ts_project_impl(ctx):
if ctx.attr.extends:
inputs.extend(ctx.files.extends)

json_outs = ctx.outputs.json_outs
# We do not try to predeclare json_outs, because their output locations generally conflict with their path in the source tree.
# (The exception is when outdir is used, then the .json output is a different path than the input.)
# However tsc will copy .json srcs to the output tree so we want to declare these outputs to include along with .js Default outs
# NB: We don't have emit_declaration_only setting here, so use presence of any JS outputs as an equivalent.
# tsc will only produce .json if it also produces .js
if len(ctx.outputs.js_outs):
json_outs = [
ctx.actions.declare_file(_join(ctx.attr.outdir, src.short_path[len(ctx.label.package) + 1:]))
for src in ctx.files.srcs
if src.basename.endswith(".json")
]
else:
json_outs = []

# If there are no predeclared json_outs (probably bc of no .json or no outdir),
# let's try to search for them in srcs so we can declare them as output
if not json_outs:
json_outs = [ctx.actions.declare_file(src.basename, sibling = src) for src in ctx.files.srcs if src.basename.endswith(".json")]
outputs = json_outs + ctx.outputs.js_outs + ctx.outputs.map_outs + ctx.outputs.typings_outs + ctx.outputs.typing_maps_outs
if ctx.outputs.buildinfo_out:
outputs.append(ctx.outputs.buildinfo_out)
Expand Down Expand Up @@ -398,9 +405,6 @@ def ts_project_macro(
tsconfig = tsconfig,
extends = extends,
outdir = outdir,
# JSON files are special. They are output if outdir is set since tsc will
# copy them to outdir. Otherwise they are kind of both.
json_outs = [_join(outdir, f) for f in srcs if f.endswith(".json")] if outdir and not emit_declaration_only else [],
js_outs = _out_paths(srcs, outdir, ".js") if not emit_declaration_only else [],
map_outs = _out_paths(srcs, outdir, ".js.map") if source_map and not emit_declaration_only else [],
typings_outs = _out_paths(srcs, outdir, ".d.ts") if declaration or composite else [],
Expand Down
31 changes: 15 additions & 16 deletions packages/typescript/test/ts_project/json/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", "nodejs_test")
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
load("//packages/typescript:index.bzl", "ts_project")

SRCS = glob(["**/*.ts*"]) + [
SRCS = [
"subdir/a.ts",
"subdir/foo.json",
"bar.json",
]
Expand All @@ -18,25 +19,23 @@ ts_project(
tsconfig = "tsconfig.json",
)

generated_file_test(
name = "test-subdir",
src = "foo.golden.json",
# Refers to the output from tsconfig ts_project above
generated = ":foobar/subdir/foo.json",
)

generated_file_test(
name = "test",
src = "bar.golden.json",
# Refers to the output from tsconfig ts_project above
generated = ":foobar/bar.json",
# Test that we don't try to declare .json outputs when tsc isn't producing any JS
ts_project(
name = "tsconfig-decl-only",
srcs = SRCS,
emit_declaration_only = True,
)

nodejs_test(
name = "test-no-outdir",
data = [":tsconfig-no-outdir"],
name = "test",
data = [
":tsconfig",
":tsconfig-decl-only",
":tsconfig-no-outdir",
],
entry_point = "verify.js",
templated_args = [
"$(locations :tsconfig)",
"$(locations :tsconfig-no-outdir)",
],
)
3 changes: 0 additions & 3 deletions packages/typescript/test/ts_project/json/bar.golden.json

This file was deleted.

3 changes: 0 additions & 3 deletions packages/typescript/test/ts_project/json/foo.golden.json

This file was deleted.

26 changes: 8 additions & 18 deletions packages/typescript/test/ts_project/json/verify.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,8 @@
const files = process.argv.slice(2)
let barJson
let subdirFooJson
files.forEach(f => {
if (f.endsWith('bar.json')) {
barJson = f
} else if (f.endsWith('subdir/foo.json')) {
subdirFooJson = f
}
})
if (!barJson) {
console.error('Missing bar.json')
process.exitCode = 1
}
else if (!subdirFooJson) {
console.error('Missing subdir/foo.json')
process.exitCode = 1
}
const assert = require('assert');

const files = process.argv.slice(2);
assert.ok(files.some(f => f.endsWith('json/bar.json')), 'Missing bar.json');
assert.ok(files.some(f => f.endsWith('json/subdir/foo.json')), 'Missing subdir/foo.json');
assert.ok(files.some(f => f.endsWith('json/foobar/bar.json')), 'Missing outdir bar.json');
assert.ok(
files.some(f => f.endsWith('json/foobar/subdir/foo.json')), 'Missing outdir subdir/foo.json');

0 comments on commit 6c81478

Please sign in to comment.