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

Suggestion: Return a discriminable object instead of a tuple array #3

Closed
Not-Jayden opened this issue Aug 15, 2024 · 25 comments
Closed

Comments

@Not-Jayden
Copy link

Not-Jayden commented Aug 15, 2024

Taking inspiration from the until package and this Twitter discussion:

Why does until return an object and not an array?

The until function used to return an array of shape [error, data] prior to 2.0.0. That has been changed, however, to get proper type-safety using discriminated union type.

Compare these two examples:

const [error, data] = await until(() => action())

if (error) {
  return null
}

// Data still has ambiguous "DataType | null" type here
// even after you've checked and handled the "error" above.
console.log(data)
const result = await until(() => action())

// At this point, "data" is ambiguous "DataType | null"
// which is correct, as you haven't checked nor handled the "error".

if (result.error) {
  return null
}

// Data is strict "DataType" since you've handled the "error" above.
console.log(result.data)

for usage in TypeScript, an object is the only nice way to be able to narrow the type.

You can of course do something like this:

const result = await until(() => action())

if (result[0]) { // result[0] is the error
  return null
}

// Here, result[1] (data) still has an ambiguous "DataType | null" type
// even after you've checked and handled the "error" above.
console.log(result[1])

but that's a lot less explicit/prone to error, which can also be said for the destructuring approach in general, as it will be quite easy to forget the correct order for data and error, especially without TypeScript being involved.

In summary, I'm proposing to change the syntax/type to something like this:

// ?= result type resolves to this:
export type Result<
  TError extends any = Error,
  TData extends any = unknown,
> =
  | {
      error: TError;
      data: null;
    }
  | { 
      error: null; 
      data: TData 
    };
 
async function getData() {
  const requestResult ?= await fetch("https://api.example.com/data");

  if (requestResult.error) {
    handleRequestError(requestResult.error);
    return;
  }

  const jsonResult ?= await requestResult.data.json();

  if (jsonResult.error) {
    handleParseError(jsonResult.error);
    return;
  }

  const validationResult ?= validationSchema.parse(jsonResult.data);

  if (validationResult.error) {
    handleValidationError(validationResult.error);
    return;
  }

  return validationResult.data;
}

EDIT: Disregard the TError generic usage, it would just always be unknown when erroring.

@arthurfiorette
Copy link
Owner

for usage in TypeScript, an object is the only nice way to be able to narrow the type.

Typescript type narrowing is not broken with this apporach.

https://github.com/arthurfiorette/tuple-it

The above package I created follows the same code but uses .tuple instead of [Symbol.result] and is type safe and can be type narrowed.

@arthurfiorette
Copy link
Owner

However moving to an object (with [Symbol.iterable] so it can still be destructured as [error, data]) would allow us to add things easier to its prototype, like .expect(), .or(), .orElse(), .ok(), .err(), .map()... that can be found on Rust's std::Result

@Not-Jayden
Copy link
Author

for usage in TypeScript, an object is the only nice way to be able to narrow the type.

Typescript type narrowing is not broken with this approach.

arthurfiorette/tuple-it

The above package I created follows the same code but uses .tuple instead of [Symbol.result] and is type safe and can be type narrowed.

Oh wow I stand corrected, my mind is blown 🤯 I've still been working under the assumptions of pre TypeScript 4.6 where destructured discriminated narrowing wasn't a thing. Though I guess it's worth considering anyway.

That said I guess I do still stand by the suggestion for it to be an object over the tuple approach.

I can definitely see the pros for a tuple, particularly:

  • It's more concise
  • It's easier to name than renaming the destructured properties
  • It forces you to think about handling the error first

I still generally favour the explicitness/foolproof-ness of having the named data and error object properties, despite those tradeoffs, but I'm also definitely less opinionated with narrowing working with destructuring.

However moving to an object (with [Symbol.iterable] so it can still be destructured as [error, data]) would allow us to add things easier to its prototype, like .expect(), .or(), .orElse(), .ok(), .err(), .map()... that can be found on Rust's std::Result

I also only just learnt it's possible to use Symbol.iterator to support both cases 🤯. While it's super cool that it's technically possible, I do think it's probably preferable to only have one way to do things. Being able to do both feels a bit too magical to me.

@DScheglov
Copy link

DScheglov commented Aug 15, 2024

UPD: this comment is something like dreaming ... Unfortunately there is no graceful way to mix typed errors and exceptions, see my later comment


I also think that it is better to use an object instead of tuple, more then type narrowing must support types like: Err<unknown> to be able express the exception throwing:

