Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Syntax considerations: How we define functions #487

Closed
sodic opened this issue Mar 3, 2022 · 4 comments · Fixed by #502
Closed

Syntax considerations: How we define functions #487

sodic opened this issue Mar 3, 2022 · 4 comments · Fixed by #502
Assignees
Labels
refactoring Keeping that code clean!

Comments

@sodic
Copy link
Contributor

sodic commented Mar 3, 2022

Wasp-generated code prefers defining functions as arrow function expressions. The docs suggest the same approach for user-defined Queries and Actions. This issue makes a case on why we should perhaps reconsider this preference.

The dilemma

There are two "dimensions" when it comes to differentiating between function definitions in JavaScript:

  1. Function statements vs. function expressions
  2. Arrow syntax vs. regular syntax

Arrow functions statements do not exist, meaning that there are a total of three options: regular-syntax statements, regular-syntax expressions, and arrow-syntax expressions. Out of the three options, the most appropriate way to define a named function seems to be a regular-syntax statement:

function f(foo) {
   // ....
} 

While the other two options certainly do have their use cases (e.g., forcing a lexical binding of this, shorter inline callbacks, etc.), they seem to be lacking when it comes to defining named functions. I'll do my best to list the advantages and disadvantages of all approaches. Most of the concerns are general, meaning some may not be particularly important for Wasp.

Advantages statements have over expressions

  1. Statements force you to name them - Function statements can't be anonymous, while expressions can (and often are). Even if we assign an anonymous function expression to a named variable, it remains anonymous in the call stack. Anonymous functions make debugging more difficult, and stack traces less readable. We could eliminate this issue by explicitly naming the function expression, but it only works with the regular syntax: const f = function f() { ... }. This same can't be done with arrow functions, as arrows are always anonymous.
  2. File organization - We will run into problems if we decide to put exports (i.e., public members) at the top of the file (this is a different discussion :)). It isn't possible to reference functions defined as expressions before their assignment, which forces us to put non-exported functions at the top and exported functions (that use the non-exported ones) at the bottom. Function statements are hoisted, allowing us to list the public API before its implementation details.
  3. Readability - Statements are (arguably) easier to read. It's immediately clear that function f... represents a function. With const f = function () ..., not so much.
  4. Mutual recursion - Since function expressions can't be referenced before their assignment, mutual recursion isn't possible (though we'll very likely never need it).

Advantages expressions have over statements

They become pretty handy in TypeScript. Let's suppose we want to define several functions that represent binary operations on numbers. All functions must follow the same interface (in Haskell, this would be Double -> Double -> Double).

Show example
function add(x: number, y: number): number {
    return x + y;
}

function multiply(x: number, y: number): number {
    return x + y;
}

// Decorating a function (i.e,. keep the signature, add something extra)
function logMultiply(x: number, y: number): number {
    console.log(`Multiplying ${x} and ${y}`)
}

Function expressions (working together with type inference) reduce code duplication:

type BinaryOperation = (x: number, y: number) => number;

const add: BinaryOperation = (x, y) => x + y;

const multiply: BinaryOperation = (x, y) => x * y;

// Decorating a function (i.e,. keep the signature, add something extra)
const logMultiply: typeof multiply = () => {
    console.log(`Multiplying ${x} and ${y}`)
}

Advantages the regular syntax has over the arrow syntax

  1. Regular functions don't have to be anonymous - The regular syntax allows us to name our functions, while the arrow syntax doesn't. As mentioned above, anonymous functions cause a lot of pain during debugging.
  2. Arrow functions come with subtle semantic differences - Arrow functions are not just syntactic sugar. They also have slightly different semantics than regular functions, making them less expressive in several ways.
  3. Don't think about this if you don't have to - Arrow functions' most popular semantic feature is the lexical binding of this. Sine we aren't doing anything with this in most cases, it's better to opt for a regular function and spare the reader from thinking about irrelevancies.
  4. Readability - const f = () => ... is (IMO) even less clear than const f = function () .... Also, the less non-standard non-alphanumeric characters, the better (controversial for a project written in Haskell, I know :)).

Advantages the arrow syntax has over the regular syntax

  1. Arrows are shorter and look a lot cooler 😎
  2. For better or worse, Arrows have pretty much become a standard way to define functions in modern JavaScript. Since Wasp is an open-source project and people seem to love writing arrow functions, we might have an easier time dealing with contributors by sticking with the arrow syntax.

