Fix/prioritize local imports#62688
Open
Ronitsabhaya75 wants to merge 4 commits intomicrosoft:mainfrom
Open
Conversation
- Removed duplicate local vs external comparison that conflicted with compareLocalVsExternal - Fixed test expectations in importNameCodeFixOptionalImport1.ts to expect local imports first - Fixed formatting issues (trailing newline) This resolves the test worker crashes and makes all CI checks pass.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the TypeScript import fix logic to prioritize local/relative imports over external node_modules imports when auto-importing symbols. The change ensures that when the same symbol is available from both a local file and a node_modules package, the local import is suggested first.
- Added a new comparison function to prioritize local imports over external imports
- Updated the import comparison logic to check local vs external first before other heuristics
- Updated existing test and added new comprehensive test to verify the prioritization behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/services/codefixes/importFixes.ts | Added compareLocalVsExternal function and integrated it into compareModuleSpecifiers to prioritize local imports |
| tests/cases/fourslash/importNameCodeFixOptionalImport1.ts | Swapped the order of expected import fixes to reflect local imports being prioritized first |
| tests/cases/fourslash/importFixesPrioritizeLocal.ts | Added comprehensive test covering multiple scenarios where local imports should be prioritized over node_modules |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
typescript-bot
added
the
For Uncommitted Bug
Collaborator
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Local imports are not prioritized over node_modules imports when multiple import options exist for the same symbol.
The reason is that compareModuleSpecifiers was not considering module locality (local vs external) before comparing other factors like package.json filters and path complexity, so external packages would often appear first in the quick fix menu even when local project exports were more relevant.This fix adds compareLocalVsExternal() which runs as the first comparison in compareModuleSpecifiers(), ensuring imports with moduleSpecifierKind !== "node_modules" are prioritized over moduleSpecifierKind === "node_modules".
solves #62667
Before:
import { useTheme } from "@mui/material";
import { useTheme } from "../utils/store";
After
import { useTheme } from "../utils/store";
import { useTheme } from "@mui/material";