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

Enforce use of function for declared functions #324

Open
mcmire opened this issue Oct 19, 2023 · 4 comments
Open

Enforce use of function for declared functions #324

mcmire opened this issue Oct 19, 2023 · 4 comments

Comments

@mcmire
Copy link
Contributor

mcmire commented Oct 19, 2023

There are two ways to declare functions:

// 1
function foo(x: number): string {
  // ...
}

// 2
const foo = (x: number): string => {
  // ...
}

I propose that we add a rule which enforces that declared functions use the first style rather the second. There are a couple of issues with using an arrow function that I think function solves:

  1. It's not immediately obvious that the thing being set is a function. function pops out more visually.
  2. Adding type information for arrow functions can be confusing syntactically. (I've seen less senior developers struggle with doing this.) => is a bit overloaded, as it's used to indicate the return value for a function type in TypeScript. When using function, however, you don't have to deal with the ambiguity of => nor do you have extra characters in the way. : always separates the arguments from the return value, and the return value always goes before the {.

There are also some benefits for using function:

  • Currently we enforce that function functions are JSDoc'd, whereas we don't do that for const functions. We could certainly add another rule, but it would be nice if we didn't have to.
  • function functions are hoisted; when defined at the top level, this means they are automatically available anywhere in the file no matter where they're defined. This comes in handy when writing tests. If you want to define test helpers for tests, you can put them below the describe to hide them, thereby making describe the first thing that readers see when reading the file.
@MajorLift
Copy link

MajorLift commented Oct 19, 2023

Edge case for consideration:

The const F: (arg: T) => U syntax for typing functions is only supported by const function declarations, and it can be used for conveniently defining and typing higher-order constructs e.g.

const printCoordinates: (x: number) => (y: number) => `(${string}, ${string})` 
  = (x) => (y) => `(${x}, ${y})`;

console.log(printCoordinates(1)(-1));
// "(1, -1)"

If we stick to the function keyword for this, the type annotations become a lot more spread apart and redundant.

function printCoordinates(x: number): (y: number) => `(${string}, ${string})` {
    return function printCoordinatesForGivenX(y: number): `(${string}, ${string})` {
        return `(${x}, ${y})`
    }
}

But for most cases I agree with preferring the function keyword for the same reasons given in the post.

@brad-decker
Copy link
Contributor

I'm assuming the rule won't be applied for things like props, or functions passed in objects? For example if a function has an argument that is an object, and one of those keys can be a function should we be enforcing declaration of the function outside the object?

callSiteExample({
  prop1: 'foo',
  prop2: () => console.log('bar'),
})

versus

function prop2() {
  console.log('bar');
}

callSiteExample({ prop1: 'foo', prop2 });

There's definitely some cleanliness to the second example, just curious what your thoughts are here.

@mcmire
Copy link
Contributor Author

mcmire commented Oct 20, 2023

@MajorLift Ah yes, thanks for mentioning that edge case. It would make sense that we'd use const in that case.

@mcmire
Copy link
Contributor Author

mcmire commented Oct 20, 2023

@brad-decker Good point! Yes, my thinking was that this rule would only apply to function declarations and not function expressions. I guess a function defined via const is a function expression, though. So perhaps this rule would be difficult to enforce fully. But to answer your question directly, no, I think we wouldn't need to enforce this rule for functions created on the fly like in your example.

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

No branches or pull requests

3 participants