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

fix: add tsconfig root_dir/exclude validation #764

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

Mivr
Copy link
Contributor

@Mivr Mivr commented Jan 15, 2025

Close #644

--

Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: N/A
  • Breaking change (forces users to change their own code or config): no
  • Suggested release notes appear below: Yes

Normalize ts_project *_dir attribute paths.

Validate any use of root_dir includes the exclude: [] workaround for #644

Test plan

  • New test cases added

Before the change any usage of a declaration_dir that starts with "./" was leading to an error, now if the path starts with "./" it is simply removed and the behavior is as if it was not added at all.

Before the change:

declaration_dir tsconfig declarationDir Result Message
undefined PASS
. undefined PASS
./ undefined FAIL invalid target name './/foo.d.ts': target names may not contain '.' as a path segment
./dir undefined FAIL invalid target name './dir/foo.d.ts': target names may not contain '.' as a path segment
./dir ./dir FAIL invalid target name './dir/foo.d.ts': target names may not contain '.' as a path segment
out undefined FAIL not all outputs were created or valid
out out PASS
undefined out FAIL not all outputs were created or valid
out ./out PASS

After the change:

declaration_dir tsconfig declarationDir Result Message
undefined PASS
. undefined PASS
./ undefined PASS
./dir undefined FAIL not all outputs were created or valid
./dir ./dir PASS
dir undefined FAIL not all outputs were created or valid
dir dir PASS
undefined dir FAIL not all outputs were created or valid
dir ./dir PASS

Copy link

aspect-workflows bot commented Jan 15, 2025

Test

All tests were cache hits

160 tests (100.0%) were fully cached saving 15s.


Buildifier      Format

ts/defs.bzl Outdated Show resolved Hide resolved
@jbedard
Copy link
Member

jbedard commented Jan 16, 2025

What about out_dir, root_dir etc? I forget if those had similar issues?

@Mivr
Copy link
Contributor Author

Mivr commented Jan 16, 2025

What about out_dir, root_dir etc? I forget if those had similar issues?

They do have the same problems, do you prefer them in this PR or in an another one?

@Mivr
Copy link
Contributor Author

Mivr commented Jan 16, 2025

I moved the stripping deeper in the code, now it impacts root_dir, declaration_dir and out_dir. I will test more thoroughly and post the results.

@Mivr Mivr changed the title Sanitize declaration_dir for "./" at the start Sanitize declaration_dir, root_dir and out_dir for "./" at the start; Add verbose error when exclude is needed in tsconfig.json Jan 16, 2025
ts/defs.bzl Show resolved Hide resolved
ts/private/ts_lib.bzl Outdated Show resolved Hide resolved
Throw a better error when exclude needs to be added to tsconfig.json
@jbedard jbedard changed the title Sanitize declaration_dir, root_dir and out_dir for "./" at the start; Add verbose error when exclude is needed in tsconfig.json fix: add tsconfig root_dir/exclude validation Jan 16, 2025
@jbedard jbedard merged commit 5ac25c1 into aspect-build:main Jan 16, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: typescript 5.5.2 cannot find input files with source tsconfig.json
2 participants