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

feat(typescript): support for declarationdir on ts_project #2048

Merged
merged 5 commits into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
18 changes: 14 additions & 4 deletions packages/typescript/internal/ts_project.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ _DEFAULT_TSC = (

_ATTRS = {
"args": attr.string_list(),
"declarationdir": attr.string(),
"deps": attr.label_list(providers = [DeclarationInfo]),
"extends": attr.label_list(allow_files = [".json"]),
"outdir": attr.string(),
Expand Down Expand Up @@ -63,7 +64,9 @@ def _ts_project_impl(ctx):
if len(ctx.outputs.typings_outs) > 0:
arguments.add_all([
"--declarationDir",
_join(ctx.bin_dir.path, ctx.label.package, ctx.attr.outdir),
(
_join(ctx.bin_dir.path, ctx.label.package, ctx.attr.declarationdir) if ctx.attr.declarationdir else _join(ctx.bin_dir.path, ctx.label.package, ctx.attr.outdir)
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh we made a naming mistake on the others - TS has "rootDir" so we should have named the attribute "root_dir". Same with "out_dir" and "declaration_dir"
I think maybe it was because other rules have "outdir" so we wanted that one to match

It's not too late to change, none of the attributes were released yet
https://github.com/bazelbuild/rules_nodejs/blob/1.x/packages/typescript/src/internal/ts_project.bzl

Would you change all three of them in a second commit here? If you don't have time today I'll merge this and do it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

this logic would be simpler if we make declarationDir default to outDir prior to this statement

),
])

# When users report problems, we can ask them to re-build with
Expand Down Expand Up @@ -115,7 +118,7 @@ def _ts_project_impl(ctx):
if ctx.outputs.buildinfo_out:
outputs.append(ctx.outputs.buildinfo_out)
runtime_outputs = depset(json_outs + ctx.outputs.js_outs + ctx.outputs.map_outs)
typings_outputs = ctx.outputs.typings_outs + [s for s in ctx.files.srcs if s.path.endswith(".d.ts")]
typings_outputs = ctx.outputs.typings_outs + ctx.outputs.typing_maps_outs + [s for s in ctx.files.srcs if s.path.endswith(".d.ts")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

why make this change? I don't think .map files belong in DeclarationInfo, they are meant to be consumed by editors not by other tsc actions that need to type-check

Copy link
Collaborator

Choose a reason for hiding this comment

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

checked with @gregmagolan - you are correct we transport .map files along with the thing they map (it's like that for .js.map files for example)


if len(outputs) > 0:
run_node(
Expand Down Expand Up @@ -230,6 +233,7 @@ def ts_project_macro(
emit_declaration_only = False,
tsc = None,
validate = True,
declarationdir = None,
outdir = None,
rootdir = None,
**kwargs):
Expand Down Expand Up @@ -357,6 +361,9 @@ def ts_project_macro(

validate: boolean; whether to check that the tsconfig settings match the attributes.

declarationdir: a string specifying a subdirectory under the bazel-out folder where generated declaration
outputs are written.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Include what is the default value pls


outdir: a string specifying a subdirectory under the bazel-out folder where outputs are written.
Note that Bazel always requires outputs be written under a subdirectory matching the input package,
so if your rule appears in path/to/my/package/BUILD.bazel and outdir = "foo" then the .js files
Expand Down Expand Up @@ -402,19 +409,22 @@ def ts_project_macro(
)
extra_deps.append("_validate_%s_options" % name)

typings_outdir = declarationdir if declarationdir else outdir

ts_project(
name = name,
srcs = srcs,
args = args,
deps = deps + extra_deps,
tsconfig = tsconfig,
extends = extends,
declarationdir = declarationdir,
outdir = outdir,
rootdir = rootdir,
js_outs = _out_paths(srcs, outdir, rootdir, ".js") if not emit_declaration_only else [],
map_outs = _out_paths(srcs, outdir, rootdir, ".js.map") if source_map and not emit_declaration_only else [],
typings_outs = _out_paths(srcs, outdir, rootdir, ".d.ts") if declaration or composite else [],
typing_maps_outs = _out_paths(srcs, outdir, rootdir, ".d.ts.map") if declaration_map else [],
typings_outs = _out_paths(srcs, typings_outdir, rootdir, ".d.ts") if declaration or composite else [],
typing_maps_outs = _out_paths(srcs, typings_outdir, rootdir, ".d.ts.map") if declaration_map else [],
buildinfo_out = tsconfig[:-5] + ".tsbuildinfo" if composite or incremental else None,
tsc = tsc,
**kwargs
Expand Down
36 changes: 36 additions & 0 deletions packages/typescript/test/ts_project/declarationdir/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
load("//packages/typescript:index.bzl", "ts_project")

# Ensure that subdir/a.ts produces outDir/a.js and outDir/a.d.ts
SRCS = [
"subdir/a.ts",
]

ts_project(
name = "tsconfig",
srcs = SRCS,
declaration = True,
declaration_map = True,
outdir = "out",
rootdir = "subdir",
source_map = True,
)

filegroup(
name = "types",
srcs = [":tsconfig"],
output_group = "types",
)

nodejs_test(
name = "test",
data = [
":tsconfig",
":types",
],
entry_point = "verify.js",
templated_args = [
"$(locations :types)",
"$(locations :tsconfig)",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const a: string = 'hello';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"sourceMap": true,
"declaration": true,
"declarationMap": true,
"types": []
}
}
8 changes: 8 additions & 0 deletions packages/typescript/test/ts_project/declarationdir/verify.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const assert = require('assert');

const types_files = process.argv.slice(2, 4);
const code_files = process.argv.slice(4, 6);
assert.ok(types_files.some(f => f.endsWith('declarationdir/out/a.d.ts')), 'Missing a.d.ts');
assert.ok(types_files.some(f => f.endsWith('declarationdir/out/a.d.ts.map')), 'Missing a.d.ts.map');
assert.ok(code_files.some(f => f.endsWith('declarationdir/out/a.js')), 'Missing a.js');
assert.ok(code_files.some(f => f.endsWith('declarationdir/out/a.js.map')), 'Missing a.js.map');
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
load("//packages/typescript:index.bzl", "ts_project")

# Ensure that subdir/a.ts produces outDir/a.js and declarationDir/a.d.ts
SRCS = [
"subdir/a.ts",
]

ts_project(
name = "tsconfig",
srcs = SRCS,
declaration = True,
declaration_map = True,
declarationdir = "out/types",
outdir = "out/code",
rootdir = "subdir",
source_map = True,
)

filegroup(
name = "types",
srcs = [":tsconfig"],
output_group = "types",
)

nodejs_test(
name = "test",
data = [
":tsconfig",
":types",
],
entry_point = "verify.js",
templated_args = [
"$(locations :types)",
"$(locations :tsconfig)",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const a: string = 'hello';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"sourceMap": true,
"declaration": true,
"declarationMap": true,
"types": []
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
const assert = require('assert');

const types_files = process.argv.slice(2, 4);
const code_files = process.argv.slice(4, 6);
assert.ok(
types_files.some(f => f.endsWith('declarationdir_with_value/out/types/a.d.ts')),
'Missing a.d.ts');
assert.ok(
types_files.some(f => f.endsWith('declarationdir_with_value/out/types/a.d.ts.map')),
'Missing a.d.ts.map');
assert.ok(
code_files.some(f => f.endsWith('declarationdir_with_value/out/code/a.js')), 'Missing a.js');
assert.ok(
code_files.some(f => f.endsWith('declarationdir_with_value/out/code/a.js.map')),
'Missing a.js.map');