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: validate tsconfig outDir vs ts_project(out_dir) #730

Merged
merged 23 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c60761a
Add a failing test for tsconfig exclude validation
Mivr Nov 1, 2024
1d71c3f
Remove analysistime test as we are expecting a build time failure
Mivr Nov 4, 2024
eb00478
Add tsconfig validation for exclude option while using out_dir
Mivr Nov 4, 2024
1f9748d
Fix validator and examples
Mivr Nov 4, 2024
8a7e569
Remove TODO as validation is now added
Mivr Nov 4, 2024
4613d4e
Merge branch 'aspect-build:main' into main
Mivr Dec 3, 2024
44a0940
Revert not needed excludes in tests
Mivr Dec 3, 2024
afc1dad
Add check for declaration dir and root dir with exclude usage
Mivr Dec 3, 2024
541c5c1
Merge branch 'aspect-build:main' into main
Mivr Dec 9, 2024
a5c9575
MR comments
Mivr Dec 9, 2024
337fbff
Add out_dir attribute validation
Mivr Dec 9, 2024
17297a5
Add a bats test for validation
Mivr Dec 16, 2024
bd07d75
Merge branch 'aspect-build:main' into main
Mivr Dec 22, 2024
3226a2f
Revert "Add out_dir attribute validation"
Mivr Jan 6, 2025
302bebb
Revert "Add a bats test for validation"
Mivr Jan 6, 2025
6794228
Revert not needed changes, add only out_dir parameter validation
Mivr Jan 6, 2025
74e52e7
Revert not needed exclude
Mivr Jan 6, 2025
d673bdb
Revert comment removed
Mivr Jan 6, 2025
3731be7
Make the check to only trigger when build will fail anyway
Mivr Jan 6, 2025
5bbadc2
Clean up the logic
Mivr Jan 6, 2025
a0a43f2
Merge branch 'aspect-build:main' into main
Mivr Jan 6, 2025
7bde70e
Use a new function to check the out_dir attr to avoid too complex logic
Mivr Jan 6, 2025
dd338bf
Revert not needed changes
Mivr Jan 6, 2025
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
6 changes: 4 additions & 2 deletions examples/declaration_dir/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
"compilerOptions": {
"sourceMap": true,
"declaration": true,
"declarationMap": true
}
"declarationMap": true,
"declarationDir": "out/types"
},
"exclude": []
jbedard marked this conversation as resolved.
Show resolved Hide resolved
}
1 change: 0 additions & 1 deletion examples/out_dir/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ ts_project(
declaration_dir = "decl_map",
declaration_map = True,
out_dir = "declaration_dir",
tsconfig = {},
jbedard marked this conversation as resolved.
Show resolved Hide resolved
)

build_test(
Expand Down
7 changes: 7 additions & 0 deletions examples/out_dir/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"compilerOptions": {
"declaration": true,
"declarationMap": true,
"declarationDir": "decl_map"
}
}
1 change: 1 addition & 0 deletions examples/root_dir/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ ts_project(
"compilerOptions": {
"rootDir": "subdir",
},
"exclude": [],
},
)

Expand Down
4 changes: 3 additions & 1 deletion examples/root_dir/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
{}
{
"exclude": []
}
3 changes: 0 additions & 3 deletions ts/private/ts_project.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
13 changes: 13 additions & 0 deletions ts/private/ts_project_options_validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,15 @@ function main(_a) {
}
}
}

function check_exclude_and_rootDir() {
if (config.exclude === undefined && attrs['root_dir'] !== undefined && attrs['root_dir'] !== '') {
console.error('tsconfig validation failed: when root_dir is set, exclude must also be set. See: https://github.com/aspect-build/rules_ts/issues/644 for more details.')
jbedard marked this conversation as resolved.
Show resolved Hide resolved

throw new Error('tsconfig validation failed: when root_dir is set, exclude must also be set.')
}
}

if (options.preserveSymlinks) {
console.error(
'ERROR: ts_project rule ' +
Expand All @@ -163,8 +172,10 @@ function main(_a) {
check('declaration')
check('incremental')
check('tsBuildInfoFile', 'ts_build_info_file')
check('declarationDir', 'declaration_dir')
check_nocheck()
check_preserve_jsx()
check_exclude_and_rootDir()
if (failures.length > 0) {
console.error(
'ERROR: ts_project rule ' +
Expand Down Expand Up @@ -201,6 +212,8 @@ function main(_a) {
attrs.declaration +
'\n// declaration_map: ' +
attrs.declaration_map +
'\n// declaration_dir: ' +
attrs.declaration_dir +
'\n// incremental: ' +
attrs.incremental +
'\n// source_map: ' +
jbedard marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 2 additions & 0 deletions ts/private/ts_validate_options.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def _validate_action(ctx, tsconfig_inputs):
config = struct(
allow_js = ctx.attr.allow_js,
declaration = ctx.attr.declaration,
declaration_dir = ctx.attr.declaration_dir,
declaration_map = ctx.attr.declaration_map,
root_dir = ctx.attr.root_dir,
preserve_jsx = ctx.attr.preserve_jsx,
composite = ctx.attr.composite,
no_emit = ctx.attr.no_emit,
Expand Down
1 change: 1 addition & 0 deletions ts/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ ts_project(
"declaration": True,
"sourceMap": True,
},
"exclude": [],
},
)

Expand Down
Loading