Skip to content

Conversation

@bbenshalom
Copy link
Contributor

@bbenshalom bbenshalom commented Jun 26, 2025

Related GitHub Issue

Closes: #5133

Description

Fixes TypeScript compiler watch mode reporting incorrect file paths without the src/ prefix, which prevented VS Code from properly navigating to error locations.

Before:

  • Error paths: core/webview/webviewMessageHandler.ts:439:7
  • VS Code navigation: ❌ Broken

After:

  • Error paths: src/core/webview/webviewMessageHandler.ts:439:7
  • VS Code navigation: ✅ Working

Changes Made

  • Removed rootDir: "." from src/tsconfig.json
    • The watch script already runs from project root (cd .. && tsc --noEmit --watch --project src/tsconfig.json)
    • Removing explicit rootDir allows TypeScript to use default behavior and include src/ in error paths

Test Procedure

  • Manually produced a forced compile error before and after the change
  • All lint checks and automatic tests pass successfully

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

Additional Notes

Get in Touch

Discord username is gooner4ever


Important

Fixes TypeScript compiler watch mode path inconsistency by removing rootDir from tsconfig.json, enabling correct VS Code navigation.

  • Behavior:
    • Fixes TypeScript compiler watch mode path inconsistency by removing rootDir: "." from src/tsconfig.json.
    • Error paths now include src/ prefix, enabling correct VS Code navigation.
  • Scripts:
    • Updates watch:tsc script in package.json to run from project root (cd .. && tsc --noEmit --watch --project src/tsconfig.json).

This description was created by Ellipsis for 994c206. You can customize this summary. It will automatically update as commits are pushed.

@bbenshalom bbenshalom requested review from cte, jr and mrubens as code owners June 26, 2025 22:14
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 26, 2025
@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jun 26, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 27, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 27, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've left a few questions and suggestions in the code for your consideration. Let me know what you think!

"noImplicitOverride": true,
"noImplicitReturns": true,
"noUnusedLocals": false,
"resolveJsonModule": true,
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious about the implications of removing rootDir. Could this affect TypeScript's module resolution or output structure in other build scenarios beyond the watch mode?

For instance, would this change how TypeScript resolves relative imports or affect the structure of declaration files if they were to be generated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see how it would affect ts module resolution. There are two tsc commands that use this tsconfig.json, and they're both without output because of --noEmit. The actual compilation is done by esbuild. So our change really only affects the error reporting.

(Plus, I think it's a better practice and the original rootDir property isn't beneficial in any way)

"publish:marketplace": "vsce publish --no-dependencies && ovsx publish --no-dependencies",
"watch:bundle": "pnpm bundle --watch",
"watch:tsc": "tsc --noEmit --watch --project tsconfig.json",
"watch:tsc": "cd .. && tsc --noEmit --watch --project src/tsconfig.json",
Copy link
Member

Choose a reason for hiding this comment

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

Is changing the working directory the most maintainable approach here? I'm wondering if there might be alternative solutions that could achieve the same result without requiring the script to cd ...

For example:

  • Could we use TypeScript's --rootDir CLI flag instead?
  • Would adjusting the include paths in tsconfig.json work?
  • Could we leverage the baseUrl compiler option?

The current approach works, but I'm curious if you considered these alternatives and what led to choosing this specific solution?

Copy link
Contributor Author

@bbenshalom bbenshalom Jun 27, 2025

Choose a reason for hiding this comment

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

This is actually the only change I managed to come up with that can be minimal in the command itself.
Changing the include paths or baseUrl doesn't affect error reporting. The --rootDir only works with an absolute path (which isn't viable).

Only other alternative I could think of was to write a script instead of a one liner, which is even uglier.

@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Changes Requested] in Roo Code Roadmap Jun 27, 2025
@bbenshalom bbenshalom requested a review from daniel-lxs June 27, 2025 14:18
@bbenshalom
Copy link
Contributor Author

Hi @daniel-lxs @hannesrudolph , I've addressed the concerns in the comments, please let me know if it makes sense to you. Thanks.

@daniel-lxs daniel-lxs moved this from PR [Changes Requested] to PR [Needs Prelim Review] in Roo Code Roadmap Jul 7, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

Tested main and this PR with a compiler error and it seems to work, I didn't notice any negative side effects.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 8, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jul 8, 2025
@mrubens mrubens merged commit edce187 into RooCodeInc:main Jul 8, 2025
32 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 8, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 8, 2025
@bbenshalom bbenshalom deleted the fix/5133-ts-compiler-watch-path branch July 8, 2025 15:22
@KJ7LNW
Copy link
Contributor

KJ7LNW commented Jul 8, 2025

yay! this has been driving me nuts!

@KJ7LNW
Copy link
Contributor

KJ7LNW commented Jul 8, 2025

I am not sure this completely fixed the issue, there is still some noise in the file name:

/home/ewheeler/src/roo/roo-main/roo-cline:watch:tsc: src/core/webview/__tests__/ClineProvider.spec.ts

The text :watch:tsc: should be /. This causes the following problem when you click the item:

image

notice the bottom two lines of this image.

  • the first line is incorrect
  • the second line is correct, and I can click on the second one but not the first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer PR - Needs Preliminary Review size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

fix: TypeScript compiler watch path inconsistency

5 participants