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

Rename of import should be a local rename #14039

Closed
ghost opened this issue Feb 13, 2017 · 7 comments
Closed

Rename of import should be a local rename #14039

ghost opened this issue Feb 13, 2017 · 7 comments
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Milestone

Comments

@ghost
Copy link

ghost commented Feb 13, 2017

TypeScript Version: nightly (2.2.0-dev.20170213)

Code

import { readFileSync } from "fs";
readFileSync("a.txt");

Rename readFileSync to read.

Expected behavior:

import { readFileSync as read } from "fs";
read("a.txt");

Actual behavior:

import { read } from "fs";
read("a.txt");

And @types/node/index.d.ts is modified too!

@ghost ghost self-assigned this Feb 13, 2017
@krryan
Copy link

krryan commented Feb 13, 2017

The current behavior is exactly what I expect and want.

To rename all instances of something in a given file, Ctrl+D is really useful.

@ghost
Copy link
Author

ghost commented Feb 13, 2017

What editor are you using? What does Ctrl+D do?

@normalser
Copy link

I think it would be better to leave the current way as it is, and use this trick to do the alias rename only

  1. Initial
    import { readFileSync } from "fs"

  2. Use alias
    import { readFileSync as readFileSync } from "fs"

  3. Rename alias (right readFileSync to read)

Unfortunately there is related bug: #10894

@krryan
Copy link

krryan commented Feb 13, 2017

@andy-ms Ah, sorry; I was actually thinking of this as an editor feature/tool, rather than a language one (kind of surprises me that the language itself controls this). I use VS Code. Ctrl+D selects other strings of text matching the one you have highlighted. This is multi-line selecting, so you can edit all of them at once. Exceedingly useful feature.

@Jessidhia
Copy link

I think the behavior should depend on the context, but I am not sure it is possible.

Say, given

import { readFileSync } from "fs";
readFileSync("a.txt");

If the cursor is in the readFileSync call, I'd expect renaming it to create an alias; while if I either go to the definition, or rename it from the import specifier itself, to rename the symbol definition.

A similar issue exists with object shorthand notation (renaming a symbol that's used in shorthand also renames its object key, instead of separating the key/value).

@mhegazy
Copy link
Contributor

mhegazy commented Oct 16, 2017

Related to #19173 and #15787

@pelotom
Copy link

pelotom commented Nov 8, 2017

@krryan

Ah, sorry; I was actually thinking of this as an editor feature/tool, rather than a language one (kind of surprises me that the language itself controls this).

It makes a lot of sense for the language to control this; correct refactorings require deep knowledge of the semantics of the language being edited.

I use VS Code. Ctrl+D selects other strings of text matching the one you have highlighted. This is multi-line selecting, so you can edit all of them at once. Exceedingly useful feature.

It is a useful approximation when you don't have better tools for the job, i.e. semantically-aware refactorings.

I think this change is needed, at least in the case where a refactor would end up editing a file under node_modules; that's never what you meant to do. And if we do it in that case, it would feel inconsistent to do otherwise in other cases.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

6 participants