neverthrow icon indicating copy to clipboard operation
neverthrow copied to clipboard

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

Open pnodet opened this issue 1 year ago • 12 comments

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 avatar Jul 04 '24 10:07 pnodet

Maybe this can also be helpful for issues like #525

pnodet avatar Jul 04 '24 10:07 pnodet

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?

supermacro avatar Aug 28 '24 15:08 supermacro

⚠️ 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

changeset-bot[bot] avatar Aug 28 '24 16:08 changeset-bot[bot]

@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 avatar Aug 28 '24 16:08 pnodet

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!

pnodet avatar Aug 28 '24 16:08 pnodet

@m-shaka @dmmulroy what are your thoughts?

supermacro avatar Sep 06 '24 19:09 supermacro

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

pnodet avatar Sep 06 '24 20:09 pnodet

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

mattpocock avatar Sep 06 '24 20:09 mattpocock

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

pnodet avatar Sep 06 '24 20:09 pnodet

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

mattpocock avatar Sep 07 '24 07:09 mattpocock

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

m-shaka avatar Sep 08 '24 08:09 m-shaka

Yeah, this feels like .unsafeUnwrap with extra steps.

mattpocock avatar Sep 08 '24 09:09 mattpocock

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

supermacro avatar Nov 09 '24 17:11 supermacro