Suggestion: refactor to "move to another (existing) file"

[OliverJAsh]

Search Terms

Suggestion

The "move to new file" refactor is really helpful, but what if you want to move the code to an existing module?

This happens very often, when you realise code has been living in the wrong module, or it has a better home in some other module.

I would like to suggest a new refactor: "move to another (existing) file".

My workaround at the moment is to move the code manually, and then manually update all imports, but this is rather tedious!

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

[jillesvangurp]

Yes please. This is slowing me down trying to refactor an existing code base currently. Having to manually copy paste + edit imports means I don't do this when I should be.

[rudi-c]

Use case: I work in a large Typescript codebase that does not enforce (e.g. via lint rules) anything about creating cyclic dependencies. We'd like to, but first we need to break the existing cycles! But over the years, there's one dependency cycle that has grown to include 500 files. Half of the work to break a cycle is really just moving definitions between files...so this feature would be super helpful!

I'm sure this is harder than to move to a new file since now you'd need to check for name clashes (both the thing being moved and the imports that get moved), merge imports when they already exist on the destination file, etc.

This looks kind of fun so I might take a look at it...some guidance would be appreciated though! Recommendations on approaches would be helpful. I'm sure there's a lot of other issues I didn't think of.

@andy-ms

[jasonwilliams]

It would be nice to have this feature!
It should be similar to #23726 im guessing if someone wanted to try implementing this they could look at that PR

@RyanCavanaugh did you still need more feedback for this?

[rubenlg]

Another use case:
I'm mostly OK with "Move to a new file", but sometimes I move a bunch of symbols together and after saving and making further changes, I realize I forgot to move one symbol into that file, and have to do that manually instead.

[yhslai]

+1

IMO "Move to an existing file" is much more useful than merely creating a new file.

[jasonwilliams]

Paging @andrewbranch is this difficult to implement?

[orta]

I'm not sure of the internals, but this would probably be tricky - to my knowledge there are no refactor/fixits which require users pick a file or provide some sort of input in order for the fixit to run. You might have to make changes to the TS types and have editors accept the new format ahead of getting this in.

[OliverJAsh]

I find myself needing this far less often now, ever since I started using this VS Code extension: "Copy With Imports" https://marketplace.visualstudio.com/items?itemName=stringham.copy-with-imports

[laurent22]

Simple technique to go around this issue:

Let's say you want to move the interface AppState from the file app.ts to app/types.ts.

• Choose Refactor > Move to a New File for AppState - that will move the interface to AppState.ts
• Rename the newly created file to a unique name, such as AppStateUNIQUE.ts - this will update all references to this name
• Do a global search on /AppStateUNIQUE and replace it with /app/types
• Now simply copy and paste the AppState interface to app/types.ts and delete AppStateUNIQUE.ts

This is relatively convoluted to get there but still way faster than manually updating everything because you still take advantage of VSCode automatically updating the imports.

The step to rename to a unique name is only needed if you are moving a symbol with a relatively common name, say just "State". In that case, there will other symbols with this name and it's not safe to do a global search and replace.

[jasonwilliams]

I'm not sure of the internals, but this would probably be tricky - to my knowledge there are no refactor/fixits which require users pick a file or provide some sort of input in order for the fixit to run. You might have to make changes to the TS types and have editors accept the new format ahead of getting this in.

This is a valid point, I think it would be worth finding out if fixits can accept user inputs. Or if this could be a command instead?

@laurent22 that solution is very convoluted is probably more easier/faster to use the extension mentioned above to just copy and paste then remap everything that fails.

[laurent22]

The extension just copy the imports to the destination file but it doesn't update references, so not sure how that solves the issue.

[jeremyisatrecharm]

This would be a very useful feature! As codebases grow, I find myself wanting to shift and move stuff around quite a bit.

[uglycoyote]

On a related note, I find it a bit annoying that the existing "Move to New File" refactoring guesses the name for you and doesn't pop up a dialog asking you. It uses the name of the class or function as its guess, which I think is reasonable, or as good as it can possibly do, but 9 times out of 10 I'd rather name it something else.

So I think rather than creating a completely new refactoring option, I'd suggest renaming the existing one to "Move to Another File" and have it perform both functions.

The way I envision it, it would pop up a dialog asking what file to move it to which would:

• default to a new file with the name that it guesses,
• allow you to type a new name
• Show auto-completion suggestions of existing filenames
• Create a new file if you press enter with a new filename
• Move to an existing file if you press enter with an existing filename

[asci]

I use abracadabra extension that has option to move to existing files, it works fine, but sometimes it's not possible to invoke it (I did not investigated why tho)

[abulka]

With the abracadabra extension, don't highlight the entire function - instead position the cursor on the function name, then click on the yellow lightbulb.

[sam-s4s]

I, too, would love this feature. I was a bit surprised/perplexed when I went to go do this action, and all that was in the Refactor menu was "Move to new file", and I couldn't find a way to do the more common task of "Move to existing file".

(basically, you have a function, class, interface, etc. in one file, and you want it to be somewhere else :D)

Happy to discuss possible implementation concepts, such as a path box or browse dialog, etc.

