RTDB should use JSON types instead of any

[inlined]

Describe your environment

• Operating System version: N/A (type bug)
• Browser version: N/A (type bug)
• Firebase SDK version: 9.0.2
• Firebase Product: database

Describe the problem

The Firebase Realtime Database uses any to represent a valid JSON value, but this allows customers to accidentally use disallowed values, such as this SO question

Relevant Code:

// Notice that this handler uses `async`, so its return type is Promise<number>, not
// number. We don't handle this case.
const counterTxn = await runTransaction(firebaseRef, async node => {
  if (!node) { return 1 };
  return +node + 1;
});

This code compiles cleanly because we asked for a handler that takes an any and returns an any, but the customer is returning a Promise thanks to the async keyword and we definitely don't handle Promises in transaction.

Suggestion

We should be much more explicit about our type info. For example, if we add the following type:

type JsonValue = string | number | boolean | null | Array<JsonValue> | Record<string, JsonValue>;

We can now be much more explicit in our SDK and turn some runtime errors into compile-time errors

export interface DataSnapshot {
  exportVal(): JsonValue;
  toJSON(): JsonValue;
  val(): JsonValue;
}

export interface OnDisconnect {
  set(value: JsonValue, onComplete?: (a: Error | null) => any): Promise<void>
  setWithPriority(
    value: JsonValue,
    priority: number | string | null,
    onCompelte?: (a: Error | null) => any
  ): Promise<any>
  update(values: Record<string, JsonValue>, onComplete?: (a: Error | null) => any): Promise<any>;
}

export interface Reference extends Query {
  push(value?: JsonValue, onComplete?: (a: Error | null) => any): ThenableReference;
  set(value?: JsonValue, onComplete?: (a: Error | null) => any): Promise<any>;
  setWithPriority(
    newVal: JsonValue,
    newPriority: string | number | null,
    onComplete?: (a: Error | null) => any
  ): Promise<any>
  transaction(
    transactionUpdate: (a: JsonValue) => JsonValue | undefined,
    onComplete?: (a: Error | null, b: boolean, c: DataSnapshot | null) => any,
    applyLocally?: boolean
  ): Promise<any>;
  update(values: Record<string, JsonValue>, onComplete?: (a: Error | null) => any): Promise<any>;
}

If we used these more descriptive typings, then the Stack Overflow asker would instead have had an error:

Argument of type '() => Promise<number>' is not assignable to parameter of type '(a: JsonValue) => JsonValue | undefined'

in the above sample

[Blakeinstein]

While I agree with the suggested solution, I am lost on why the transaction cant be async.

Rejecting the promise should cause the transaction to be cancelled, instead of having to return undefined. This way you can also await async api's inside. (I had to read fs, and some other things)

[inlined]

It might be valid to accept a Promise, though that would be a feature request, not a bugfix. I would argue that rejecting the promise inside transaction should actually reject the promise returned by transaction, not just cleanly abort the transaction.

Historically the realtime database has made the transaction handler synchronous because Firebase transactions are passive transaction. The longer they take to run, the more likely it is that another client will collide with its transaction write. They were designed for small atomic operations, like implementing a counter before we added ServerValue.increment(). But I'm not on the database team anymore, so I'm not the person to accept or reject your feature request to allow async handlers.