Skip to content

Port tryLoadInputFileForPath from TypeScript codebase#1346

Closed
AlCalzone wants to merge 5 commits into
microsoft:mainfrom
AlCalzone:subpath-import-issue
Closed

Port tryLoadInputFileForPath from TypeScript codebase#1346
AlCalzone wants to merge 5 commits into
microsoft:mainfrom
AlCalzone:subpath-import-issue

Conversation

@AlCalzone
Copy link
Copy Markdown
Contributor

Fixes: #1345
and improves several baselines of existing tests
I don't fully grasp some of the error diffs, but the other changes at least look like an improvement to me.

Disclaimer: I burned a couple of tokens to get Claude to port this, since I don't understand enough of the codebase to make this change on my own - so please check that no nonsense slipped in.

Copilot AI review requested due to automatic review settings July 2, 2025 09:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ports the TypeScript tryLoadInputFileForPath logic into our Go resolver, adds two helpers for mapping output files back to source inputs, and updates a slew of compiler tests and baselines to reflect the new “self-name + outDir” behavior.

  • Implement tryLoadInputFileForPath support for package self-names with outDir/declarationDir, guessing common source directories and mapping outputs back to inputs
  • Add helpers getPossibleOriginalInputExtensionForExtension and getOutputDirectoriesForBaseDirectory
  • Introduce a new test (subpathImportJS.ts) and update many baseline files under testdata/baselines to match the new resolution behavior

Reviewed Changes

Copilot reviewed 61 out of 61 changed files in this pull request and generated 2 comments.

File Description
internal/module/resolver.go Ported tryLoadInputFileForPath logic from TS; added helpers for extension and directory guesses
testdata/tests/cases/compiler/subpathImportJS.ts New test exercising package import maps and self-paths
testdata/baselines/reference/... Updated expected outputs/errors/symbols/types to match new resolution results

Comment thread internal/module/resolver.go
Comment thread internal/module/resolver.go
@JoostK
Copy link
Copy Markdown

JoostK commented Jul 2, 2025

This is pending in #1302

@jakebailey
Copy link
Copy Markdown
Member

Yeah, I don't think we need this PR, given #1302 is the same? Just, a discussion about if we are keeping this logic or if there's some better thing we can do?

@jakebailey
Copy link
Copy Markdown
Member

Going to close this for clarity, but if you see something in my PR that your PR did better, absolutely tell me.

@jakebailey jakebailey closed this Jul 2, 2025
@AlCalzone
Copy link
Copy Markdown
Contributor Author

I trust that you have a clear idea what you want to achieve - I merely told Copilot to port the TypeScript implementation as close as possible.

@AlCalzone AlCalzone deleted the subpath-import-issue branch July 2, 2025 17:40
@jakebailey
Copy link
Copy Markdown
Member

I have to admit, I also did that (which succeeded).

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.

Subpath imports referencing compiled JS files not resolved

4 participants