function sqrt(x: number): number {
  if (x < 0) throw new Error('SQRT: expected x to be greater or equal zero, but negative value recieved');
  return Math.sqrt(x);
}

const result ?= sqrt(x);

if ("error" in result) {
  // error is type of `unknown` here
  throw resul.error;
}

const root: number = result.data; // data is type of `number` here

But if we do something like that:

type Ok<T> = {
  [Symbol.result](): { data: T }
}

type Err<E> = {
  [Symbol.result](): { error: E }
}

type Result<E, T> = Err<E> | Ok<T>;

const ok = <T, >(data: T): Ok<T> => ({ [Symbol.result]: () => ({ data }) });
const err = <E, >(error: E): Err<E> => ({ [Symbol.result]: () => ({ error }) });

function sqrt(x: number): Result<'ERR_SQRT_NEGATIVE_VAL', number> {
  if (x < 0) return err('ERR_SQRT_NEGATIVE_VAL');
  return ok(Math.sqrt(x));
}

const result ?= sqrt(x);

if ("error" in result) {
  // error is type of `'ERR_SQRT_NEGATIVE_VAL'`
  throw resul.error;
}

const root: number = result.data; // data is type of number here

UPD:

Also it is better to avoid using in to check for error, so it is better to have an explicite descriminator:

type Ok<T> = {
  [Symbol.result](): { isError: false; data: T; }
}

type Err<E> = {
  [Symbol.result](): { isError: true; error: E; }
}

// -- snip --

if (result.isError) {
  result.error; // we know that "error" is in result, so we can access it
} else {
  result.data; // we know that "data" is in result, so we can access it
}

@garretmh
Copy link

What happens if undefined is thrown?

I'm aware that this isn't a normal occurrence and would be stupid to do, but try...catch handles this situation as expected, but as is either of the suggested result types would break down in this technically valid situation.

// try to generate a random number but undefined is thrown.
// value will not be a number even though error is undefined.

function randomNumber() {
  throw undefined
}

const [error, value] ?= randomNumber()
assert(error === undefined)
assert(value === undefined)

const { error, value }  ?= randomNumber()
assert(error === undefined)
assert(value === undefined)

It seems necessary to use a result object with a discriminator value.

Prior art: Promise.allSettled()

// try to generate a random number asynchronously but undefined is thrown.
// use `Promise.allSettled()` to get an "outcome object" for the number.
// the outcome object's status is "rejected", reason is undefined, and value is not set.

async function asyncRandomNumber() {
  throw undefined
}

async function asyncSettle(promise) {
  const [result] = await Promise.allSettled([promise])
  return result
}

const result = await asyncSettle(asyncRandomNumber())
result satisfies { status: "fulfilled", value: T } | { status: "rejected", reason: any }
assertEqual(result, { status: "rejected", reason: undefined })

@reececomo
Copy link

reececomo commented Aug 16, 2024

Note: the object overhead might be more expensive for low-level usage (webGPU, encoding, realtime applications) -- imho its not worth the trade off

edit: operative word is might

(I guess the performance-conscious alternative would be an optional unwrap with no error unwrapping at all):

const thing: FooType | undefined = try getFoo();

@JAForbes
Copy link

Just found out about this proposal, very excited! I'm also really glad to see others were thinking about future potential for a Result/Maybe/Option type.

I think ?= should return a Result object that coerces to a tuple if the left hand side is destructuring it as an array. This could be achieved with Symbol.iterator at runtime (example). For now the Result object would have no methods.

I think the Result object should have some kind of discriminated tag to help with type narrowing when interacting with object manually. It could be a Symbol or a read only string. This allows us to capture if the source of the result threw an error even if the Error path itself was the value null.

You can model this interface in Typescript like this:

type Result<Error, Data> = 
  (
    [Error, null] 
    &
    { tag: 'Error'
    , value: Error
    }
  )
  |
  (
    [null, Data]
    &
    { tag: 'Data'
    , value: Data
    }
  )

This means we get to keep the sugar but also have an extensible interface for future proposals/additions.

@mindplay-dk
Copy link

mindplay-dk commented Aug 16, 2024

I came here to post the same issue, but...

Oh wow I stand corrected, my mind is blown 🤯 I've still been working under the assumptions of pre TypeScript 4.6 where destructured discriminated narrowing wasn't a thing. Though I guess it's worth considering anyway.

TIL 😄

That said I guess I do still stand by the suggestion for it to be an object over the tuple approach.

I do not. 🤔

Destructuring a tuple makes it easy and natural (and necessary) to assign local names to both values.

