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

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

Closed
wants to merge 2 commits into from

Conversation

pnodet
Copy link

@pnodet pnodet commented Jul 4, 2024

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
Copy link
Author

pnodet commented Jul 4, 2024

Maybe this can also be helpful for issues like #525

@supermacro
Copy link
Owner

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?

Copy link

changeset-bot bot commented Aug 28, 2024

⚠️ No Changeset found

Latest commit: 674840f

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 added 2 commits August 28, 2024 18:34
bump typescript and jest dependencies
@pnodet pnodet reopened this Aug 28, 2024
@pnodet
Copy link
Author

pnodet commented Aug 28, 2024

@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
Copy link
Author

pnodet commented Aug 28, 2024

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
Copy link
Owner

@m-shaka @dmmulroy what are your thoughts?

@pnodet
Copy link
Author

pnodet commented Sep 6, 2024

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

@mattpocock
Copy link
Contributor

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

@pnodet
Copy link
Author

pnodet commented Sep 6, 2024

@mattpocock
Copy link
Contributor

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

@m-shaka
Copy link
Collaborator

m-shaka commented Sep 8, 2024

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
Copy link
Contributor

Yeah, this feels like .unsafeUnwrap with extra steps.

@supermacro
Copy link
Owner

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 supermacro closed this Nov 9, 2024
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.

4 participants