Typescript: Import refactoring results in broken code

[oliversalzburg]

Issue Type: Bug

This is likely best reproduced in a larger project with existing references between files.

For me, it always goes like this:

1. Move a file within the directory structure of the project.
2. VSC will ask if I want to automatically update imports, I agree and my imports throughout the project are updated to the new path. So far, so good.
3. I rename the file that I just moved.
4. VSC will ask if I want to automatically update imports, I agree and now all the files that were adjusted in step 2 are broken.

I'm assuming this related to me having source.organizeImports enabled on save, but that's just a guess.

The way the code files break is always with some invalid syntax in the imports.

In this case I moved MikroORM.ts from the root of the project into the container directory. We can see the updated path in the imports.
Then I renamed the file to ProvideMikroORM and everything breaks.

While I can reproduce this almost with 100% success, I've had cases where everything went as expected. I felt like it's relevant which document is currently focused when the refactoring runs. Just a guess so far though.

I also want to note another possibly related issue I'm having, as it also relates to renaming files: https://github.com/microsoft/TypeScript/issues/40616

VS Code version: Code 1.49.1 (58bb7b2331731bf72587010e943852e13e6fd3cf, 2020-09-16T23:27:51.792Z)
OS version: Windows_NT x64 10.0.19041
Remote OS version: Linux x64 4.19.128-microsoft-standard
Remote OS version: Linux x64 4.19.128-microsoft-standard
Remote OS version: Linux x64 4.19.128-microsoft-standard

System Info

|Item|Value|
|---|---|
|CPUs|Intel(R) Core(TM) i7-6850K CPU @ 3.60GHz (12 x 3672)|
|GPU Status|2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
opengl: enabled_on
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled|
|Load (avg)|undefined|
|Memory (System)|63.91GB (45.62GB free)|
|Process Argv|--folder-uri=vscode-remote://wsl+Ubuntu/home/oliver/fairmanager/fairmanager/static2 --remote=wsl+Ubuntu --crash-reporter-id f825acfe-92fa-4526-a38a-f8815cf9f7ea|
|Screen Reader|no|
|VM|0%|

|Item|Value|
|---|---|
|Remote|WSL: Ubuntu|
|OS|Linux x64 4.19.128-microsoft-standard|
|CPUs|Intel(R) Core(TM) i7-6850K CPU @ 3.60GHz (12 x 3672)|
|Memory (System)|50.14GB (43.72GB free)|
|VM|0%|

|Item|Value|
|---|---|
|Remote|WSL: Ubuntu|
|OS|Linux x64 4.19.128-microsoft-standard|
|CPUs|Intel(R) Core(TM) i7-6850K CPU @ 3.60GHz (12 x 3672)|
|Memory (System)|50.14GB (43.72GB free)|
|VM|0%|

|Item|Value|
|---|---|
|Remote|WSL: Ubuntu|
|OS|Linux x64 4.19.128-microsoft-standard|
|CPUs|Intel(R) Core(TM) i7-6850K CPU @ 3.60GHz (12 x 3672)|
|Memory (System)|50.14GB (43.72GB free)|
|VM|0%|

Extensions (28)

Extension|Author (truncated)|Version
---|---|---
javascript-ejs-support|Dig|1.3.1
eslint-disable-snippets|drK|1.2.0
output-colorizer|IBM|0.1.2
vscode-docker|ms-|1.6.0
remote-containers|ms-|0.140.1
remote-ssh|ms-|0.55.0
remote-ssh-edit|ms-|0.55.0
remote-wsl|ms-|0.44.5
vscode-remote-extensionpack|ms-|0.20.0
vscode-icons|vsc|11.0.0
better-toml|bun|0.3.2
vscode-eslint|dba|2.1.8
eslint-disable-snippets|drK|1.2.0
gitlens|eam|10.2.2
EditorConfig|Edi|0.15.1
prettier-vscode|esb|5.6.0
vs-code-xml-format|fab|0.1.5
generateuuid|Fir|0.0.5
shell-format|fox|7.0.1
terraform|has|2.2.3
vscode-esformatter|how|1.6.0
code-beautifier|mic|2.3.3
vscode-docker|ms-|1.6.0
js-debug-nightly|ms-|2020.9.2117
vscode-paste-image|mus|1.0.4
vscode-xml|red|0.13.0
vscode-yaml|red|0.11.0
lit-plugin|run|1.2.1

(2 theme extensions excluded)

[mjbvz]

Please share an example project that demonstrates this issue

[oliversalzburg]

I can reproduce this in https://github.com/mikro-orm/mikro-orm

1. I clone the repo, yarn install in the root (ignoring failing node-gyp), and open in VS Code.
2. Create workspace settings:

{
  "[typescript]": {
    "editor.codeActionsOnSave": {
      "source.organizeImports": true
    }
  }
}