This becomes important when making multiple calls to functions that may return errors, in the same local scope.

With objects, it's tempting to just name the return values as they're named in the returned object - but this falls apart of you need to make more than one call. You have to rename them then, which is rather clunky with object destructuring.

const { error: error1, data: value1 } ?= function1();
const { error: error2, data: value2 } ?= function2();

as opposed to:

const [error1, value1] ?= function1();
const [error2, value2] ?= function2();

Tuple return types encourage you to destructure and rename the values - it's the idiomatic "multiple return values" pattern in JS, and multiple return values is exactly what we have here.

With the object approach, it's likely some people will prefer const result1 ?= function1() and accessing the properties, rather than destructuring, which invites inconsistencies in code.

With tuples, no one wants to write result1[0], so they will learn to consistently destructure and assign meaningful names.

I believe this is part of the reason why e.g. Solid and React uses tuple returns when creating getters and setters for state management - we're expecting to have several of these pairs in the same scope, it's the idiomatic multiple return value pattern in JS, and the returned values should be named at the call site, just as any singular return value from any other function call gets named at the call site.

@DScheglov
Copy link

@JAForbes despite of my previous comment, it is not possible to include typed errors in this proposal because the function that returns a typed error can also throw.

function stringify(value: unknown): Result<'ERR_FUNCTION_UNSERIALIZABLE', string> {
  if (typeof value === 'function') return err('ERR_FUNCTION_UNSERIALIZABLE');
  return JSON.stringify(value);
}

const x: { x?: unknown } = {};
x.x = x;

const result ?= stringify(x);

Here we get an error that is definitelly is not 'ERR_FUNCTION_UNSERIALIZABLE';

So, I guess it is senseless to try type errors in this proposal, that is aimed to handle exceptions gracefully.

Sure, we can try to do the following:

type Result<Error, Data> = 
  (
    [Error, null] 
    &
    { tag: 'Error'
    , value: Error
    }
  )
  |
  (
    [null, Data]
    &
    { tag: 'Data'
    , value: Data
    }
  )
  |
  (
    [unknown, Data]
    &
    { tag: 'Exception'
    , value: unknown
    }
  )

But it doesn't seem to simplify error handling both in TS and in JS

@JAForbes
Copy link

But this proposal would absorb the throw so in this context its fine right?

@JAForbes
Copy link

JAForbes commented Aug 16, 2024

To be a bit more clear, the return type of the function isn't a Result, but the result of the expression of ?= in conjunction with the function execution has a type of Result<Error, Data>.

@Not-Jayden
Copy link
Author

That said I guess I do still stand by the suggestion for it to be an object over the tuple approach.

I do not. 🤔

Destructuring a tuple makes it easy and natural (and necessary) to assign local names to both values.

This becomes important when making multiple calls to functions that may return errors, in the same local scope.

With objects, it's tempting to just name the return values as they're named in the returned object - but this falls apart of you need to make more than one call. You have to rename them then, which is rather clunky with object destructuring.

const { error: error1, data: value1 } ?= function1();
const { error: error2, data: value2 } ?= function2();

as opposed to:

const [error1, value1] ?= function1();
const [error2, value2] ?= function2();

Tuple return types encourage you to destructure and rename the values - it's the idiomatic "multiple return values" pattern in JS, and multiple return values is exactly what we have here.

With the object approach, it's likely some people will prefer const result1 ?= function1() and accessing the properties, rather than destructuring, which invites inconsistencies in code.


To clarify, my opinion here is a lot more indifferent than it was originally after learning destructured type discrimination works.


With tuples, no one wants to write result1[0], so they will learn to consistently destructure and assign meaningful names.

I believe this is part of the reason why e.g. Solid and React uses tuple returns when creating getters and setters for state management - we're expecting to have several of these pairs in the same scope, it's the idiomatic multiple return value pattern in JS, and the returned values should be named at the call site, just as any singular return value from any other function call gets named at the call site.


I'd agree it's a relatively small learning curve, but still larger than having named properties.

To your point that "no one wants to write result1[0]", this could also be seen as an argument in favour of the object approach if you do just want to assign it to a single result1 variable, as you can instead just access them as result1.error and result1.data without destructuring.

I'm still keen to hear more perspectives as I think it's worth giving careful consideration, despite my current sway to neutrality 😅

@smeijer
Copy link

smeijer commented Aug 16, 2024

I don't think that TypeScript should be the deciding factor in how JS should evolve. TypeScript should just evolve to match the language, not the other way around. That said,

  • Tuples can be typed just fine
  • Thrown errors are always of type unknown, as we can throw literally anything
  • [fetchUserError, fetchedUser] is much cleaner to change names than { error: fetchUserError, data: fetchedUser }. Imagine doing this literally everywhere in your code.
