From c60761aec6348d664b4ff64a0a542b6f95acfec5 Mon Sep 17 00:00:00 2001 From: Mihail Vratchanski Date: Fri, 1 Nov 2024 10:59:14 +0200 Subject: [PATCH 1/5] Add a failing test for tsconfig exclude validation --- ts/test/ts_project_test.bzl | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/ts/test/ts_project_test.bzl b/ts/test/ts_project_test.bzl index b9fcb597..5abb9723 100644 --- a/ts/test/ts_project_test.bzl +++ b/ts/test/ts_project_test.bzl @@ -99,6 +99,15 @@ def _use_dir_test_impl(ctx): _use_dir_test = analysistest.make(_use_dir_test_impl) +def _validate_tsconfig_exclude_test_impl(ctx): + env = analysistest.begin(ctx) + + asserts.expect_failure(env, "tsconfig validation failed: when out_dir is set, exclude must also be set. See: https://github.com/aspect-build/rules_ts/issues/644 for more details.") + + return analysistest.end(env) + +_validate_tsconfig_exclude_test = analysistest.make(_validate_tsconfig_exclude_test_impl, expect_failure = True) + # Use a dict() defined at the top level so it gets analyzed as part of .bzl file loading # and is therefore immutable. See https://bazel.build/rules/language#mutability _TSCONFIG = { @@ -150,6 +159,21 @@ def ts_project_test_suite(name): target_under_test = "use_dir", ) + ts_project( + name = "validate_tsconfig_exclude", + srcs = [], + tsconfig = { + "compilerOptions": { + "outDir": ".", + }, + }, + tags = ["manual"], + ) + _validate_tsconfig_exclude_test( + name = "validate_tsconfig_exclude_test", + target_under_test = "validate_tsconfig_exclude", + ) + write_file( name = "wrapper_ts", out = "wrapper.ts", @@ -175,6 +199,7 @@ def ts_project_test_suite(name): ":dir_test", ":use_dir_test", ":wrapper_test", + ":validate_tsconfig_exclude_test", ], ) From 1d71c3fe122cc9f19f4e9200b88a0858cda397ac Mon Sep 17 00:00:00 2001 From: Mihail Vratchanski Date: Mon, 4 Nov 2024 17:43:21 +0200 Subject: [PATCH 2/5] Remove analysistime test as we are expecting a build time failure --- ts/test/ts_project_test.bzl | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/ts/test/ts_project_test.bzl b/ts/test/ts_project_test.bzl index 5abb9723..b9fcb597 100644 --- a/ts/test/ts_project_test.bzl +++ b/ts/test/ts_project_test.bzl @@ -99,15 +99,6 @@ def _use_dir_test_impl(ctx): _use_dir_test = analysistest.make(_use_dir_test_impl) -def _validate_tsconfig_exclude_test_impl(ctx): - env = analysistest.begin(ctx) - - asserts.expect_failure(env, "tsconfig validation failed: when out_dir is set, exclude must also be set. See: https://github.com/aspect-build/rules_ts/issues/644 for more details.") - - return analysistest.end(env) - -_validate_tsconfig_exclude_test = analysistest.make(_validate_tsconfig_exclude_test_impl, expect_failure = True) - # Use a dict() defined at the top level so it gets analyzed as part of .bzl file loading # and is therefore immutable. See https://bazel.build/rules/language#mutability _TSCONFIG = { @@ -159,21 +150,6 @@ def ts_project_test_suite(name): target_under_test = "use_dir", ) - ts_project( - name = "validate_tsconfig_exclude", - srcs = [], - tsconfig = { - "compilerOptions": { - "outDir": ".", - }, - }, - tags = ["manual"], - ) - _validate_tsconfig_exclude_test( - name = "validate_tsconfig_exclude_test", - target_under_test = "validate_tsconfig_exclude", - ) - write_file( name = "wrapper_ts", out = "wrapper.ts", @@ -199,7 +175,6 @@ def ts_project_test_suite(name): ":dir_test", ":use_dir_test", ":wrapper_test", - ":validate_tsconfig_exclude_test", ], ) From eb004788d5d4c1925cb2fcc42a7fd8a8b5494e6b Mon Sep 17 00:00:00 2001 From: Mihail Vratchanski Date: Mon, 4 Nov 2024 17:45:22 +0200 Subject: [PATCH 3/5] Add tsconfig validation for exclude option while using out_dir --- ts/private/ts_project_options_validator.js | 13 ++++++++++++- ts/private/ts_validate_options.bzl | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/ts/private/ts_project_options_validator.js b/ts/private/ts_project_options_validator.js index 016213b6..00d7199c 100755 --- a/ts/private/ts_project_options_validator.js +++ b/ts/private/ts_project_options_validator.js @@ -21,7 +21,8 @@ function main(_a) { output = _a[1], target = _a[2], packageDir = _a[3], - attrsStr = _a[4] + attrsStr = _a[4], + out_dir = _a[5] // The Bazel ts_project attributes were json-encoded // (on Windows the quotes seem to be quoted wrong, so replace backslash with quotes :shrug:) var attrs = JSON.parse(attrsStr.replace(/\\/g, '"')) @@ -142,6 +143,15 @@ function main(_a) { } } } + + function check_exclude_and_outDir() { + if (attrs.exclude === undefined && out_dir !== undefined && out_dir !== '.' && out_dir !== '') { + console.error('tsconfig validation failed: when out_dir is set, exclude must also be set. See: https://github.com/aspect-build/rules_ts/issues/644 for more details.') + + throw new Error('tsconfig validation failed: when out_dir is set, exclude must also be set.') + } + } + if (options.preserveSymlinks) { console.error( 'ERROR: ts_project rule ' + @@ -165,6 +175,7 @@ function main(_a) { check('tsBuildInfoFile', 'ts_build_info_file') check_nocheck() check_preserve_jsx() + check_exclude_and_outDir() if (failures.length > 0) { console.error( 'ERROR: ts_project rule ' + diff --git a/ts/private/ts_validate_options.bzl b/ts/private/ts_validate_options.bzl index ff5f2669..0d7c6f7b 100644 --- a/ts/private/ts_validate_options.bzl +++ b/ts/private/ts_validate_options.bzl @@ -44,6 +44,7 @@ def _validate_action(ctx, tsconfig_inputs): str(ctx.label), ctx.label.package, json.encode(config), + ctx.attr.out_dir, ]) ctx.actions.run( From 1f9748d065bc6c8c309b4fbe545e33190ce24046 Mon Sep 17 00:00:00 2001 From: Mihail Vratchanski Date: Mon, 4 Nov 2024 18:12:08 +0200 Subject: [PATCH 4/5] Fix validator and examples --- examples/assets/BUILD.bazel | 5 ++++- examples/declaration_dir/BUILD.bazel | 3 +++ examples/extends_chain/main/BUILD.bazel | 1 + examples/jsx/BUILD.bazel | 10 ++++++++++ examples/module_ext/BUILD.bazel | 6 ++++++ examples/no_emit/BUILD.bazel | 1 + examples/out_dir/BUILD.bazel | 9 +++++++-- examples/resolve_json_module/BUILD.bazel | 2 ++ examples/root_dir/BUILD.bazel | 3 +++ examples/root_dirs/BUILD.bazel | 5 ++++- examples/simple/BUILD.bazel | 3 +++ examples/srcs/BUILD.bazel | 9 +++++++++ examples/targets/tsconfig.json | 3 ++- examples/transpiler/BUILD.bazel | 12 ++++++++++++ ts/private/ts_project_options_validator.js | 2 +- 15 files changed, 68 insertions(+), 6 deletions(-) diff --git a/examples/assets/BUILD.bazel b/examples/assets/BUILD.bazel index 3bc54c70..db01b5ac 100644 --- a/examples/assets/BUILD.bazel +++ b/examples/assets/BUILD.bazel @@ -41,7 +41,10 @@ ts_project( ], extends = ":config", out_dir = "out", - tsconfig = {"compilerOptions": {"outDir": "out"}}, + tsconfig = { + "compilerOptions": {"outDir": "out"}, + "exclude": [], + }, ) filegroup( diff --git a/examples/declaration_dir/BUILD.bazel b/examples/declaration_dir/BUILD.bazel index 261d515f..1bcca036 100644 --- a/examples/declaration_dir/BUILD.bazel +++ b/examples/declaration_dir/BUILD.bazel @@ -13,6 +13,9 @@ ts_project( out_dir = "out/code", root_dir = "dir", source_map = True, + tsconfig = { + "exclude": [], + }, ) # Assert that the output locations we wrote to match expectations diff --git a/examples/extends_chain/main/BUILD.bazel b/examples/extends_chain/main/BUILD.bazel index 139c2355..c49e5bba 100644 --- a/examples/extends_chain/main/BUILD.bazel +++ b/examples/extends_chain/main/BUILD.bazel @@ -17,5 +17,6 @@ ts_project( "compilerOptions": { "declaration": False, }, + "exclude": [], }, ) diff --git a/examples/jsx/BUILD.bazel b/examples/jsx/BUILD.bazel index 45371f25..41ec84b8 100644 --- a/examples/jsx/BUILD.bazel +++ b/examples/jsx/BUILD.bazel @@ -14,6 +14,16 @@ ts_project( out_dir = "out", preserve_jsx = True, source_map = True, + tsconfig = { + "exclude": [], + "compilerOptions": { + "allowJs": True, + "sourceMap": True, + "declaration": True, + "declarationMap": True, + "jsx": "preserve", + }, + }, ) filegroup( diff --git a/examples/module_ext/BUILD.bazel b/examples/module_ext/BUILD.bazel index deab80d0..1426e6ee 100644 --- a/examples/module_ext/BUILD.bazel +++ b/examples/module_ext/BUILD.bazel @@ -22,6 +22,9 @@ ts_project( declaration_map = True, out_dir = "out", source_map = True, + tsconfig = { + "exclude": [], + }, ) # You can also produce different module syntaxt outputs from a .ts file. @@ -44,6 +47,9 @@ ts_project( # Write the output files to an extra nested directory out_dir = format, source_map = True, + tsconfig = { + "exclude": [], + }, ) for format in [ "commonjs", diff --git a/examples/no_emit/BUILD.bazel b/examples/no_emit/BUILD.bazel index 0b75e2b9..d4ae6398 100644 --- a/examples/no_emit/BUILD.bazel +++ b/examples/no_emit/BUILD.bazel @@ -39,6 +39,7 @@ ts_project( "allowJs": True, "noEmit": True, }, + "exclude": [], }, ) diff --git a/examples/out_dir/BUILD.bazel b/examples/out_dir/BUILD.bazel index 01e1c538..c267f70c 100644 --- a/examples/out_dir/BUILD.bazel +++ b/examples/out_dir/BUILD.bazel @@ -10,6 +10,7 @@ ts_project( "declaration": True, "declarationMap": True, }, + "exclude": [], }, ) @@ -19,7 +20,9 @@ ts_project( declaration = True, declaration_map = True, out_dir = "param", - tsconfig = {}, + tsconfig = { + "exclude": [], + }, ) ts_project( @@ -29,7 +32,9 @@ ts_project( declaration_dir = "decl_map", declaration_map = True, out_dir = "declaration_dir", - tsconfig = {}, + tsconfig = { + "exclude": [], + }, ) build_test( diff --git a/examples/resolve_json_module/BUILD.bazel b/examples/resolve_json_module/BUILD.bazel index 0151ccf3..86151671 100644 --- a/examples/resolve_json_module/BUILD.bazel +++ b/examples/resolve_json_module/BUILD.bazel @@ -25,6 +25,7 @@ ts_project( "outDir": "ts-dict-override", "resolveJsonModule": True, }, + "exclude": [], }, ) @@ -40,6 +41,7 @@ ts_project( "compilerOptions": { "outDir": "ts-dict-unspecified", }, + "exclude": [], }, ) diff --git a/examples/root_dir/BUILD.bazel b/examples/root_dir/BUILD.bazel index b417662e..4d7f5ef8 100644 --- a/examples/root_dir/BUILD.bazel +++ b/examples/root_dir/BUILD.bazel @@ -51,6 +51,9 @@ ts_project( name = "replace", out_dir = "otherdir", root_dir = "subdir", + tsconfig = { + "exclude": [], + }, ) assert_outputs( diff --git a/examples/root_dirs/BUILD.bazel b/examples/root_dirs/BUILD.bazel index 03542d1b..364c7537 100644 --- a/examples/root_dirs/BUILD.bazel +++ b/examples/root_dirs/BUILD.bazel @@ -12,7 +12,9 @@ ts_project( srcs = ["src/lib.ts"], declaration = True, out_dir = "dist", - tsconfig = {}, + tsconfig = { + "exclude": [], + }, ) ts_project( @@ -26,6 +28,7 @@ ts_project( "dist", ], }, + "exclude": [], }, deps = [":lib"], ) diff --git a/examples/simple/BUILD.bazel b/examples/simple/BUILD.bazel index cc96ab34..78973306 100644 --- a/examples/simple/BUILD.bazel +++ b/examples/simple/BUILD.bazel @@ -13,6 +13,9 @@ ts_project( # "." is the same as default # explicitly given as a regression test for https://github.com/aspect-build/rules_ts/issues/195 out_dir = ".", + tsconfig = { + "exclude": [], + }, # Note, the tsconfig attribute defaults to the tsconfig.json file in this directory. # tsconfig = "", deps = [ diff --git a/examples/srcs/BUILD.bazel b/examples/srcs/BUILD.bazel index a902f228..f4363637 100644 --- a/examples/srcs/BUILD.bazel +++ b/examples/srcs/BUILD.bazel @@ -10,6 +10,9 @@ load("@bazel_skylib//rules:write_file.bzl", "write_file") ts_project( name = "srcs-auto", out_dir = "auto", + tsconfig = { + "exclude": [], + }, ) # The sources can come from the default output of some other target. @@ -24,6 +27,9 @@ ts_project( name = "srcs-filegroup", srcs = [":srcs"], out_dir = "filegroup", + tsconfig = { + "exclude": [], + }, ) # Tools can also generate .ts source files, so the sources are actually in the bazel-out tree. @@ -45,6 +51,9 @@ ts_project( "generated.ts", ], out_dir = "generated", + tsconfig = { + "exclude": [], + }, ) # Testing what outputs are actually produced diff --git a/examples/targets/tsconfig.json b/examples/targets/tsconfig.json index 33e44fdd..c9805554 100644 --- a/examples/targets/tsconfig.json +++ b/examples/targets/tsconfig.json @@ -2,5 +2,6 @@ "compilerOptions": { "declaration": true, "sourceMap": true - } + }, + "exclude": [] } diff --git a/examples/transpiler/BUILD.bazel b/examples/transpiler/BUILD.bazel index 540f1b20..679951b6 100644 --- a/examples/transpiler/BUILD.bazel +++ b/examples/transpiler/BUILD.bazel @@ -47,6 +47,9 @@ ts_project( out_dir = "build-babel", source_map = True, transpiler = babel, + tsconfig = { + "exclude": [], + }, ) # Runs babel to transpile ts -> js @@ -76,6 +79,7 @@ ts_project( "compilerOptions": { "declaration": False, }, + "exclude": [], }, ) @@ -137,6 +141,9 @@ ts_project( tsc_js, out_dir = "build-custom_transpilers", ), + tsconfig = { + "exclude": [], + }, ) build_test( @@ -207,6 +214,9 @@ ts_project( out_dir = "build-custom_dts_transpiler", source_map = True, transpiler = "tsc", + tsconfig = { + "exclude": [], + }, ) build_test( @@ -236,6 +246,7 @@ ts_project( "compilerOptions": { "declaration": False, }, + "exclude": [], }, ) @@ -263,6 +274,7 @@ ts_project( "declaration": True, "emitDeclarationOnly": True, }, + "exclude": [], }, ) diff --git a/ts/private/ts_project_options_validator.js b/ts/private/ts_project_options_validator.js index 00d7199c..884efb99 100755 --- a/ts/private/ts_project_options_validator.js +++ b/ts/private/ts_project_options_validator.js @@ -145,7 +145,7 @@ function main(_a) { } function check_exclude_and_outDir() { - if (attrs.exclude === undefined && out_dir !== undefined && out_dir !== '.' && out_dir !== '') { + if (config.exclude === undefined && out_dir !== undefined && out_dir !== '.' && out_dir !== '') { console.error('tsconfig validation failed: when out_dir is set, exclude must also be set. See: https://github.com/aspect-build/rules_ts/issues/644 for more details.') throw new Error('tsconfig validation failed: when out_dir is set, exclude must also be set.') From 8a7e569769078dadddb363774eaa02c9bc5a64ca Mon Sep 17 00:00:00 2001 From: Mihail Vratchanski Date: Mon, 4 Nov 2024 19:34:33 +0200 Subject: [PATCH 5/5] Remove TODO as validation is now added --- ts/private/ts_project.bzl | 3 --- 1 file changed, 3 deletions(-) diff --git a/ts/private/ts_project.bzl b/ts/private/ts_project.bzl index 367bba07..e2cf38ab 100644 --- a/ts/private/ts_project.bzl +++ b/ts/private/ts_project.bzl @@ -126,9 +126,6 @@ See https://github.com/aspect-build/rules_ts/issues/361 for more details. common_args.extend(ctx.attr.args) if (ctx.attr.out_dir and ctx.attr.out_dir != ".") or ctx.attr.root_dir: - # TODO: add validation that excludes is non-empty in this case, as passing the --outDir or --declarationDir flag - # to TypeScript causes it to set a default for excludes such that it won't find our sources that were copied-to-bin. - # See https://github.com/microsoft/TypeScript/issues/59036 and https://github.com/aspect-build/rules_ts/issues/644 common_args.extend([ "--outDir", _lib.join(ctx.label.workspace_root, ctx.label.package, ctx.attr.out_dir),