1. Move EntityManager.ts from packages/core/src to packages/core/src/entity. Accept refactoring. Many documents are opened with unsaved changes.
2. Rename EntityManager.ts to MyEntityManager.ts. Accept refactoring. Problems panel is now full of syntax error reports.

I was not able to reproduce this without the settings adjustment. Which makes me more confident that the problem is related to the source.organizeImports feature.

[mjbvz]

I just followed those steps but can't reproduce this issue.

Do you see this when all extensions are disabled?

Can you also clarify when you are saving these files (or if you have auto save enabled)

[oliversalzburg]

I can reproduce this with all extensions disabled outside of WSL.

I have "files.autoSave": "onFocusChange", configured and am performing no manual save operations.

My complete user settings are here: https://gist.github.com/oliversalzburg/501aff25e1e553e51e5cdc1b621e5abf

[andrewbranch]

Doesn't repro for me on macOS or Windows.

[oliversalzburg]

Hmmm, do you maybe have an idea for anything I can try to gather more useful information or which part of my local setup could be causing this?

I can reproduce this on Windows 10 and in WSL reliably on 2 different machines. I'm happy to accept that it's an error on my part due to faulty configuration or something like that, but I'm really out of ideas where to look for clues.

[andrewbranch]

@oliversalzburg could you record a log file while reproducing the issue?

[oliversalzburg]

Here it is: tsserver.log

[andrewbranch]

Hmm, I don’t actually see a request for updating paths after a file rename in that log? Could you try again on mikro-orm with exactly the steps you outlined earlier?

[oliversalzburg]

@andrewbranch I went through exactly those steps. I can do it more times and collect more samples if it helps.

[andrewbranch]

@oliversalzburg ack, my bad, I opened the wrong log file, didn’t realize I already had a file named tsserver.log in my downloads 🤦 The one you sent is good 👍

[andrewbranch]

@oliversalzburg part of what I don’t understand is that following those steps, with "files.autoSave": "onFocusChange" doesn’t trigger any saves, which means organizeImports is never triggered either.

Move EntityManager.ts from packages/core/src to packages/core/src/entity. Accept refactoring. Many documents are opened with unsaved changes.

We observe the same behavior here...

Rename EntityManager.ts to MyEntityManager.ts. Accept refactoring. Problems panel is now full of syntax error reports.

After I accept the refactoring, all those unsaved files are _still unsaved_, which should mean that organizeImports has not yet run. Do you see syntax errors even before those files have been saved?

[oliversalzburg]

When I rename the file, and the "Update imports for 'MyEntityManager.ts'" dialog appears, that's when the files are saved. I would assume the focus being switched over to the dialog is what causes the focus change and thus the save operation.

At that point in time, before confirming the dialog, I see some problems being indicated. I assume these are the imports that were broken by my renaming.

After confirming the dialog, the files will have the syntax errors.

I removed all of my user settings and only put the autoSave into the project settings and recorded my reproduction. Maybe I'm skipping some important detail in my description: https://www.youtube.com/watch?v=AskpAN6hK9Y
I also enabled ESLint in this reproduction for no particular reason, which causes unrelated problems to be shown in the problems panel. Sorry for the noise.

[andrewbranch]

Ah, ok, on macOS the confirmation dialog doesn’t trigger a focus change. I’ll try switching back to Windows again.

[andrewbranch]

Yep, it repros on Windows with the save on focus change 😮

[andrewbranch]

@mjbvz - the issue is that the editor is sending updateOpen calls out of order. The first file move in the repro steps executes correctly; its only purpose is to open a bunch of files with unsaved changes. So we’re in a consistent state prior to the rename (step 4), and there are a bunch of files open with unsaved changes. The sequence of events for the rename happens like this:

1. User initiates rename
2. VS Code requests getEditsForFileRename from TS Server; server responds with an edit for each of those other open files
3. VS Code issues system prompt asking user whether to accept the rename edits
4. The system prompt being focused causes all unsaved open files to be saved
5. The file save causes VS Code to request organizeImports edits from TS Server; server responds with edits
6. VS Code applies those edits in the background and sends corresponding updateOpen info to server
7. If the user accepts the rename edits in the system prompt, VS Code applies the changes that were calculated with respect to the file state prior to (6), which is no longer the current state

[oliversalzburg]

As I noted initially, I felt like #40616 is related to this and I even have another similar case that is even more destructive and requires even less interaction.

I open a file through the Problems panel and it will open with a pretty much completely replaced import section that overlays into the code section and appears to originate from a completely different file.

The file it seems to originate from, is the file I had opened before this file.

I am already so used to it, that I recognize it from the dimmed imports. I then use Undo, to revert to the previous, unbroken state.

I sadly have no reproduction steps for this one. It's rare, but when I do large refactor operations, it's pretty much bound to happen several times a day.

Sorry if this is unrelated to the original issue, but I didn't want to open another one of these mystery behavior tickets again when it seemes so related.

[oliversalzburg]

Just FYI, after removing "files.autoSave": "onFocusChange" from my settings, all of these issues are gone.