JamieMason / eslint-plugin-prefer-arrow-functions

Auto-fix plain Functions into Arrow Functions, in all cases where conversion would result in the same behaviour
https://www.npmjs.com/package/eslint-plugin-prefer-arrow-functions
MIT License
43 stars 11 forks source link

fix: adds support for function overloads #26

Closed renato-bohler closed 4 months ago

renato-bohler commented 5 months ago

Description (What)

This PR adds support for function overloads. Function overloads are not supported as arrow functions.

Fixes #17

Justification (Why)

Because otherwise the plugin will try to convert overloaded functions to arrow functions, breaking its intended behavior.

How Can This Be Tested?

  1. Checkout this branch
  2. Install dependencies
  3. Run yarn test
  4. The new entries in alwaysValid should pass on the test
renato-bohler commented 5 months ago

FYI: this implementation is missing support for exported overloaded functions. I'll add that soon.

// Currently working:
function foo(): any; // This node has type `TSDeclareFunction`
function foo() {}

// Missing implementation:
export function bar(): any; // This node has type `ExportNamedDeclaration` with a property `declaration` of type `TSDeclareFunction`
export function bar() {}
renato-bohler commented 5 months ago

FYI: this implementation is missing support for exported overloaded functions. I'll add that soon.

That's fixed with https://github.com/JamieMason/eslint-plugin-prefer-arrow-functions/pull/26/commits/0d1f6203554b80dc582be93a5aace8fe5e4f5cc9. I validated this change on a large codebase.

JamieMason commented 5 months ago

Thanks for all your work on this, will get to this when I can, maybe will be able to Sunday.

JamieMason commented 4 months ago

Merged in f60923a

bensaufley commented 1 month ago

Does this require a change to configuration? I don't see anything but I've updated to 3.3.2 and prefer-arrow-functions/prefer-arrow-functions is still tripping for me. Code looks like this:

export default function useGoalTracking(wait?: string, groupId?: string): GoalMapping;
export default function useGoalTracking(wait: string, groupIds: string[]): { [groupId: string]: GoalMapping };
export default function useGoalTracking(
  wait = 'goals.get',
  groupId: string | string[] | undefined = undefined,
): GoalMapping | { [groupId: string]: GoalMapping } {

Works as expected, except I'm currently having to insert eslint-disable-next-line prefer-arrow-functions/prefer-arrow-functions before the implementation.

Maybe I'm misunderstanding what this change was intended to do? But looking at the tests, I feel like the implication is that my code should work without tripping this rule.

bensaufley commented 1 month ago

Ope, of course as soon as I post this – it looks like export default is what's doing it.

renato-bohler commented 1 month ago

Sounds like another overload scenario I missed.

image

This should not report an issue.

Related: https://github.com/JamieMason/eslint-plugin-prefer-arrow-functions/pull/31