Should intoTuple use undefined instead of null?

[traverse1984]

From this comment: https://github.com/traverse1984/oxide.ts/pull/11#issuecomment-1294157267.

On the one hand, this pattern looks nice and simplifies the case where you want to introduce default values. On the other hand, given that it may sometimes be necessary to be more explicit to correctly narrow the types, it would require somewhat unsightly uses of if (err === undefined) or using ==, neither of which I am a huge fan of.

[thelinuxlich]

But undefined returns true for the same truthy comparisons of null

[thelinuxlich]

I have a helper for this style of error handling, similar to tiny-invariant:

function throwIf<T>(
  err: T | undefined,
  message?: string
): asserts err is undefined {
  if (err !== undefined) throw new Error(message ?? err?.toString())
}

// then on common usage:
const [err, res] = await generateResult().intoTuple()
throwIf(err) // if err is undefined res gets narrowed to T instead of T | undefined
doSomething(res)

[traverse1984]

But undefined returns true for the same truthy comparisons of null

Yeah - I think I'm just suffering from visual issues. I don't like how x === undefined looks and I don't want to encourage truthy comparisons, not least because I'm so used to writing === that I'm sure to mess that up on a number of occasions. However, I'm willing to concede this might be (probably is) a non-issue.

I have a helper for this style of error handling, similar to tiny-invariant:

I think your helper and that pattern could indicate the need for a better expect API - eg:

class Result<T, E> {
   // ...
   expect(this: Result<T, E>, msg: string): T;
   expect<E extends Error>(this: Result<T, E>, msg?: string): T;
   expect(this: Result<T, E>, msg?: string): T {
      if (this[T]) {
         return this[Val] as T;
      } else if (msg === undefined) {
         throw this[Val];
      } else {
         throw new Error(msg);
      }
   }
}

const res = (await generateResult()).expect();
doSomething(res);

[thelinuxlich]

You gave me a good idea, but this would be a optional behavior because sometimes you want to do something else instead of throwing the err

[traverse1984]

Will implement this in next release, see #15.