-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Skip adding imports at some locations #59719
base: main
Are you sure you want to change the base?
Conversation
…kipImportAtSomeLocations
src/services/pasteEdits.ts
Outdated
let newText = targetFile.text; | ||
for (let i = pasteLocations.length - 1; i >= 0; i--) { | ||
const { pos, end } = pasteLocations[i]; | ||
newText = actualPastedText ? newText.slice(0, pos) + actualPastedText + newText.slice(end) : newText.slice(0, pos) + pastedText[i] + newText.slice(end); | ||
} | ||
|
||
let importAdder: codefix.ImportAdder; | ||
const symbolsUsageCopiedFrom = new Map<string, [Symbol, boolean, codefix.ImportOrRequireAliasDeclaration | undefined]>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use a (preferably named) object type instead of this tuple?
addExportsInOldFile(copiedFrom.file, usage.targetFileImportsFromOldFile, changes, useEsModuleSyntax); | ||
addTargetFileImports(copiedFrom.file, usage.oldImportsNeededByTargetFile, usage.targetFileImportsFromOldFile, originalProgram.getTypeChecker(), updatedProgram, importAdder); | ||
// Map created with `symbol.name` as the key to retrieve symbols declared in the old file that need to be resolved in the target file (only the one's that were exported). | ||
for (const [symbol, [isValidTypeOnlyUseSite, declaration]] of usage.oldImportsNeededByTargetFile) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should oldImportsNeededByTargetFile
just be a bunch of SymbolInfo
s as well? If you did that, you could just copy all the same objects over.
copiedFrom?: { file: SourceFile; range: TextRange[]; } | undefined, | ||
symbolsUsageCopiedFrom?: Map<string, SymbolInfo>, | ||
) { | ||
if (copiedFrom) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You technically only need to pass in symbolsUsageCopiedFrom
. That avoids these two parameters from drifting apart in any intended meaning.
This pr fixes #59464. If the file the text was copied from is known, then pasteEdits adds imports only for symbols that were exported from that file or for imports that need to be copied over to the target file.