I now have to go and move 17 functions by hand, and then somehow fix up their many import usages throughout my project. It could take a while :(
I might have to install abracadabra for this, but it does feel like the kind of useful feature that could be built in.

-EDIT-

lol yeah that extension did not work. First time trying to use it, got an error, will have to post an issue on their github :(
Maybe it doesn't work with Typescript properly or something... Anyway, a built-in feature would solve this problem in a much nicer way. Off I go to move them all by hand :'( There goes an hour...

"😅 I'm sorry, something went wrong: I can't build the AST from the source code. This may be due to a syntax error that you can fix. Here's what went wrong: Unexpected token (309:39)"

[mjbvz]

Here's what the VS Code <-> TS Server flow could look like here. I've marked protocol changes/additions with 🚧:

1. VS Code requests refactorings with getApplicableRefactors

2. If the cursors is on a relevant symbol, TS Server returns a Move to file refactoring.

   🚧 Here VS Code needs some way to tell that the returned refactoring is a move to file refactoring. This is needed in step 4 so that we know to handle this refactoring differently than other refactorings

3. The refactorings are shown to the user and the user selects move to file from the list of refactorings

4. Instead of calling getEditsForRefactor, VS Code now shows the user UI to select which file the symbol should be moved to. We will use a quick pick for this and need to list of files from TS

5. 🚧 VS Code makes a request to get potential file targets for the move refactor. This should return files in the current project that the selected symbol can be moved to. In addition, TS should return a proposed file name if a new file were to be created.

6. VS Code now will shows this file list in a quick pick. Additionally, we should allow the user to use the file dialog to select another file and an option to create a new file

7. 🚧 After the user has selected the target file, VS Code sends a new getEditsForMoveToFileRefactor request to TS Server.

   This request should include:

   • The same fields as GetEditsForRefactorRequestArgs
   • The new file name

8. TS server computes the edit and returns it to VS Code to be applied

---

During our sync today, we also noted that move to existing file is similar to copy/paste the brings along imports (microsoft/vscode#30066). In fact, this refactoring would probably be a good starting point for copy/paste+imports because:

• A refactoring has to be explicitly triggered by users. This makes it more obvious what happens if something goes wrong.
• With this refactoring, we're moving entire functions/classes/types instead of selections of code
• However the logic of moving code between files should be very similar between the two

[DanielRosenwasser]

Quick thoughts:

• TSServer should produce the list of files
• TypeScript code should only be able to move to other TypeScript files
• JavaScript code should only be able to move to other JavaScript files
• We probably don't want to offer files in node_modules
• If the UI allows you to go between a file picker and creating a new file, the new file dialog should be pre-populatable with a directory and a file name.

Open Questions:

• How does this work when a file belongs to multiple projects?
• How does this work with an inferred project (which only includes open files)?

[jasonwilliams]

I agree with all those thoughts. I don’t see a use case for ever moving something into node_modules. 1 step further would be using a .gitignore/ignore list if supplied but I guess there’s been no precedent of TSServer reading from this file up until now. I’d be ok with just node_modules being ignored as a starting point.

Does TSServer already know the list of files or will this impact performance?

For me the most common case has been somewhere else within the same directory, so having the “pre-populated” picker be the directory you’re already in would be a great start. For the sake of performance can it lazily traverse from there if the user requires it?

If the UI allows you to go between a file picker and creating a new file, the new file dialog should be pre-populatable with a directory and a file name.

Do we know if any other language server offers this? I don’t know what the UI would look like for such a feature. A fuzzy search in VSCode would be useful over navigating in and out of directories for example

How does this work when a file belongs to multiple projects?

What’s the definition of a file belonging to multiple projects? Do you have an example?

How does this work with an inferred project (which only includes open files)?

I would expect this to “just work” like before. Is an explicit project required for this feature to work? I assume TS already makes many assumptions on inferred projects.

[DanielRosenwasser]

Do we know if any other language server offers this? I don’t know what the UI would look like for such a feature.

Basically what the Pylance team has prototyped is that "move to..." would be a picker that allows a list of files, along with a top-level option to use the native file picker. The native file picker might allow a pre-populated directory and file name.

What’s the definition of a file belonging to multiple projects? Do you have an example?

A file can be included by another file which belongs to a tsconfig.json

proj/
├── blah/
│   ├── tsconfig.json
│   └── a.ts
└── blah-tests/
    ├── tsconfig.json
    └── b.ts

If b.ts imports from ../blah/a, then a belongs to the projects in blah/ and blah-tests/.

Does TSServer already know the list of files or will this impact performance?
Is an explicit project required for this feature to work?

These two are interconnected. Projects have the full file list. Inferred projects only look at open files.

For unconfigured JavaScript projects, you'll often get an inferred project, which means this feature won't really work well if we rely on a fuzzy search of files pre-populated by TypeScript.

[mjbvz]

@DanielRosenwasser I've updated my proposal for using quick pick to select the target file. Here's some drafts of the relevant protocol additions:

1. Way to identify the move to file refactoring

   We can likely just use RefactorActionInfo.name with a name we standardize on

2. Way to get a candidate file list shown to user:

interface GetMoveToRefactoringFileSuggestionsRequest extends FileLocationOrRangeRequestArgs {
     // Pass along the same arguments that we first passed to `GetApplicableRefactorsRequest`
}

interface GetMoveToRefactoringFileSuggestionsResponse extends Response {
    body?: {
        /// Suggested name if a new file is created
        newFileName: string;

       /// List of file paths that the selected text can be moved to
       // TODO: should this be an object instead so that TS could customize how files are shown in the file picker UI?
       files: string[]; 
    };
}

3. Way to apply a 'move to' refactoring after the user has selected a file:

interface GetEditsForMoveToFileRefactorRequest extends FileLocationOrRangeRequestArgs { {
     // Pass along the same arguments that we first passed to `GetApplicableRefactorsRequest`

    /// target file path that the user selected (may also be new file)
    filePath: string;
}

interface GetEditsForRefactorResponse extends Response {
    body?: RefactorEditInfo; // TODO: maybe use a new type
}