neverthrow icon indicating copy to clipboard operation
neverthrow copied to clipboard

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

Open mattpocock opened this issue 1 year ago • 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.

mattpocock avatar Sep 07 '24 07:09 mattpocock

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 avatar Sep 28 '24 02:09 tsuburin

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

mattpocock avatar Sep 28 '24 08:09 mattpocock

@mattpocock

Has this been raised as a separate issue?

I found this.

tsuburin avatar Sep 28 '24 09:09 tsuburin

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

mattpocock avatar Sep 28 '24 09:09 mattpocock

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

tsuburin avatar Sep 28 '24 09:09 tsuburin