Skip to content

fix: emit source dts files in ts_project output - #984

Open
jbedard wants to merge 1 commit into
aspect-build:3.xfrom
jbedard:dts-passthru
Open

fix: emit source dts files in ts_project output#984
jbedard wants to merge 1 commit into
aspect-build:3.xfrom
jbedard:dts-passthru

Conversation

@jbedard

@jbedard jbedard commented Aug 28, 2026

Copy link
Copy Markdown
Member

Changes are visible to end-users: yes

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

Source ts_project(srcs) dts files are now passed through to the resulting JsInfo as-is with no modifications or outDir/rootDir applied just like tsc does not emit them while emitted dts may still depend on them.

Test plan

  • Covered by existing test cases
  • New test cases added

@jbedard
jbedard requested a review from acozzette August 28, 2026 20:59
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@jbedard
jbedard marked this pull request as draft August 28, 2026 21:34
jbedard

This comment was marked as resolved.

@jbedard
jbedard marked this pull request as ready for review August 28, 2026 21:49
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@jbedard

jbedard commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Summary:

Consequences match tsc too:

  • dist/index.d.ts re-exporting ./types dangles — identical to tsc (verified with both compilers earlier).
  • A consumer importing the .d.ts at its source path works — identical to tsc.
  • npm_package ships the .d.ts at its source path — same as npm publish with files covering it.

@jbedard

jbedard commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

@acozzette WDYT of this? Technically I think this is "more aligned with tsc", but it sure is odd 🤷

@jbedard

jbedard commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

Where things like .js when allowJson or .json when resolveJsonModule do have rootDir+outDir applied and get fully copied/outputted to bin...

@jbedard

jbedard commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

FWIW I'm trying to make oxc_transpile do similar to what rules_ts/tsc does: aspect-build/rules_oxc#48

@acozzette

Copy link
Copy Markdown
Contributor

I feel like I'm not familiar enough with the situation to have a very informed opinion. But if the choice is between copying the file and not copying it, doing less for now seems like a good call.

@jbedard

jbedard commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

tsc does not copy it, so I think we should do the same. However outside bazel that original .d.ts in your source is still available - which is where this PR comes in, should the original .d.ts source file be put into the JsInfo.types? That's what this PR adds...

@acozzette

Copy link
Copy Markdown
Contributor

I think that makes sense. If we're already doing it in the common case where out_dir matches root_dir then it seems reasonable to do that when they're not equal as well.

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.

2 participants