supermacro / neverthrow

Type-Safe Errors for JS & TypeScript
MIT License
4.06k stars 85 forks source link

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

Closed 3846masa closed 3 months ago

3846masa commented 9 months ago

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');
  });
}
m-shaka commented 7 months ago

@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 commented 3 months ago

@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

changeset-bot[bot] commented 3 months ago

🦋 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

3846masa commented 3 months ago

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

m-shaka commented 3 months ago

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.

3846masa commented 3 months ago

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.