const { error: errorOne, data: user } ?= await fetchUser('one');
if (errorOne) // handle

const { error: errorTwo, data: updated } ?= await updateUser('one', patch);
if (errorTwo) // handle

// vs

const [errorOne, user] ?= await fetchuser('one');
if (errorOne) // handle

const [errorTwo, updated] ?= await updateuser('one', patch);
if (errorTwo) // handle

That would get annoying real fast.

@JAForbes
Copy link

To summarise, what I was saying before I think we can have our cake and eat it too, just have an object with a [Symbol.iterator] that evaluates to the tuple. You can use either syntax, tuple or object.

@DScheglov
Copy link

@JAForbes

To be a bit more clear, the return type of the function isn't a Result, but the result of the expression of ?= in conjunction with the function execution has a type of Result<Error, Data>.

Unfortunately, we cannot have Result<Error, Data> type ...
We will have Result<Data>, so errors will always be of unknown type.

But from the TS point of view it is better to have an object in any case:

interface Result<T> {
  [Symbol.result](): 
    | { isError: false, value: T }
    | { isError: true, value: unknown }
}

@mindplay-dk
Copy link

mindplay-dk commented Aug 16, 2024

Unfortunately, we cannot have Result<Error, Data> type ...
We will have Result<Data>, so errors will always be of unknown type.

we will rue the day when TS finally solves this. 😅

(probably never though?)

the interface Result {
Symbol.result:
| { isError: false, value: T }
| { isError: true, value: unknown }
}

btw your type example doesn't look right? shouldn't it be something like this?

interface Result<T> {
  [Symbol.result](): 
    | { error: undefined, value: T }
    | { error: unknown, value: unknown }
}

although I'm not sure error would work a discriminator for narrowing the type then, since unknown includes undefined? or does it? I'm not sure we have type in TS that means either "not null" or "not undefined"?

@DScheglov
Copy link

@mindplay-dk

btw your type example doesn't look right? shouldn't it be something like this?

interface Result<T> {
 [Symbol.result](): 
   | { error: undefined, value: T }
   | { error: unknown, value: unknown }
}

Exactly error of type unknown couldn't be a discriminator, but currently TS has useUnknownInCatchVariables set to true in strict mode, so all exceptions are treated as unknown.

That's why it is better to have an explicit descriminator like I proposed.

The Result type could be the following:

interface Result<T> {
  [Symbol.result]():
    | { error: {}, value: undefined } // {} means any non-nullable value
    | { error: undefined | null, value: T }
}

or the following

interface Result<T> {
  [Symbol.result]():
    | [{}, undefined]
    | [undefined | null, T]
}

But even in this case we have an issue with type narrowing:

  1. TS Playground: Non-destructured object
  2. TS Playground: Descructured object
  3. TS Playground: Non-destructured Tuple
  4. TS Playground: destructued tuple

But having the:

interface Result<T> {
  [Symbol.result]():
    | { isError: false, value: T }
    | { isError: true, value: unknown }
}

Everything works as expected see TS Playground: Explicit Descriminator

@DScheglov
Copy link

@arthurfiorette

About the tuple-it library. It works if error is of type Error or something like that.
The library doesn't work if error is {} or unknown:

import 'tuple-it/register'

type WorkData = { id: string; title: string; }

async function work(promise: Promise<WorkData>) {
  const [error, data] = await promise.tuple<unknown>()
  // const [error, data] = await promise.tuple<{}>()

  if (error) {
    console.log('Operation failed!')
    return false
  }

  /**
   * Type 'WorkData | undefined' is not assignable to type 'WorkData'.
   * Type 'undefined' is not assignable to type 'WorkData'.ts(2322)
   */
  const value: WorkData = data;

  console.log('Operation succeeded!')
  return true
}

Typescript in strict mode treats exceptions as unknown by default, so semantically to use something else doesn't seem to be correct.

@arthurfiorette
Copy link
Owner

arthurfiorette commented Aug 16, 2024

Hmm, nice catch with the typings @DScheglov!

arthurfiorette/tuple-it#23

However, if the error throw is not an error, the library catches it:

https://github.com/arthurfiorette/tuple-it/blob/32dc86108ae76e5c118bedf320c574c6ac8d6e41/index.js#L13

@Not-Jayden
Copy link
Author

Not-Jayden commented Aug 16, 2024

@arthurfiorette

