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

CodeAction: convert lambda expression declaration to function #23299

Closed
Kingwl opened this issue Apr 10, 2018 · 15 comments · Fixed by #28250
Closed

CodeAction: convert lambda expression declaration to function #23299

Kingwl opened this issue Apr 10, 2018 · 15 comments · Fixed by #28250
Assignees
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Refactorings e.g. extract to constant or function, rename symbol Suggestion An idea for TypeScript

Comments

@Kingwl
Copy link
Contributor

Kingwl commented Apr 10, 2018

TypeScript Version: 2.7.0-dev.201xxxxx

Code

const a = (arg: number) => (
    <div>{arg}foo</div>
)

Expected behavior:

function a (arg: number) {
  return (
    <div>{arg}foo</div>
  )
}

Actual behavior:
none

btw: also convert function to lambda expression

@Kingwl Kingwl changed the title codeAction: convert lambda expression declaration to function CodeAction: convert lambda expression declaration to function Apr 10, 2018
@mhegazy mhegazy added Suggestion An idea for TypeScript Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Refactorings e.g. extract to constant or function, rename symbol labels Apr 10, 2018
@Kingwl
Copy link
Contributor Author

Kingwl commented Apr 12, 2018

image

from IDEA

@zaaack
Copy link

zaaack commented Apr 12, 2018

+1. Add braces is very useful, too.

1 similar comment
@zzj3720
Copy link

zzj3720 commented Apr 12, 2018

+1. Add braces is very useful, too.

@AngryPowman
Copy link

Kingwl's proposals always help me a lot.

@vizee
Copy link

vizee commented Apr 12, 2018

Kingwl's proposals always help AngryPowman a lot.

@ly0
Copy link

ly0 commented Apr 12, 2018

Kingwl's proposals always help AngryPowman a lottery.

@DanielRosenwasser
Copy link
Member

@ly0's comments always help reiterate @vizee's points.

@whtsky
Copy link

whtsky commented Apr 18, 2018

I can't live without Kingwl's proposals.

@D0nGiovanni
Copy link
Contributor

Hi

We're a team of two, currently on our bachelor thesis. We'd like to implement this issue. Estimated timeframe is about 2 weeks, at most 4 weeks. We're looking to do other refactorings and code fixes as well, but this would come first.

@Kingwl
Copy link
Contributor Author

Kingwl commented Oct 5, 2018

could bachelor thesis based on an open source project with Apache License 2.0?😂
I suggest you confirm it in advance.

@D0nGiovanni
Copy link
Contributor

I don't know about other countries, but I don't see why it couldn't. It's approved anyway. We're already 3 weeks in. It's more of an engineering project than anything else.
I looked it up anyway, because I was curious. I didn't find any restrictions regarding license.

But we're getting slightly off-topic here... :P

@D0nGiovanni
Copy link
Contributor

I've got a question about reversing your example, @Kingwl. Of course, reversing an anonymous function makes sense. (More likely to go from right to left here)

foo(function (arg) { return <div>{arg}foo</div>; });

to

foo(arg => <div>{arg}foo</div>);

or even

foo(arg => <div>{arg}foo</div>);

to

foo(bar);
function bar (arg) {
  return <div>{arg}foo</div>; 
}

But what about named function to lambda? Is that actually a use-case? Afaik, Webstorm does not support this. Maybe for a good reason?

function a (arg: number) {
  return (
    <div>{arg}foo</div>
  )
}

to

const a = (arg: number) => (
    <div>{arg}foo</div>
)

Last question (for now): Do we also support lambda to method? Of course, we would have to check for class scope.

class kitty{
  someMethod(){
    foo(arg => <div>{arg}foo</div>);
  }
}

to

class kitty{
  someMethod(){
    foo(bar);
  }
  bar (arg) {
    return <div>{arg}foo</div>;
  }
}

@Kingwl
Copy link
Contributor Author

Kingwl commented Oct 12, 2018

lambda to method or the first case seems another refactor extract to(or combined more transform)

But what about the named function to lambda? Is that actually a use-case? Afaik,

if you want to edit, convert, or transform the parameter, but actually, add braces could cover the case

Webstorm does not support this. Maybe for a good reason?
yes, such as 'Hoisting'

@D0nGiovanni
Copy link
Contributor

D0nGiovanni commented Oct 12, 2018

You are right. They are different - and also already implemented.

So to clarify, there are only 2 cases that are in the scope of this refactor. Your example and this:

someFunction(function (arg) { return <div>{arg}foo</div>; }); <=> someFunction(arg => <div>{arg}foo</div>);

Edit: But lambda to named function should not be supported because of hoisting - or loss thereof.

@Kingwl
Copy link
Contributor Author

Kingwl commented Jun 2, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: Refactorings e.g. extract to constant or function, rename symbol Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.