Skip to content

Commit c18c1c2

Browse files
a-tarasyuka-tarasyuk
authored
fix(57302): Cannot move symbols from tsx to ts files (#57305)
1 parent c4de2af commit c18c1c2

6 files changed

Lines changed: 589 additions & 17 deletions

File tree

‎src/services/refactors/moveToFile.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -893,12 +893,10 @@ export interface TopLevelVariableDeclaration extends VariableDeclaration {
893893
export type TopLevelDeclaration = NonVariableTopLevelDeclaration | TopLevelVariableDeclaration | BindingElement;
894894

895895
/** @internal */
896-
export function createNewFileName(oldFile: SourceFile, program: Program, context: RefactorContext, host: LanguageServiceHost): string {
896+
export function createNewFileName(oldFile: SourceFile, program: Program, host: LanguageServiceHost, toMove: ToMove | undefined): string {
897897
const checker = program.getTypeChecker();
898-
const toMove = getStatementsToMove(context);
899-
let usage;
900898
if (toMove) {
901-
usage = getUsageInfo(oldFile, toMove.all, checker);
899+
const usage = getUsageInfo(oldFile, toMove.all, checker);
902900
const currentDirectory = getDirectoryPath(oldFile.fileName);
903901
const extension = extensionFromPath(oldFile.fileName);
904902
const newFileName = combinePaths(
@@ -974,6 +972,11 @@ export function getStatementsToMove(context: RefactorContext): ToMove | undefine
974972
return all.length === 0 ? undefined : { all, ranges };
975973
}
976974

975+
/** @internal */
976+
export function containsJsx(statements: readonly Statement[] | undefined) {
977+
return find(statements, statement => !!(statement.transformFlags & TransformFlags.ContainsJsx));
978+
}
979+
977980
function isAllowedStatementToMove(statement: Statement): boolean {
978981
// Filters imports and prologue directives out of the range of statements to move.
979982
// Imports will be copied to the new file anyway, and may still be needed in the old file.
@@ -1000,8 +1003,7 @@ export function getUsageInfo(oldFile: SourceFile, toMove: readonly Statement[],
10001003
const oldImportsNeededByTargetFile = new Map<Symbol, /*isValidTypeOnlyUseSite*/ boolean>();
10011004
const targetFileImportsFromOldFile = new Set<Symbol>();
10021005

1003-
const containsJsx = find(toMove, statement => !!(statement.transformFlags & TransformFlags.ContainsJsx));
1004-
const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx);
1006+
const jsxNamespaceSymbol = getJsxNamespaceSymbol(containsJsx(toMove));
10051007

10061008
if (jsxNamespaceSymbol) { // Might not exist (e.g. in non-compiling code)
10071009
oldImportsNeededByTargetFile.set(jsxNamespaceSymbol, false);

‎src/services/refactors/moveToNewFile.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
nodeSeenTracker,
1919
Program,
2020
QuotePreference,
21-
RefactorContext,
2221
RefactorEditInfo,
2322
SourceFile,
2423
Symbol,
@@ -75,16 +74,15 @@ registerRefactor(refactorName, {
7574
getEditsForAction: function getRefactorEditsToMoveToNewFile(context, actionName): RefactorEditInfo {
7675
Debug.assert(actionName === refactorName, "Wrong refactor invoked");
7776
const statements = Debug.checkDefined(getStatementsToMove(context));
78-
const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context.preferences, context));
77+
const edits = textChanges.ChangeTracker.with(context, t => doChange(context.file, context.program, statements, t, context.host, context.preferences));
7978
return { edits, renameFilename: undefined, renameLocation: undefined };
8079
},
8180
});
8281

83-
function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences, context: RefactorContext): void {
82+
function doChange(oldFile: SourceFile, program: Program, toMove: ToMove, changes: textChanges.ChangeTracker, host: LanguageServiceHost, preferences: UserPreferences): void {
8483
const checker = program.getTypeChecker();
8584
const usage = getUsageInfo(oldFile, toMove.all, checker);
86-
87-
const newFilename = createNewFileName(oldFile, program, context, host);
85+
const newFilename = createNewFileName(oldFile, program, host, toMove);
8886

8987
// If previous file was global, this is easy.
9088
changes.createNewFile(oldFile, newFilename, getNewStatementsAndRemoveFromOldFile(oldFile, usage, changes, toMove, program, host, newFilename, preferences));

‎src/services/services.ts‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,9 @@ import {
323323
import * as NavigateTo from "./_namespaces/ts.NavigateTo";
324324
import * as NavigationBar from "./_namespaces/ts.NavigationBar";
325325
import {
326+
containsJsx,
326327
createNewFileName,
328+
getStatementsToMove,
327329
} from "./_namespaces/ts.refactor";
328330
import * as classifier from "./classifier";
329331
import * as classifier2020 from "./classifier2020";
@@ -3030,13 +3032,20 @@ export function createLanguageService(
30303032
const sourceFile = getValidSourceFile(fileName);
30313033
const allFiles = Debug.checkDefined(program.getSourceFiles());
30323034
const extension = extensionFromPath(fileName);
3033-
const files = mapDefined(allFiles, file =>
3034-
!program?.isSourceFileFromExternalLibrary(sourceFile) &&
3035-
!(sourceFile === getValidSourceFile(file.fileName) || extension === Extension.Ts && extensionFromPath(file.fileName) === Extension.Dts || extension === Extension.Dts && startsWith(getBaseFileName(file.fileName), "lib.") && extensionFromPath(file.fileName) === Extension.Dts)
3036-
&& extension === extensionFromPath(file.fileName) ? file.fileName : undefined);
3035+
const toMove = getStatementsToMove(getRefactorContext(sourceFile, positionOrRange, preferences, emptyOptions));
3036+
const toMoveContainsJsx = containsJsx(toMove?.all);
3037+
3038+
const files = mapDefined(allFiles, file => {
3039+
const fileNameExtension = extensionFromPath(file.fileName);
3040+
const isValidSourceFile = !program?.isSourceFileFromExternalLibrary(sourceFile) && !(
3041+
sourceFile === getValidSourceFile(file.fileName) ||
3042+
extension === Extension.Ts && fileNameExtension === Extension.Dts ||
3043+
extension === Extension.Dts && startsWith(getBaseFileName(file.fileName), "lib.") && fileNameExtension === Extension.Dts
3044+
);
3045+
return isValidSourceFile && (extension === fileNameExtension || (extension === Extension.Tsx && fileNameExtension === Extension.Ts || extension === Extension.Jsx && fileNameExtension === Extension.Js) && !toMoveContainsJsx) ? file.fileName : undefined;
3046+
});
30373047

3038-
const newFileName = createNewFileName(sourceFile, program, getRefactorContext(sourceFile, positionOrRange, preferences, emptyOptions), host);
3039-
return { newFileName, files };
3048+
return { newFileName: createNewFileName(sourceFile, program, host, toMove), files };
30403049
}
30413050

30423051
function getEditsForRefactor(

‎src/testRunner/unittests/tsserver/getMoveToRefactoringFileSuggestions.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,61 @@ import { value1 } from "../node_modules/.cache/someFile.d.ts";`,
116116
});
117117
baselineTsserverLogs("getMoveToRefactoringFileSuggestions", "skips lib.d.ts files", session);
118118
});
119+
120+
it("should show ts files when moving non-tsx content from tsx file", () => {
121+
const file1: File = {
122+
path: "/bar.tsx",
123+
content: `export function bar() { }`,
124+
};
125+
const file2: File = {
126+
path: "/foo.ts",
127+
content: "export function foo() { }",
128+
};
129+
const tsconfig: File = {
130+
path: "/tsconfig.json",
131+
content: jsonToReadableText({
132+
jsx: "preserve",
133+
files: ["./foo.ts", "./bar.tsx"],
134+
}),
135+
};
136+
137+
const host = createServerHost([file1, file2, tsconfig]);
138+
const session = new TestSession(host);
139+
openFilesForSession([file1], session);
140+
141+
session.executeCommandSeq<ts.server.protocol.GetMoveToRefactoringFileSuggestionsRequest>({
142+
command: ts.server.protocol.CommandTypes.GetMoveToRefactoringFileSuggestions,
143+
arguments: { file: file1.path, line: 1, offset: 7 },
144+
});
145+
baselineTsserverLogs("getMoveToRefactoringFileSuggestions", "should show ts files when moving non-tsx content from tsx file", session);
146+
});
147+
148+
it("should show js files when moving non-jsx content from jsx file", () => {
149+
const file1: File = {
150+
path: "/bar.jsx",
151+
content: `export function bar() { }`,
152+
};
153+
const file2: File = {
154+
path: "/foo.js",
155+
content: "export function foo() { }",
156+
};
157+
const tsconfig: File = {
158+
path: "/tsconfig.json",
159+
content: jsonToReadableText({
160+
allowJS: true,
161+
jsx: "preserve",
162+
files: ["./foo.js", "./bar.jsx"],
163+
}),
164+
};
165+
166+
const host = createServerHost([file1, file2, tsconfig]);
167+
const session = new TestSession(host);
168+
openFilesForSession([file1], session);
169+
170+
session.executeCommandSeq<ts.server.protocol.GetMoveToRefactoringFileSuggestionsRequest>({
171+
command: ts.server.protocol.CommandTypes.GetMoveToRefactoringFileSuggestions,
172+
arguments: { file: file1.path, line: 1, offset: 7 },
173+
});
174+
baselineTsserverLogs("getMoveToRefactoringFileSuggestions", "should show js files when moving non-jsx content from jsx file", session);
175+
});
119176
});

0 commit comments

Comments
 (0)