Suggestion: "extract to constant" for function is too verbose

[OliverJAsh]

Search Terms

extract constant refactoring expression arrow type parameter argument

Suggestion

Given:

[1,2,3].map(x => x + 1)

If you highlight x => x + 1 then run "extract to constant", you'll get:

const newLocal: (value: number, index: number, array: number[]) => number = x => x + 1;
[1,2,3].map(newLocal)

I can understand why this might make sense in some cases, but 99% of the time (in my experience), I end up having to tediously alter the constant that TypeScript has created for me. What I actually want is:

const newLocal = (x: number): number => x + 1;
[1,2,3].map(newLocal)

1. I want my parameter and return types to be inlined in the definition
2. (Less important.) I only care about the parameters that are in use

Perhaps this suggested behaviour could be a setting for the existing refactoring, or another refactoring entirely, or a new/better default for this existing refactoring.

Another example:

[1,2,3].map((x): number => x + 1)

produces

const newLocal: (value: number, index: number, array: number[]) => number = (x): number => x + 1;
[1,2,3].map(newLocal)

The return type is duplicated: it appears both inline and in the constant annotation.

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

[fatcerberus]

I only care about the parameters that are in use

Just my two cents: This part seems tricky, as for function types it’s not determinable from the type alone, it has to look inside the function body. TS doesn’t even do that for type inference (see #32254), so it’s probably not practical.

However I will note that: In your example you’re only picking up the extra parameters because of contextual typing, so maybe those could be ignored or something... hmm...

100% agree with point 1, though. The generated declaration is way too verbose.

[dragomirtitian]

@fatcerberus I don't think @OliverJAsh was suggesting doing anything overly complicated.

Just take the exiting function and make the inferred types of parameters explicit and move it to the constant. The types of the parameters are either known and can be extracted or are unknown and are implicitly any don't think there is a third option.

[fatcerberus]

I was talking specifically about the extra parameters introduced by the contextual typing for .map - you would need to selectively ignore those, since contextual typing is also needed in this case to get the type of the parameter that was actually used.

[dragomirtitian]

@RyanCavanaugh @DanielRosenwasser If you are willing to take PRs on this, I already have a solution that is about 90% of the way there, I would just need to polish it up and submit.

What i do is if the function type has only one non generic signature, I move the the parameter and return types to the function/arrow expression and remove the variable type.

I also remove the variable type if the parameter and return types are already fully specified in the existing expression regardless of the above single signature, non generic rule.

I still need to see about parameter that are inferred to any. If I encounter any such cases I think I should fallback to the current behavior, adding any is bad behavior for a refactor (IMO) and is probably caused by a more complex function signature.

[RyanCavanaugh]

I mean maaaaybe you want the original contextual type in case you were going to add parameters later, but that seems speculative at best. The proposed output looks way better than the current behavior.