nicoespeon / abracadabra Goto Github PK
View Code? Open in Web Editor NEWAutomated refactorings for VS Code (JS & TS) ✨ It's magic ✨
Home Page: https://marketplace.visualstudio.com/items?itemName=nicoespeon.abracadabra
License: MIT License
Automated refactorings for VS Code (JS & TS) ✨ It's magic ✨
Home Page: https://marketplace.visualstudio.com/items?itemName=nicoespeon.abracadabra
License: MIT License
This is the reverse of #61.
Sometimes I could simplify a JSX attribute and I don't need the wrapping braces anymore. Having an Abracadabra quick fix would make this change a little bit faster.
With following code:
function render() {
return <Header title={"Hello"} />;
}
If my cursor is on the title={"Hello"}
attribute, I'd like to propose a quick fix to remove braces around:
function render() {
return <Header title="Hello" />;
}
It's the reverse of #61, so we need to unwrap the content of the JSXExpressionContainer
back into the JSX attribute value
.
A clear and concise description of what the bug is.
Please detail steps to reproduce the behavior. Add code if necessary.
Clear and concise description of what you expected to happen.
If applicable, add screenshots to help explain your problem.
🧙 Add any other context about the problem here.
With release 4.0.0 the Convert to Template Literal
was removed from the extension. The explanation for this is that VSCode introduced a similar refactoring (see here). The change log suggests that the refactoring is now obsolete in Abracadabra, so it was removed.
I am not so happy with this change, because the VSCode feature does not cover the same workflow that was possible with the Abracadabra refactoring.
It seems VSCode works like this: you take an already written string concatination like
const a = "I have " + numApples + " apples";
, apply the VSCode refactoring which changes the code to
const a = "I have ${numApples} apples";
This is useful, but it only works if you already have such a concatination.
My workflow with the refactoring often looked like this: I have a string in which I now want to inject a dynamic value. So I could place the cursor into the string and apply the Convert to Template Literal
refactoring, e.g.
const a = "I have some apples";
to
const a = "I have ${numApples} apples";
This now longer works now.
Would you consider re-introducing the refactoring to the extension?
A lot of times I would write:
return <button onClick={handleClick}/>
And then, above the return statement I would implement handleClick.
I would be nice if when the cursor is within the identifier handleClick
, I could have the option to create a function with the same name, resulting in the following code:
function handleClick() {
}
return <button onClick={handleClick}/>
And just wanted to add that I like this extension a lot! "Extract Variable" especially has been very useful in my day to day coding.
Hey!
I'm trying to contribute to this extension, but couldn't figure out how can I make changes in Abracadabra's code and see them live within the Extension Development Host (EDH). I tried running Webpack in watch mode, but then the EDH wouldn't start. I tried in regular build (yarn run build), but then I couldn't see the changes in the EDH until I restart webpack.
It's the reverse of #24
I prefer to use one-liner If Statements to write guard clause.
function doSomething() {
if (!isValid) return;
// … rest of the code
}
Sometimes, the guard clause is implemented with braces:
function doSomething() {
if (!isValid) {
return;
}
// … rest of the code
}
I'd like to simply remove braces with a quick fix, so it will take 2 keystrokes. Without that, I have to:
Given:
function doSomething() {
if (!isValid) {
return;
}
console.log("Something");
}
When I trigger the refactoring on the If Statement node,
Then I want:
function doSomething() {
if (!isValid) return;
console.log("Something");
}
It's a similar refactoring than Remove Braces from Arrow Function.
You can use yarn new
to help you bootstrap the refactoring.
I'm here to help.
The documentation is really great and give plenty of information.
As I started to contribute, I unfortunately missed some useful links about AST.
The links are actually provided in the CONTRIBUTING
file.
A table of contents, with clickable titles, might help to get a quick look on the file structure and go to the desired section.
I suggest to add a basic table of contents.
e.g.: https://github.com/jamiebuilds/babel-handbook/blob/master/translations/en/plugin-handbook.md#table-of-contents
Provided guidelines have a dynamic table of contents.
Not applicable.
Not applicable.
Current implementation of "Extract Variable" only extract the selected occurrence.
It could detect other similar occurrences. If there is more than one, it can propose to extract all of them to the user.
Webstorm does that quite well:
If the user decides to extract all occurrences, we should replace all of them with the extracted variable.
The trick is to provide a nice UX when asking the user about that:
Few options in mind:
So, if we manage to get the selection popup appear where the cursor is when we ask for extraction, I'd order the choices differently:
That's because my personal experience is I want to extract all the occurrences 80% of the time I do this.
After digging into VS Code capabilities, I think there is no other way than using "Quick Pick" to do so. I'd have preferred the popup to be at the cursor position to stay in context, but I don't see how to do that with VS Code.
Thus, the ideal behaviour would be:
This is a refactoring we started working on in February, but life came across and we didn't finish it: https://github.com/nicoespeon/abracadabra/tree/extract-generic-type
Creating an issue so we don't forget to finish this.
The main idea is to be able to make a type generic (so it's TS specific):
interface Position {
x: number;
y: number;
}
With cursor on x: number
would propose "Extract Generic Type" refactoring. If you trigger it, code will become:
interface Position<T = number> {
x: T;
y: number;
}
And T
symbol should enter rename mode.
Creating a new refactoring usually involves the same boilerplate code:
src/refactorings/
extension.ts
to use the commandextension.ts
accordinglyWe could simplify this step with a code generator.
An example of "new refactoring" is available here: #11
I think we could use https://www.hygen.io/ for that.
It doesn't need to be perfect. A starting point would be good. Hence, this is something that could be done as a first issue (and I can help!) 🧙♂️
If you want this refactoring to be developed next, please vote on this issue with 👍
VS Code does provide this refactoring. But again, the UX is not the one I expect.
It's not that bad, for sure. It handles extraction into a method if we're in a class.
But:
Webstorm does this well: https://www.jetbrains.com/help/webstorm/specific-javascript-refactorings.html#javascript_extract_method
It's even better because, when you select where to extract the function, it does scroll to the position, making it explicit.
It's not a trivial one. Check if there is a way to rely on VS Code extraction, but provide a better UX (partial selection & showing "where" the function will be extracted).
Otherwise, the mechanics is:
The Extract Variable refactoring currently only runs when the cursor is placed in a piece of code. To have more control about what exactly is being extracted, the refactoring should allow the user to select an expression in the code editor. Currently, if you select a piece of code and run the refactoring, you get an error message: "I didn't found a valid code to extract from current selection".
Not sure if this is a bug or just not supported yet.
The refactoring takes into account what is being selected. Only the selected expression is extracted to a variable.
Example:
function test() {
const myVar = {
someArray: [
{
somethingNested: 42
}
]
};
console.log(myVar.someArray[0].somethingNested);
}
Select myVar.someArray[0]
and run the refactoring ->
...
const extracted = myVar.someArray[0];
console.log(extracted.somethingNested);
...
Select myVar.someArray[0].somethingNested
and run the refactoring ->
...
const extracted = myVar.someArray[0].somethingNested;
console.log(extracted);
...
Abracadabra propose to replace way too many values when I "extract variable" an array with the entire array selected.
With the following snippet:
const a = []
const b = [{a: "foo", b: "bar"}, {a: "foo", b: "bar"}]
[{a: "foo", b: "bar"}, {a: "foo", b: "bar"}]
const extracted = [{a: "foo", b: "bar"}, {a: "foo", b: "bar"}];
const a = extracted
const b = extracted
Don't replace a
when a != b
This bug is only present with empty arrays (a = []
), so probably in the comparison algo.
Deno has top level await enabled, and so does TypeScript 3.8.
At the moment any code with this in stops abracadabra working. I think it could be cured by updating https://www.npmjs.com/package/@babel/parser to the latest, it seems it supports top level await now, but I don't know if you need to enable it? I have not used Babel. Cheers!
Jest it.each
seemed great, but they are limited.
Specifically, I want to be able to .only
a single test of the list when I'm working on it, which is not doable. I know this can be done at runtime through Jest runner, but I use other tools that are integrated with the editor (so I don't look at the terminal). Thus, I need .only
.
With the following snippet as example:
import PanelGroup from "react-panelgroup";
import React from "react";
const MyAwesomeComp: React.FC = () => {
// ...
return (
<PanelGroup onUpdate={([sidebar]) => setSidebarWidth(prev => sidebar.size || prev)}>
{/* Implementation details… */}
</PanelGroup>)
};
If I try to "Extract variable" the onUpdate
function in a typescript context, the refactor will not be as good as expected due to the lack of types.
Indeed, when a function is defined as props, the types inferred, but not when it's extracted.
In a perfect world, I would like this after the "Extract variable" on this kind of case.
import PanelGroup, {PropsTypes} from "react-panelgroup";
import React from "react";
const MyAwesomeComp: React.FC = () => {
// ...
const extracted: PropsType["onUpdate"] = ([sidebar]) => setSidebarWidth(prev => sidebar.size || prev)
return (
<PanelGroup onUpdate={extracted}>
{/* Implementation details… */}
</PanelGroup>)
};
In an ever better world, what I really would like is to have a Extract in useCallback
!
"Inline variable" refactoring doesn't work if the variable is declared via destructuring
// Case 1.
// Command+Option+N ->
// Throws "I didn't found a valid code to inline from current selection 🤔" error
const { userId } = session
return messages.map(message => ({ userId }))
// Case 2.
// Command+Option+N ->
// Inlines just fine
const playerId = session.playerId
return messages.map(message => ({ playerId }))
Expecting Case 1. to work
Thank you for this amazing project btw ❤️
When I write following code:
error message:
Unexpected token (29:4): SyntaxError: Unexpected token (29:4)
at Object.raise (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:657452)
at Object.unexpected (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:677107)
at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:698149)
at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:575944)
at Object.parseExprSubscripts (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:690154)
at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:689746)
at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:646315)
at Object.parseExprOps (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:687156)
at Object.parseMaybeConditional (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:686688)
at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:685576)
at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:645779)
at Object.parseObjectProperty (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706149)
at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706631)
at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:644824)
at Object.parseObjectMember (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:704931)
at Object.parseObj (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:703762)
at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:697112)
at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:575944)
at Object.parseExprSubscripts (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:690154)
at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:689746)
at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:646315)
at Object.parseExprOps (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:687156)
at Object.parseMaybeConditional (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:686688)
at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:685576)
at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:645779)
at Object.parseObjectProperty (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706149)
at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:706631)
at Object.parseObjPropValue (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:644824)
at Object.parseObjectMember (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:704931)
at Object.parseObj (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:703762)
at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:697112)
at Object.parseExprAtom (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:575944)
at Object.parseExprSubscripts (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:690154)
at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:689746)
at Object.parseMaybeUnary (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:646315)
at Object.parseExprOps (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:687156)
at Object.parseMaybeConditional (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:686688)
at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:685576)
at Object.parseMaybeAssign (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:645779)
at Object.parseVar (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:728033)
at Object.parseVarStatement (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:724611)
at Object.parseStatementContent (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:717106)
at Object.parseStatementContent (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:641972)
at Object.parseStatement (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:715840)
at Object.parseBlockOrModuleBlockBody (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:726551)
at Object.parseBlockBody (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:726360)
at Object.parseTopLevel (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:714491)
at Object.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:743906)
at Object.t.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:745123)
at Object.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1040107)
at Object.t.parse (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1281972)
at Object.y [as parse] (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1040079)
at t.RefactoringActionProvider.provideCodeActions (c:\Users\YuTengjing\.vscode\extensions\nicoespeon.abracadabra-3.2.1\out\extension.js:1:1345737)
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:586:864
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:54:345
at new Promise (<anonymous>)
at Object.t.asPromise (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:54:317)
at T.provideCodeActions (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:586:835)
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:609:631
at V._withAdapter (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:604:832)
at V.$provideCodeActions (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:609:609)
at p._doInvokeHandler (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:688:464)
at p._invokeHandler (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:688:156)
at p._receiveRequest (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:686:766)
at p._receiveOneMessage (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:685:623)
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:683:791
at l.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:48:845)
at v.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:191:325)
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:851:104
at l.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:48:845)
at v.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:191:325)
at t.PersistentProtocol._receiveMessage (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:195:717)
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:192:897
at l.fire (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:48:845)
at p.acceptChunk (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:188:897)
at c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:188:249
at Socket.t (c:\applications\Microsoft VS Code\resources\app\out\vs\workbench\services\extensions\node\extensionHostProcess.js:197:232)
at Socket.emit (events.js:203:13)
at addChunk (_stream_readable.js:295:12)
at readableAddChunk (_stream_readable.js:276:11)
at Socket.Readable.push (_stream_readable.js:210:10)
at Pipe.onStreamRead (internal/stream_base_commons.js:166:17)
write following code:
const obj = {
a:
}
no error message
Version: 1.43.0 (user setup)
Commit: 78a4c91400152c0f27ba4d363eb56d2835f9903a
Date: 2020-03-09T19:47:57.235Z
Electron: 7.1.11
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Windows_NT x64 10.0.18363_
As we introduced Add Braces to Arrow Function, we should implement the reverse.
It should work as a Quick Fix.
It should not work if arrow function body doesn't have just a return statement.
Same reason than Add Braces to Arrow Function.
VS Code does provide this refactoring too, but it has the same problem as usual: it only works if you got the correct selection:
If your cursor is inside the arrow function, no cookie for you:
Trying to refactor there is a necessity to select or put the cursor on the exact place to see the available refactorings. I know that it is a default for VSC built-in refactoring but it is problematic/ irritating.
It would be perfect if there will be a command which will combine two steps:
The feature I mean is implemented in that way in another extension: Intelli Refactor
Thank you for great extension!
When extracting a variable, Abracadabra proposes to replace all occurrences. When doing so, it's possible to generate invalid code because the variable declaration will not be placed in a scope that's accessible by all occurrences.
function brokenScenario() {
if (isValid) {
if (item.name == "sulfuras") {
}
}
console.log("sulfuras");
}
Put your cursor on any "sulfuras"
and extract the variable. Select to replace the 2 occurrences.
Here's the resulting code:
function brokenScenario() {
if (isValid) {
const sulfuras = "sulfuras";
if (item.name == sulfuras) {
}
}
console.log(sulfuras);
}
The second occurrence can't access the variable.
The declaration is placed before the top-most occurrence:
abracadabra/src/refactorings/extract-variable/extract-variable.ts
Lines 32 to 41 in 2abc82f
It doesn't take the scope of all occurrences into account.
The declaration should be put in the closest common ancestor of all occurrences.
function brokenScenario() {
const sulfuras = "sulfuras";
if (isValid) {
if (item.name == sulfuras) {
}
}
console.log(sulfuras);
}
There already is a built-in move to file refactoring but it only moves functions to a new file with the function name as file name.
A "Move to file" refactoring should be able to either move the function to a new file (already built-in) or an existing file. Imports and global constants should be moved (or copied if moving is not possible because they are used elsewhere) as well.
First of all: Thanks for this great extension 👍
I'm experiencing an issue when trying to start the debugger in "Run Extension" mode and i'm not sure if this is a bug or just my somewhat misconfigured VSCode 😅
VSCode shows me an error telling me that the npm: build
task does not exist:
The .vscode/tasks.json
indeed only contains an npm: watch
task and i'm not sure if the build task is actually missing or if VSCode "somehow" should figure it out.
Extension version: latest master commit (a72c6f9)
VSCode version: Version: 1.41.1
The for loop to foreach refactoring apparently does not work on for (const entry of entries)
style loops. At least the plugin says it did not detect a valid for-loop.
Would be great to make it work for this style of loops.
Abracadabra version: 3.2.3
If you want this refactoring to be developed next, please vote on this issue with 👍
This is the reverse operation of "Extract Function".
VS Code doesn't seem to propose that.
Webstorm does this refactoring quite well: https://www.jetbrains.com/help/webstorm/specific-javascript-refactorings.html#javascript_inline_refactorings
If it's a method of a class, we'd need to check if it's overridden in a subclass. It's fine to not handle that case for now and tell the user about it.
The mechanics is the following:
Currently, when you choose "Add braces to if statement" it works with a single line; however, it's often the case that multiple lines need to be added to an if statement.
Say I have three lines that are not yet part of an if statement (S1):
let ok
ok = someFunc()
callAnotherFunc(ok)
I've started typing an if statement (S2):
if (condition === 'true')
let ok
ok = someFunc()
callAnotherFunc(ok)
And now I'd like to make all three lines part of an if statement block (S3):
if (condition === 'true') {
let ok
ok = someFunc()
callAnotherFunc(ok)
}
I'd like to be able to highlight the three lines above (in S2), and then choose "Add braces to if statement" and have it result in S3.
Version of the extension used: 4.1.0
Shift + Ctrl + Arrow up/down control the multiple selection cursor which is a top VS Code feature, it seems abracadabra overrides this with the Move Statement shortcut... this can't be right?!
I do use one-liner If Statements to write guard clause.
function doSomething() {
if (!isValid) return;
// … rest of the code
}
But sometimes, the guard clause might evolve and do something else. In that scenario, I want to add braces.
function doSomething() {
if (!isValid) {
// … now I can add some logic here
return;
}
// … rest of the code
}
Adding braces can be a bit painful because the editor will likely insert the closing one. So I usually have to:
Some extensions might help me wrap selection into braces. I want almost the same thing, but something simpler and specific to If Statements: a quick fix that would trigger whenever the selection is inside the correct node.
Given:
function doSomething() {
if (!isValid) return;
console.log("Something");
}
When I trigger the refactoring on the If Statement node,
Then I want:
function doSomething() {
if (!isValid) {
return;
}
console.log("Something");
}
It's a similar refactoring than Add Braces to Arrow Function.
You can use yarn new
to help you bootstrap the refactoring.
I'm here to help.
Example on Webstorm:
This is a proposal for extensions to show refactoring documentation to end users.
We could use that to show a link to the docs when refactorings can be made (JS, JSX, TS, TSX files), for Abracadabra users.
We'd use that to redirect people to Abracadabra refactoring documentation (which is in the README, as of today).
When I try to apply the Extract Variable refactoring, the generated code is messed up, resulting in not compilable code. See below.
Consider this code:
function test(arg: string[]) {
console.log(arg.length);
const a = arg.length + 1;
const b = arg.length % 2;
return { a, b };
}
Put the cursor on the length
part of arg.length
(e.g. on the "n"). Execute Extract Variable refactoring.
The refactoring generates this code:
function test(arg: string[]) {
conconst { length } = arg;
sole.log(arglength
const a = arglength1;
const b = arglength2;
return { a, b };
}
The expression is extracted to a single variable and each occurence is replaced with the created variable. No syntax error is created.
E.g. something like this:
function test(arg: string[]) {
const length = arg.length;
console.log(length);
const a = length + 1;
const b = length % 2;
return { a, b };
}
This would help me and others a lot. Would it be complex to implement, assuming we already have if/else to switch?
Sent with GitHawk
It would be great to have tests run against PRs and master. I've activated the repo on Travis. Now we only miss:
Often when I'm refactoring, I find a hidden class. A collection of functions and properties that would be better in a new class or in another existing class.
I'd like to be able to select a function or more, maybe even some variables, and extract it out to a new class.
I'd also like for all existing calls to those functions to now be prepended with the new class.
For example, I'd like to turn this:
class Foo () {
barSize: int = 30;
fooString: string = "baz";
doFoo () {
console.log(fooString);
}
doBar() {
return barSize /2;
}
doubleBar() {
return barSize *2;
}
}
class Baz() {
Foo.doBar();
Foo.doFoo();
}
into this:
class Foo () {
fooString: string = "baz";
doFoo () {
console.log(fooString);
}
}
class Bar() {
barSize: int = 30;
doBar() {
return barSize /2;
}
doubleBar() {
return barSize *2;
}
}
class Baz() {
Bar.doBar();
Foo.doFoo();
}
Being able to then move it into a new file would be a nice bonus.
A quick fix option in the VSCode Quick Fix menu (CTRL+.) for the Inline Variable
/ Inline Function
refactoring when it is applicable.
It would check whether the refactoring can be applied at the currently selected location in the code and would then add the refactoring to the Quick Fix menu just like it's being done with other refactorings
Consider this code:
const test = "hello";
console.log(test);
Put cursor on the variable declaration ("test" in line 1). VSCode should then offer the Inline Variable Quick Fix.
I am not sure if this is a bug or a feature request. As soon as I started using this plugin I am finding immediately many cases like this:
function calculateYear(selectedDates) {
if (
selectedDates[0].getMonth() === 0 ||
(selectedDates[0].getMonth() === 1 && selectedDates[0].getDate() < 4)
) {
return selectedDates[0].getFullYear() - 1;
}
return selectedDates[0].getFullYear();
}
abracadabra does not seem to recognise this as an ideal chance to convert to a ternary, and there is no lighbulb or option to convert. It would be great if it did see this opportunity and output:
function calculateYear(selectedDates) {
return selectedDates[0].getMonth() === 0 ||
(selectedDates[0].getMonth() === 1 && selectedDates[0].getDate() < 4)
? selectedDates[0].getFullYear() - 1
: selectedDates[0].getFullYear();
}
I know this is not a great bit of logic in my code, but this is just an example, I am having a go at refactoring an old app :) Cheers!
My workflow often involves writing the call site of a new function first and then to write the actual function. This means I have to declare the function signature so I usually copy and paste the function name, check the infered types of the parameters and manually add the parameters to the signature. This is a lot of repeated manual work and I'd like some automatic solution for that.
Edit: I understand that technically this isn't a refactoring since it does not involve transforming working code to a different form of working code, but rather broken code to working code. But maybe this would also be in the scope of this extension.
I suggest a new refactoring which implements the declaration of a new function based on its call site. The new function declaration is put in the enclosing scope of the current function.
For example, say I have this code:
function doSomething(): number {
const a = 3;
const b = 4;
return sum(a, b);
}
The function sum
is not declared in this scope. I put my cursor on sum
and invoke the refactoring. The refactoring generates code like this:
function doSomething(): number {
const a = 3;
const b = 4;
return sum(a, b);
}
function sum(a: number, b: number): number {
}
This could also work with async code, i.e. invoking the refactoring on return await sum(a, b);
would generate async function sum(...)
.
Ideally, the refactoring is only offered to the user if the function is not declared yet.
Here's how Android Studio does it:
This could also be extended to types, class members, etc.
Sometimes, I need to transform a primitive JSX attribute into an expression. To do so, I need to wrap the attribute with braces. I'd be nice if Abracadabra could do it for me.
With following code:
function render() {
return <Header title="Hello" />;
}
If my cursor is on the title="Hello"
attribute, I'd like to propose a quick fix to add braces around:
function render() {
return <Header title={"Hello"} />;
}
The difference between these 2 snippets in terms of AST is a JSXExpressionContainer.
First AST is:
Second AST is:
So we need to put the value
of the attribute into a JSXExpressionContainer
if it's not already.
I think the way we provide the Quick Fixes might lead to performance issues. So far, it seems to be fine. But I'm not sure how much current implementation will scale.
If you experienced performance issues because of the extension, please let us know here 🙏
Today, each Quick Fix is independent.
To tell VS Code if it should propose a refactoring as a Quick Fix, we traverse the code, check if the refactoring could be made from current selection, and tell VS Code if we can.
Example of such behavior, implemented in action-provider.ts
:
The performance problem would be due to the fact that each refactoring parses, traverses and transforms the AST. At some point, this could be too much.
If this is confirmed, the solution would be to traverse the AST less. Ideally, we would traverse it once and try to figure out what list of refactorings could be performed from that.
The challenge would be to provide individual action providers to VS Code. That's why current implementation is very naive, so every refactoring is independent—which simplifies development a lot!
But until we got actual confirmation that this is a concern, we'll keep current implementation. #makeItWork #makeItRight #makeItFast
It would be nice to be able to extract selected text subset of a string or template literal.
For example, with this code:
const message = "[*] Mailing list worker started (attached to web app)";
If we select Mailing list worker
and run Extract Variable, the code should be:
const extracted = "Mailing list worker";
const message = `[*] ${extracted} started (attached to web app)`;
Same for template literals.
We should probably handle a bunch of edge cases:
Following the principle of proximity: everything that changes together, should be close together.
Current structure for commands & action providers is mostly "a technical choice" as it's "the VS Code part". We're mature enough to move these along refactorings now.
Today, Abracadabra limits its scope to the open file. Thus, we don't inline variables that are exported.
Also, unlocking inter-files refactorings will allow for a whole new set of refactorings, including class-related refactorings since classes are often separated in distinct files from my experience.
We should be able to index symbols across the editor workspace. Based on that, we should be able to extend refactoring outside of a single file.
That might change the way we handle refactorings too. Today, we parse the AST every time we trigger the refactoring. Tomorrow, we might want to have a language server or something that keeps track of the code structure, so it's faster to update.
When I inline an exported variable, it is inlined in other files that imports it.
It would be awesome if we could rely on existing work. I guess TypeScript or VS Code already does that at some point. It might be the point where it'd be more valuable to contribute to native refactorings that trying to re-do the work in this extension though 🤷♂
I find myself refactoring boolean expressions quite often and sometimes I (i.e. the available refactorings) end up with one of the following expressions:
const one = a ? b : true;
const two = a ? b : false;
const three = a ? true : c;
const four = a ? false : c;
const five = a ? true : false;
const six = a ? false : true;
All of these can be simplified:
const one = !a || b; // = a ? b : true;
const two = Boolean(a) && b; // = a ? b : false;
const three = Boolean(a) || c; // = a ? true : c;
const four = !a && c; // = a ? false : c;
const five = Boolean(a); // = a ? true : false;
const six = !a; // = a ? false : true;
If a
is boolean, the casts are redundant and can be omitted.
I‘d love a refactoring on any ternary that simplifies these expressions.
Additionally, an "Expand to ternary expression" refactoring which is the inverse of the described "Simplify ternary expression" would be awesome.
There are other expressions which can be simplified, e.g.:
const seven = a || !a; // => a
const eight = true || b; // => true
const nine = false || b; // => b
const ten = true || true; // => true
const eleven = false || false; // => false
const twelve = true || false; // => true
const thirteen = false || true; // => true
const fourteen = a && a; // => a
const fifteen = a && !a; // => false
const sixteen = a && true; // => Boolean(a)
const seventeen = a && false; // => false
const eighteen = true && a; // => a
const nineteen = false && a; // => false
const twenty = a && b || a && c; // => a && (b || c)
const twentyOne = !(a && b); // => !a || !b
const twentyTwo = !(!a || !b); // => Boolean(a && b)
...
This feature request covers ternary expressions but my desired solution would be called "Simplify expression“ and it would recursively try to find the shortest /most simple version of a given expression.
First of all, thank you for this great extension! ❤️
One shortcut built-in to VS Code which I frequently use is command + shift + down/up, to select from the cursor to the start/end of file.
Unfortunately this extension defines a shortcut which conflicts with this. I can probably reconfigure the extension to avoid this conflict, but I thought I would file an issue in case this effects other users. Perhaps we could replace the default shortcut with one that isn't already being used? 😄
This is a continuation of #77.
"Extract Variable", "Inline Variable" and "Negate Expression" are not implemented the same way than the more recent refactorings.
Instead of manipulating the AST, we identify the nodes, then ask VS Code to read the code and finally we act on that. We could probably refactor them to have the same implementation than the recent ones though.
The problem is their tests are not super explicit like the others can be.
What we ideally want is:
Improve the tests of these refactorings to show that. It will require implementing an in-memory adapter for that.
The current UX of "Move Statements" refactorings is not as good as I'd expect.
Few things that needs to be improved:
Webstorm does it very well. What I want is to feel the same comfort in moving statements within VS Code, thanks to Abracadabra.
Let's do some usability tests (e.g. try to move statements on some patterns and see if it doesn't feel perfect) and fix the issues.
I'll close the issue when I feel Move Statements UX is so good that I would only use that to move things up and down.
After playing with current implementation, here are the things we want to improve:
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.