Skip to content

Renaming an object property on destructuring (aliasing) also changes type definition. #44627

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

Closed
dalmo3 opened this issue Jun 17, 2021 · 6 comments
Labels
Needs More Info The issue still hasn't been fully clarified

Comments

@dalmo3
Copy link

dalmo3 commented Jun 17, 2021

Bug Report

I ran into this strange bug where renaming a property on destructuring will also rename the original property in the type definition.

This is a gif that shows what happens: I'm using VSCode's rename function on F2.

destruct-rename

The expected behaviour is that it should just create an alias for the property and update its usage down the file, without also changing the type. In fact, that's what usually happens.

This bug is somewhat random and I can't easily reproduce it. It was just the 4th or 5th time it happened when I decided to write it here. Apparently it doesn't go away by restarting the ts server, but it does by reloading VSCode.

🕗 Version & Regression Information

TS 4.3.2
VSCode insiders 1.57
WSL2

💻 Code

typescript.preferences.useAliasesForRenames: true

{
  "compilerOptions": {
    "target": "es5",
    "lib": ["dom", "dom.iterable", "esnext"],
    "allowJs": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "noFallthroughCasesInSwitch": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "isolatedModules": true,
    "noEmit": true,
    "jsx": "react-jsx",
    "baseUrl": "."
    // "noImplicitAny": false
  },
  "include": ["src"]
}

I understand that there is very little actionable information here. Let me know how can I collect more info about it next time it happens.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 17, 2021

We can poke at this but need code (per policy we do not transcribe screenshots)

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Jun 17, 2021
@dalmo3
Copy link
Author

dalmo3 commented Jun 17, 2021

Absolutely. I'll come back whenever I have some piece of code that reproduces this again. Just some more info for the record:

I just remembered another time this happened (on another codebase) when I renamed a div to a span and it went all the way to rename JSX definitions in node_modules. Very similar to #7458 except it only renamed the scoped div (as expected) and the type definition (totally unexpected).

Also related: #25273, #8227, #42367

It seems different people have conflicting ideas on what should/shouldn't be renamed automatically. I won't discuss that. Could it be that different PRs fixing those little corner cases are causing a race condition between conflicting rules? I think it works consistently 99.9% of the time, but sometimes it goes awry, hence why I opened this as a bug.

@fatcerberus
Copy link

I'll come back whenever I have some piece of code that reproduces this again.

You posted a screenshot of code that reproduces it. Can’t you just copy & paste that?

@dalmo3
Copy link
Author

dalmo3 commented Jun 18, 2021

In theory, yes. But there's nothing special about that piece of code. It just happened that the issue was happening at that point in time, all throughout the project, and I could've used any part of the codebase to show it. The piece in the image was just the first one I found simple enough to describe the issue concisely. The code worked just fine after I reloaded VSCode.

Anything as simple as this could potentially trigger the issue:

--- A.ts ---

export type A = {
  a: string;
};

--- B.ts ---

import { A } from "./A";
const { a }: A = { a: "" };

I added something similar to that to a sandbox and was able to reproduced it, but only once*: https://codesandbox.io/s/optimistic-water-teghj?file=/src/App.tsx (renaming a at App.tsx:13, changes type definition in A.ts). Note that the sandbox also contains the same tsconfig as the original post.

Edit: * By only once I mean it would reproduce consistently in the sandbox, then I saved it and opened on another tab, and the issue was gone - so that's why I didn't post it here at first.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 18, 2021

The sandbox's "Change all occurrences" functionality is provided by the editor, not TS. "Rename" is a different operation that understands more semantics. e.g. in that sandbox if you add

function fn() {
  let a = 4;
}

in app.tx, "Change all occurrences" invoked at the const { a } = will rename that a, but "Rename" will not.

@dalmo3
Copy link
Author

dalmo3 commented Jun 18, 2021

I understand, but that's unrelated to the issue I'm describing, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs More Info The issue still hasn't been fully clarified
Projects
None yet
Development

No branches or pull requests

3 participants