supermacro / neverthrow

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

WIP Proposal: adding [Symbol.dispose] to check unhandled Result #552

Closed pnodet closed 1 week ago

pnodet commented 4 months ago

Typescript 5.2 introduced the using keyword. One thing that I find hard with neverthrow is checking for unhandled result. The eslint plugin brings helpful linter errors but I wanted to try implementing [Symbol.dispose] and see if it can help the DX.

pnodet commented 4 months ago

Maybe this can also be helpful for issues like #525

supermacro commented 2 months ago

I think having improvements to signal that you have unhandled / swalled errors in your stack would be awesome.

Are you able to provide examples of how this API would address this problem?

changeset-bot[bot] commented 2 months ago

⚠️ No Changeset found

Latest commit: 674840f4fb43a306113d68382286eccbd630b805

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

pnodet commented 2 months ago

@supermacro Sorry, rebased this branch with upstream commits and GitHub closed it automatically.

This idea behind this is to leverage the Symbol.dispose new functionality.

const someFunc = () => {
  using anExampleResult = aFunctionReturningAResult();
}

for example this would throw an UnhandledResultError because at the end of someFunc the anExampleResult result has not been unwrap or match or another neverthrow result's method

pnodet commented 2 months ago

tbh this was more a wild idea that a real proposal bc I see several issues (people forgetting the using keyword, having error thrown in production…)

I'd be happy to improve this or keep working on it if you or others think it would be a great addition tho!

supermacro commented 2 months ago

@m-shaka @dmmulroy what are your thoughts?

pnodet commented 2 months ago

Also curious to know @mattpocock if you have thoughts on how to improve this or examples of more complex Symbol.dispose use cases

mattpocock commented 2 months ago

Don't see how using differs from just const here - using is meant for tidying up a resource when it leaves scope.

pnodet commented 2 months ago

With using we can leverage Symbol.dispose to do something like :

https://github.com/supermacro/neverthrow/pull/552/files#diff-fe54c0b0441ef7719a22666a2bb171f2db9e99c0ed52dd8fec96a9882e7c842dR405-R409

Screenshot 2024-09-06 at 22 10 26
mattpocock commented 2 months ago

@pnodet Gotcha, that makes sense. Feels possible, but I'm not sure how intuitive the API will feel to most users.

m-shaka commented 2 months ago

I don't know if it's a use case of using. using can be used to manage "resource" with a specific lifetime. It sounds strongly connected to side-effects, which neverthrow has nothing to do with

Resource — An object with a specific lifetime, at the end of which either a lifetime-sensitive operation should be performed or a non-garbage-collected reference (such as a file handle, socket, etc.) should be closed or freed.

https://github.com/tc39/proposal-explicit-resource-management

mattpocock commented 2 months ago

Yeah, this feels like .unsafeUnwrap with extra steps.

supermacro commented 1 week ago

Identifying if a result is used or not should be done statically (prob through a linter) rather than at runtime, no? Closing for now