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

Determine our function/method definition syntax style #524

Closed
dtribble opened this issue Feb 7, 2020 · 11 comments
Closed

Determine our function/method definition syntax style #524

dtribble opened this issue Feb 7, 2020 · 11 comments
Assignees
Labels
code-style defensive correctness patterns; readability thru consistency hygiene Tidying up around the house

Comments

@dtribble
Copy link
Member

dtribble commented Feb 7, 2020

Right now we have a mix of classic method definition and arrow function. We should ideally have a single style, but there may be driving cases for each. We need to identify those and converge on a consistent usage.

? which style of method definitions should we use?
  : classic style `{ foo(args) { ... return result; } }`
    + familiar to more programmers from C. Java, C#, etc.
  : arrow function style: `{ foo: (args) => { ... return result; } }`
    + allows elimination of curlies on short one-line methods
    + sometimes ideal for capturing `this`
      - but we don't use `this` much
    - inconsistent in usage even in arrow functions (e.g. with and without returns)
    - not visually distinct from defining a non-function property

Resolved: arrow function style

Jun 7

best practice docs: Arrow Function Style

@DavidBruant
Copy link
Contributor

sometimes ideal for capturing this

this isn't used a lot in the Agoric code i've seen (mostly ERTP and smart contracts). I think it's because this isn't part of Jessie

The "classic" style is also close to the getter/setter syntax

@michaelfig
Copy link
Member

"classic" style is officially called "concise method syntax".

@katelynsills
Copy link
Contributor

Here's my thoughts:

? which style of method definitions should we use?
  : classic style `{ foo(args) { ... return result; } }`
    + familiar to more programmers from C. Java, C#, etc.
    - doesn't follow the usual format of declaring a variable
    - often takes up a significant amount of room with boilerplate
    - feels outdated
  : arrow function style: `{ foo: (args) => { ... return result; } }`
    + allows elimination of curlies on short one-line methods
    + familiar to anyone who has any experience with react (used in their frontpage example https://reactjs.org/)
    + Many of the most-recommended JavaScript books use this style (e.g JavaScript Allonge https://leanpub.com/javascriptallongesix/read#functions) 
    + not visually distinct from defining a non-function property (this is a plus!)
    + rides the rising tide of functional programming, which is getting very popular

To sum up, I think it's as easy or easier to use, and puts us into the "up-and-coming" well-written JavaScript category rather than the outdated category.

@erights what do you think?

@erights
Copy link
Member

erights commented May 13, 2020

I mostly agree with both @dtribble 's and @katelynsills 's +/- points above. Some additional points:

@DavidBruant 's point about getter/setter syntax is valid and does argue in favor of concise method syntax. There is no practical arrow function syntax for expressing accessors. Jessie does admit accessors, though perhaps it should not. From recent conversations with @michaelfig though, we are more likely to keep accessors in Jessie and to extend captp to make use of them.

All functions in JavaScript toString either as a particular unparseable form (typically builtin functions) or as their original source text. For all functions other than concise methods, their source text is also a valid expression that, if evaluated in a sufficiently similar lexical environment, produces a function with the same call and construct behaviors. This will be important for using closed functions for mobile code (e.g., Promise.there). This is not true for concise methods. Their original source text, if evaluated as an expression, can produce an object unrelated to the purpose of the concise method. (Waldemar, unsurprisingly, had good examples.) This is an additional point in favor of arrow functions, but an insignificant one I list only for completeness. It is insignificant because we never expect to take some random existing function and use it as mobile code. Rather, the functions we use as mobile code we will explicitly write to use that way.

Looking at the overall picture above I'd be on the fence though leaning more towards arrow-only. However, I've been using arrow-only style for a while now as an experiment. I like it. For me this is a tie breaker. But, to @DavidBruant 's point, my position isn't really arrow-only because I still want to admit accessors.

@katelynsills katelynsills added the hygiene Tidying up around the house label Feb 8, 2021
@erights erights self-assigned this Feb 26, 2021
@erights
Copy link
Member

erights commented May 11, 2021

See also endojs/endo#708 (review) , which is about function functions vs arrow functions. Concise methods fall somewhere in the middle.

@dckc dckc added the code-style defensive correctness patterns; readability thru consistency label Jun 4, 2021
@dckc
Copy link
Member

dckc commented Jun 4, 2021

@erights @dtribble @kriskowal , is auto-hardening exports consistent with the policy / norm that our JS dialect has the same semantics as standard JS?

@erights
Copy link
Member

erights commented Jun 7, 2021

@kriskowal and I talked about @dtribble 's idea for rescuing function functions by hardening exports at the loader before any importer can see them. Results inconclusive, but the best case is that it would take a lot of investigation and work that we would otherwise have been able to postpone. So, even for this best case, considering the overall balance of costs and benefits as I see it, I say we should omit function functions from Jessie.

As with this, class, and accessor properties, by omitting them from Jessie, it means we omit them from our general coding practice, covering say something like 90% of our code. But for code where we're interfacing with legacy code, legacy patterns, or shimming something we'd propose to tc39, we often leave Jessie and program in SES. So this is no more of a prohibition on ourselves than, for example, avoiding this or class. Possibly less. But still to be avoided in the normal case.

Without function, accessor syntax, or class, the only remaining function syntax controversy is concise methods vs arrow functions. Unlike function, for this one there's no hard issue (almost) forcing our hand. However, the comparative conciseness of the expression-only method:

const obj = {
  add(a, b) { return a + b; },
};

vs

const obj = {
  add: (a, b) => a + b,
};

more than makes up for the extra verbosity of the method-with-body case:

const obj = {
  add(a, b) {
    x = a;
    return a + b;
  },
};

vs

const obj = {
  add: (a, b) => {
    x = a;
    return a + b;
  },
};

@erights
Copy link
Member

erights commented Jun 7, 2021

Remember that part of the goal of the Jessie definition is that it be extremely easy to implement Jessie in any conventional sequential imperative language by a simple eval/apply style interpreter. This is why Jessie already omits yield and therefore both generators and async generators. It is one of the reasons why Jessie admits async function with only top-of-function await. This special case
* provides most of the benefits of async functions that we really need,
* avoids the semantic hazards of general async functions, and
* is still easily implementable by simple Jessie interpreters in other languages.

A Jessie implementation must then implement three kinds of functions: builtin functions, normal arrow functions, and async arrow functions. Our overall programming style does not prohibit other options when there's a reason --- typically interfacing to a legacy something --- but it would be outside our normal practice, and outside Jessie.

For code opting into Jessie rules, these non-Jessie options would indeed be prohibited. But that's part of why we allow code that does not opt into Jessie rules. And why we define the semantics of Jessie under the assumption that Jessie objects co-exist with SES objects.

@erights
Copy link
Member

erights commented Jun 7, 2021

I say we omit concise methods from Jessie. Thus, Jessie is left with only those three kinds of functions I listed: builtins, normal arrows, and async arrows (with only top-of-function await).

@erights erights changed the title Determine our method definition syntax style Determine our function/method definition syntax style Jun 7, 2021
@erights erights closed this as completed Jun 7, 2021
@dckc
Copy link
Member

dckc commented Jun 8, 2021

I added some best practice docs: Arrow Function Style

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-style defensive correctness patterns; readability thru consistency hygiene Tidying up around the house
Projects
None yet
Development

No branches or pull requests

6 participants