Add async flag to methods and functions

[bennypowers]

While subsequent tooling could infer this from the return type, @justinfagnani rightly pointed out that there are some semantic differences, notably for error types, and perhaps for early returns too.

API tables would benefit from this flag as well:

{
  "name": "mutate",
  "async": true,
  "summary": "This resolves a single mutation according to the options specified and returns a Promise which is either resolved with the resulting data or rejected with an error.",
  "parameters": [{
    "name": "params",
    "optional": true,
    "type": "Partial<MutationOptions<Data<D>, Variables<D, V>>>",
    "summary": "All named arguments to mutate default to the element's corresponding instance property. So you can call `element.mutate()` without arguments and it will call using `element.mutation`, `element.variables`, etc. You can likewise override instance properties per-call by passing them in, e.g.\n\n```ts\nawait element.mutate({\n  fetchPolicy: 'network-only'\n  variables: {\n    ...element.variables,\n    name: 'overridden',\n  },\n});\n```\n\n| Property | Description | Type |\n| -------- | ----------- | ---- |\n| `awaitRefetchQueries` | See [awaitRefetchQueries](#awaitrefetchqueries) | `ApolloMutation#awaitRefetchQueries` |\n| `context` | See [context](../element/#context) | `Record<string, unknown>` |\n| `errorPolicy` | See [errorPolicy](../element/#errorpolicy) | `ErrorPolicy` |\n| `fetchPolicy` | See [fetchPolicy](#errorpolicy) | `FetchPolicy` |\n| `mutation` | See [mutation](#mutation) | `ApolloMutation#mutation`\n| `optimisticResponse` | See [optimisticResponse](#optimisticresponse) | `ApolloMutation#optimisticResponse`\n| `refetchQueries` | See [refetchQueries](#refetchqueries) | `ApolloMutation#refetchQueries`\n| `update` | See [updater](#updater) | `ApolloMutation#updater`\n| `variables` | See [variables](#variables) | `ApolloMutation#variables`\n"
  }],
  "return": {
    "type": "Promise<FetchResult<Data<D>>>",
    "summary": "A `FetchResult` object containing the result of the mutation and some metadata\n\n| Property | Description | Type |\n| -------- | ----------- | ---- |\n| `data` | The result of a successful execution of the mutation | `Data<D>` |\n| `errors` | included when any errors occurred as a non-empty array | `readonly GraphQLError[]` |\n| `extensions` | Reserved for adding non-standard properties | `ApolloMutation#awaitRefetchQueries` |\n| `context` | See [context](../element/#context) | `Record<string, unknown>` |\n"
  }
}

[justinfagnani]

Sorry, to be clear I actually think that we shouldn't add a special flag for async functions.

There can be very minor differences, but a function that returns a Promise and doesn't early return or throw is indistinguishable from an async function. I don't think the labelling of a function should change if the implementation does, but it's external contract doesn't.

[thepassle]

I think it kind of makes sense to support the async modifier. We also support the static modifier, so it would be consistent in line with that as well. Arguably, in some cases you could infer that a function is async by checking if its return value is a Promise, but it could be the case that the return type is not able to detected as being a promise (no jsdoc, no ts).

I can definitely see a need from developers to be able to easily document whether or not a function is async, and it would be a very small change/addition to the schema

[justinfagnani]

The static modifier completely changes the placement of the property. async should not be observably different from a function that returns a Promise. I think a tool could add that the return type is a Promise for an async function that's otherwise unannotated and uninsurable, but I'm not sure what the value of having it in the manifest.

IMO, async function foo() { return 'foo';} and function foo() { return Promise.resolve('foo'); } should be presented exactly the same in the manifest.

[thepassle]

IMO, async function foo() { return 'foo';} and function foo() { return Promise.resolve('foo'); } should be presented exactly the same in the manifest.

This is only a problem for implementers of analyzers, right?

It would suck to have documentation viewers/tools have to specifically check whether or not the return.type.text of a functionLike happens to be a promise, when we very easily could just add an async flag. Also, this may not always be inferrable, because not everybody uses a type system like jsdoc or TS. I really think thats something very important to keep in mind.

Given the following example, using plain JS:

async function foo() {
  return /* whatever, really */;
}

I would still like to be able to document this function as being async, because that completely changes how you use it. We have a native async keyword we can just use for it. I really see no good reason to block adding an async flag to the schema to be honest

[bennypowers]

For the sake of completeness:

function a(f) {
  if (typeof f !== 'function')
    throw new Error('no func');
  else
    return Promise.resolve(f());
}

async function b(f) {
  if (typeof f !== 'function')
    throw new Error('no func');
  else
    return await f();
}

assert(a(null) instanceof Promise); // throws

assert(b(null) instanceof Promise); // true
assert(b(() => 0) instanceof Promise); // true

These two functions have meaningfully and observably different behaviours, even if we'd replace the return statement in b with the one from a. Developers can, should, and do opt for a-style synchronous throws where appropriate and b-style promise rejections where it makes more sense.

[justinfagnani]

Circling back to this, I still don't fully agree that async functions are materially different from functions that return Promises.

Would this be marked as an async function?

function foo() {
  return new Promise((res) => res(something));
}

Or bar here:

async function foo(x) { ... }
function bar() { return foo(0); }

They have exactly the same qualities as an async function, including not returning or throwing synchronously.

I would say that because the behavior is the same and the difference unobservable, then it should be marked async. But then, how would that be done? Should a tool see that a function returns a Promise - and mark all Promise-returning functions as async? Maybe, but that tool should also write that the return type is Promise. At that point there's really almost no difference between async functions and Promise-returning functions in the manifest.

And most functions should not return a Promise or non-Promise, but if they do that should be documented and visible in the return type, not with async. The absence of async might only mean that the function trivially delegates to an async function, not that the behavior is any different.

[bennypowers]

Or bar here:

async function foo(x) { ... }
function bar() { return foo(0); }

but what about

function() {
  if (cond)
    throw new Error('sync')
  else 
    return new Promise((_, rej) => rej('async'))
}

async functions are different to regular functions, even if in many cases they behave the same