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

fix: adds support for function overloads #26

Closed

Conversation

renato-bohler
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

renato-bohler commented Jan 24, 2024

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

That's fixed with 0d1f620. I validated this change on a large codebase.

@JamieMason
Copy link
Owner

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

JamieMason pushed a commit that referenced this pull request Feb 25, 2024
@JamieMason
Copy link
Owner

Merged in f60923a

@JamieMason JamieMason closed this Feb 25, 2024
@bensaufley
Copy link

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
Copy link

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

@renato-bohler
Copy link
Contributor Author

Sounds like another overload scenario I missed.

image

This should not report an issue.

Related: #31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent or ignore overload functions
3 participants