Skip to content

Confirm TypeScript PR #60238 fix is already correctly implemented #1140

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

Closed
wants to merge 2 commits into from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 10, 2025

This PR investigates and confirms that the changes from TypeScript PR #60238 "Fix prioritization of paths specifiers over node_modules package specifiers" have already been correctly ported to the Go codebase.

Investigation Summary

TypeScript PR #60238 fixed an issue where paths specifiers from tsconfig.json were not being properly prioritized over node_modules package specifiers during module resolution. The key change was:

Before the fix:

if (!specifier) {
    const local = getLocalModuleSpecifier(
        // ... parameters
        /*pathsOnly*/ modulePath.isRedirect,
    );
    // ... logic was conditional
}

After the fix:

const local = getLocalModuleSpecifier(
    // ... parameters  
    /*pathsOnly*/ modulePath.isRedirect || !!specifier,
);
// ... logic always executes

Findings

Analysis of the Go codebase shows this fix was already correctly implemented in internal/modulespecifiers/specifiers.go:

  1. Lines 356-364: The call to getLocalModuleSpecifier is NOT conditional on !specifier (already moved outside any conditional block)
  2. Line 363: The pathsOnly parameter correctly uses modulePath.IsRedirect || len(specifier) > 0 (equivalent to the TypeScript fix)
  3. Lines 328-332: Module specifier priority comments match the intended behavior

Verification

  • ✅ All existing tests pass with no baseline changes
  • ✅ Added TestPathsPriorityOverNodeModules to document and verify the fix
  • ✅ The original TypeScript test case exists in _submodules/TypeScript/tests/cases/fourslash/autoImportPathsNodeModules.ts

Conclusion

No code changes were needed as the fix from TypeScript PR #60238 has already been correctly ported to the Go implementation. This PR adds a test to document that the fix is in place and closes the issue.

Fixes #1127.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…h test

Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Port TypeScript PR #60238: Fix prioritization of paths specifiers over node_modules package specifiers Confirm TypeScript PR #60238 fix is already correctly implemented Jun 10, 2025
@Copilot Copilot AI requested a review from andrewbranch June 10, 2025 00:55
Copilot finished work on behalf of andrewbranch June 10, 2025 00:55
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.

Port TypeScript PR #60238: Fix prioritization of paths specifiers over node_modules package specifiers
2 participants