About the tuple-it library. It works if error is of type Error or something like that. The library doesn't work if error is {} or unknown:

import 'tuple-it/register'

type WorkData = { id: string; title: string; }

async function work(promise: Promise<WorkData>) {
  const [error, data] = await promise.tuple<unknown>()
  // const [error, data] = await promise.tuple<{}>()

  if (error) {
    console.log('Operation failed!')
    return false
  }

  /**
   * Type 'WorkData | undefined' is not assignable to type 'WorkData'.
   * Type 'undefined' is not assignable to type 'WorkData'.ts(2322)
   */
  const value: WorkData = data;

  console.log('Operation succeeded!')
  return true
}

Typescript in strict mode treats exceptions as unknown by default, so semantically to use something else doesn't seem to be correct.

I knew I'd ran into issues with this quirk recently, just couldn't recall how.

I wonder if this necessitates the need to include an additional success property/element (or ok to steal from Rust?) to be able to guarantee the discrimination works if the data or error values can be nullish.

e.g.

type Result<TData = unknown> = 
    { success: true, data: TData, error: null } | 
    { success: false, data: null, error: unknown };


function handleResult({success, error, data}: Result<"Oops!" | undefined, "Success!" | undefined>) {
    if (success) {
        console.log('Success:', data);
    } else {
        console.log('Error:', error);
    }
}

or even consolidate data and error into a single value property/element:

type Result<TData = unknown> = 
    { success: true, value: TData } | 
    { success: false, value: unknown };

function handleResult({ success, value }: Result<string | undefined, TypeError | undefined>) {
    if (success) {
        console.log('Success:', value);
    } else {
        console.log('Error:', value);
    }
}

@garretmh
Copy link

garretmh commented Aug 16, 2024

I recommend looking in to prior art in JavaScript.

From Promise<T>:

Promise rejected values are not typed because promises catch thrown values. This proposal has the same issue.

From Promise.allSettled():

This takes promises and converts them to "outcome objects". Using it on a single promise is analogous to an asynchronous version of this proposal.

const [outcome] = await Promise.allSettled([promise])

outcome satisfies { status: "resolved", value: T } | { status: "rejected", reason: unknown }

From Response:

Responses have an ok property with a Boolean value that discriminates whether the HTTP Status Code is ok (i.e. in the 2XX range).

@DScheglov
Copy link

DScheglov commented Aug 21, 2024

@arthurfiorette

As I wrote in one of other comments in TS we need a way to have a type errors.
Using the tuples for the result assumes at least this value is non nullable. That actually doesn't match to a EcmaScript statement: any value could be thrown.

TS just highlights that.

But yes, from the JS point of view there is no sense to have a typed errors but using the tuple allows to simplify naming.

I see something like a compromise between the TS and JS needs:

  1. The result type must be explicitly return from the function, and this type is the following:
    type Result<T, E> = {
      [Symbol.result]():
        | { isError: true, error: E }
        | { isError: false, value: T }
    }
  2. The operator ?= implicitly wraps the expression in try .. catch and returns the ErrValueTuple, that is the following:
    type ErrValueTuple<T> =
      | readonly [NonNullable<unknown>, never]
      | readonly [NonNullable<unknown>, undefined]
      | readonly [null, T]
  3. The operator ?= checks if the expression result is type of Result<T, E> it also returns the ErrValueTuple<T>
  4. The operator ?= checks if caught, rejected or E-value is nullish, then it wraps it to some specific error class to desribe the issue and returns the correspondent ErrValueTuple
  5. The operator try? applied to Result<T, E>:
    4.1. returns T or
    4.2. exists from the current function with Result<never, E>
  6. The operator try! applied to Result<T, E>:
    5.1. return T or
    5.2. throws a E

This approach is more consistent:

  1. it matches the EcmaScript statement
  2. it doesn't prevent the Symbol.result to support the typed errors
  3. we have visible reasons to use ?= to get ErrValueTuple and try?! to apply to Result<E, void> values (meaing the cases when we don't need the value)

@arthurfiorette
Copy link
Owner

@DScheglov we don't create JS to adapt to current TS code. This is wrong and i refuse to do it as well any other member of TC39.

We first figure the best way to have it in JS and then TS adapts itself to best handle the new behavior.

@arthurfiorette
Copy link
Owner

Closed by #30

@DScheglov
Copy link

@arthurfiorette

we don't create JS to adapt to current TS code
Who does speak abou adopting current TS code?

The proposal introduces Symbol.result that just it become a part of ES brings the problem to TS.
So, it makes sense to consider the typing aspects on the early phases.

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

No branches or pull requests

8 participants