neverthrow icon indicating copy to clipboard operation
neverthrow copied to clipboard

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

Open 3846masa opened this issue 1 year ago • 1 comments

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 avatar Feb 23 '24 10:02 3846masa

@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 🙏

m-shaka avatar Apr 11 '24 04:04 m-shaka

@3846masa Hi, I'm a new co-maintainer of Neverthrow.

Thank you for your great enhancement. I believe it's exactly useful.

So, could you merge master branch into this branch? We've migrated from Cricle CI to GitHub Actions, so we have to run CI again to merge

m-shaka avatar Jul 30 '24 00:07 m-shaka

🦋 Changeset detected

Latest commit: 2e1f19899800ce5e1164412c6a693cf2f1c40b20

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
neverthrow Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Jul 30 '24 01:07 changeset-bot[bot]

@m-shaka The latest main branch was imported via git rebase.

3846masa avatar Jul 30 '24 01:07 3846masa

Sorry to bother you again. Can you select Allow edits and access to secrets by maintainers option? https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork

We need a changeset file to manage the next release, so I want to add one to your branch from here. https://github.com/supermacro/neverthrow/pull/527#issuecomment-2257309273

Or, if you are familiar with changeset, you can do it yourself.

m-shaka avatar Jul 30 '24 13:07 m-shaka

To enable the Allow edits and access to secrets by maintainers option, the fork repository must be associated with a user.

This PR is a fork repository associated with an organization, so the option cannot be enabled.

Instead, I granted write permission to m-shaka for the fork repository.

3846masa avatar Jul 31 '24 02:07 3846masa