Skip to content
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

source.organizeImports should not remove imports if file contains errors. #43051

Closed
oleg-slapdash opened this issue Mar 3, 2021 · 0 comments · Fixed by #43184
Closed

source.organizeImports should not remove imports if file contains errors. #43051

oleg-slapdash opened this issue Mar 3, 2021 · 0 comments · Fixed by #43184
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@oleg-slapdash
Copy link

Bug Report

When a user has source.organizeImports enabled on save/blur, the imports are mistakenly removed if the file contains errors. It also happens file contains an unclosed block comment /* (for example, when the file got saved in the middle of writing a comment).

🔎 Search Terms

  • Remove unused imports
  • source.organizeImports

🕗 Version & Regression Information

typescript@4.1.2

VSCode:

Version: 1.53.2
Commit: 622cb03f7e070a9670c94bae1a45d78d7181fbd4
Date: 2021-02-11T11:45:54.515Z (2 wks ago)
Electron: 11.2.1
Chrome: 87.0.4280.141
Node.js: 12.18.3
V8: 8.7.220.31-electron.0
OS: Darwin x64 19.6.0
  • This is a crash: No
  • This changed between versions ______ and _______: I do not recall this ever working
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
  • I was unable to test this on prior versions because do not have an environment setup.

⏯ Playground Link

  • I do not know how to trigger source.organizeImports in a playground.

💻 Code

An example of a broken file. This example contains JSX, hover, the point here if the file IS broken, then we should not remove imports.

File: test.ts (NOT test.tsx)

import { uniq } from "lodash";

export default function useAssetTypeFilterOptions() {
  return (
    <input
      name="aaaaaaaaaaaaaaaaaaaaaaaaaaaa"
      value={uniq(["a", "b", "c"]).join(" + ")}
    />
  );
}

2021-03-02 19 43 12

🙁 Actual behavior

  • The imports are erroneously removed

🙂 Expected behavior

  • The imports should not be removed if the source file contains errors.
@RyanCavanaugh RyanCavanaugh added Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript labels Mar 8, 2021
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Mar 8, 2021
@RyanCavanaugh RyanCavanaugh added the Help Wanted You can do this label Mar 8, 2021
rchl added a commit to typescript-language-server/typescript-language-server that referenced this issue Nov 8, 2021
This adds support for the new skipDestructiveCodeActions argument to
TypeScript's organize imports feature - [1] to support [2].

Support is added in two places:

 - Automatically inferring the proper value based on diagnostics for the
   file when returning code actions.
 - Supporting sending it when manually executing the organize imports action.

Also added documentation to the readme about the supported commands
that can be manually executed.

[1] microsoft/TypeScript#43051
[2] apexskier/nova-typescript#273

Co-authored-by: Rafal Chlodnicki <rchl2k@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
2 participants