supermacro / neverthrow

Type-Safe Errors for JS & TypeScript
MIT License
3.86k stars 80 forks source link

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

Open mattpocock opened 1 month ago

mattpocock commented 1 month ago

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.

tsuburin commented 1 month ago

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.

mattpocock commented 1 month ago

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

tsuburin commented 1 month ago

@mattpocock

Has this been raised as a separate issue?

I found this.

mattpocock commented 1 month ago

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

tsuburin commented 1 month ago

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