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

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

Open
mattpocock opened this issue Sep 7, 2024 · 5 comments

Comments

@mattpocock
Copy link
Contributor

mattpocock commented Sep 7, 2024

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
Copy link
Contributor

tsuburin commented Sep 28, 2024

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
Copy link
Contributor Author

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

@tsuburin
Copy link
Contributor

@mattpocock

Has this been raised as a separate issue?

I found this.

@mattpocock
Copy link
Contributor Author

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

@tsuburin
Copy link
Contributor

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

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

2 participants