Verdict

Since modern JavaScript can't exclusively use either syntax (there are cases where we must use arrow function expressions, as well as cases where we must not use them, details here), there will never be a consistent way to define functions across all contexts. Arrow functions will always be more practical for async callbacks, but incorrect as methods.

With that inherent inconsistency in mind, I suggest our default way of defining named functions becomes regular-syntax statements (it seems to have most going for it). We can then make exceptions to the rule when the need arises (e.g., TypeScript interfaces and decorators).

Assuming we all agree on the approach, it isn't something we must (or even should) start refactoring right away. After all, we have more pressing issues.

@Martinsos Martinsos added the refactoring Keeping that code clean! label Mar 4, 2022
@Martinsos
Copy link
Member

@sodic super cool issue, I enjoyed reading it and learned a lot :D!

I think our mean reason for using arrow functions was that they look cool :D and they feel more functional -> similar to lamdbas in Haskell. But that really comes down to just feel, not utility, as you nicely described above.

All together makes sense to me! I think you laid out your arguments very clearly, nothing to add from my side. I will personally try to stick with it if others are for it.
We have a "Code style guide" somewhere at the bottom of waspc README -> If you could add short instructions about this + link to this issue for explanation, I think that would be great.

So just to clarify, what is then the recommended approach? What I got is:

  1. Prefer statements over expressions. In that case, we have to use function, we can't use =>.
  2. For expressions, prefer using function instead of =>.

So is there a situation when => expression is better, or we should we completely abstain from using it?

@sodic
Copy link
Contributor Author

sodic commented Mar 6, 2022

  1. Prefer statements over expressions. In that case, we have to use function, we can't use =>.

That's right! But I'd like to emphasize once again that the issue only talks about top-level named function definitions.

  1. For expressions, prefer using function instead of =>.

We misunderstood each other on this one (my fault, I left it pretty ambiguous).

Assuming we implement all our top-level function definitions as statements, we're only left with inline function expressions (and those are mostly one liners). I'd stick with the arrows here. arr.map(x => x * 2) is a lot nicer than arr.map(function (x) { return x * 2}), and the functions are short and simple enough not to cause problems with this and debugging. If the arrow becomes too fat and complicated, we should probably refactor it into a named function anyway.

If we ever end up wanting to implement some top-level function definitions as expressions (e.g., the mentioned TypeScript use-case), we can decide what to use then. I could go either way: function syntax probably makes more sense on paper, but I feel a vast majority of people would prefer the arrow syntax (and it truly does look nicer).

So, to sum up:

  1. Top-level named function definitions - statements
  2. Inline expressions (i.e., lambdas) - arrows
  3. Top-level named function definitions that have to be expressions - TBD, we won't need those anytime soon

@Martinsos
Copy link
Member

Martinsos commented Mar 6, 2022

  1. Prefer statements over expressions. In that case, we have to use function, we can't use =>.

That's right! But I'd like to emphasize once again that the issue only talks about top-level named function definitions.

  1. For expressions, prefer using function instead of =>.

We misunderstood each other on this one (my fault, I left it pretty ambiguous).

Assuming we implement all our top-level function definitions as statements, we're only left with inline function expressions (and those are mostly one liners). I'd stick with the arrows here. arr.map(x => x * 2) is a lot nicer than arr.map(function (x) { return x * 2}), and the functions are short and simple enough not to cause problems with this and debugging. If the arrow becomes too fat and complicated, we should probably refactor it into a named function anyway.

If we ever end up wanting to implement some top-level function definitions as expressions (e.g., the mentioned TypeScript use-case), we can decide what to use then. I could go either way: function syntax probably makes more sense on paper, but I feel a vast majority of people would prefer the arrow syntax (and it truly does look nicer).

So, to sum up:

  1. Top-level named function definitions - statements
  2. Inline expressions (i.e., lambdas) - arrows
  3. Top-level named function definitions that have to be expressions - TBD, we won't need those anytime soon

This sounds very clear and reasonable! LGTM, and I think you can put this into README under code styles as I mentioned above.

@sodic sodic self-assigned this Mar 6, 2022
@sodic
Copy link
Contributor Author

sodic commented Mar 6, 2022

Will do, assigned myself to the issue!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
refactoring Keeping that code clean!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants