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

fix: change type definitions to make inferring types of safeTry more strict #527

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

3846masa
Copy link

In the current safeTry function, TypeScript cannot infer the type if the yielded Err and the Err returned by the generator function are different types.

interface FirstYieldError {
  name: 'FirstYieldError';
}
interface ReturnError {
  name: 'ReturnError';
}

// cannot infer the return type, but the return type is expected to be `Result<string, FirstYieldError | ReturnError>`
const result = safeTry(function*() {
  yield* ok<null, FirstYieldError>(null).safeUnwrap();
  return ok<string, ReturnError>('string');
});

In this PR, the type definition file will be changed so that TypeScript can correctly infer the return value of the safeTry function.

interface FirstYieldError {
  name: 'FirstYieldError';
}
interface ReturnError {
  name: 'ReturnError';
}

{
  // `Result<string, FirstYieldError | ReturnError>`
  const result = safeTry(function*() {
    yield* ok<null, FirstYieldError>(null).safeUnwrap();
    return ok<string, ReturnError>('string');
  });
}

{
  // As before, Generics accepts Value (T) and Error (E) type.
  const result = safeTry<string, FirstYieldError | ReturnError>(function*() {
    yield* ok<null, FirstYieldError>(null).safeUnwrap();
    return ok<string, ReturnError>('string');
  });
}

@3846masa 3846masa force-pushed the fix/strict-type-inference-for-safetry branch from 4ae10b2 to e322a92 Compare February 29, 2024 10:07
@m-shaka
Copy link

m-shaka commented Apr 11, 2024

@supermacro
Hi!
I'm not the author, but I think this PR made a great improvement on DX. I would be really happy if you could check it 🙏

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

Successfully merging this pull request may close these issues.

None yet

2 participants