supermacro/neverthrow

[Feature request] `safeTry` should automatically wrap returned values with `ok`

mattpocock opened this issue · 5 comments

Problem

One awkward thing about .safeTry is that it requires you to wrap everything returned in a Result.

const result = safeTry(function*() {
  return ok('foo');
});

This gets particularly awkward when safeTry is modelling a function that returns void.

const result = safeTry(function*() {
  // do stuff

  return ok(undefined); // bleugh
});

Solution

I propose that safeTry should automatically wrap returned values with ok.

const result = safeTry(function*() {
  return 'foo';
});

This would make safeTry returning void perfectly fine:

const result = safeTry(function*() {
  // do stuff
});

Existing safeTry functions returning ok or err would still be respected.

This matches the behaviour found in Effect.gen.

Since current compiler's type narrowing and control flow analysis do not work well with yield*, I think safeTry must support returning Err, not only yield*ing.
For example, yield* err(something).safeUnwrap() never returns, but the compiler doesn't treat it well.

declare const x: 1 | 2
function* g() {
  if (x === 1) {
    yield* err("XXX").safeUnwrap();
  }
  
  x // still typed as `1 | 2`
}

So, we need to use return to exit from it.

declare const x: 1 | 2
function* g() {
  if (x === 1) {
    return err("XXX");
  }
  
  x // typed as `2`, as expected
}

It is common behavior for Generator<T, never>, not specific for safeUnwrap.
https://www.typescriptlang.org/play/?#code/CYUwxgNghgTiAEYD2A7AzgF3gDwFzwEZ4AfeAJgCgKAzAVxTAwEtUAqeAcwIAoBKAbwrxh8JtXjds8ALyzhBXvEEiVATyYgIwdiBgwAylGogAqigDuMKAAduCgNxCRAXyoqpAeg+ES5Cq5p6RhYUdg4yPmURMQkpWWl5RSiVYTgMWhgUeF0DI1MLK1sHJ2EA93gvPwDQSFgEZHQsHMNjM0sbfAAeABUAPkl8bsVpXvgAcRAUXSgMJBgegBp4KYA3XV6gA

So, for now, I don't think it's a good idea to wrap the returned value automatically.

@tsuburin Agree, this makes sense as a 1-2 punch. Has this been raised as a separate issue?

@mattpocock

Has this been raised as a separate issue?

I found this.

@tsuburin I meant in this repo, since returning err() from safeTry makes sense as its own feature.

@mattpocock Oh, sorry. As far as I know, it hasn't.