Reqnroll icon indicating copy to clipboard operation
Reqnroll copied to clipboard

Add support for IAsyncDisposable in managed objects

Open Code-Grump opened this issue 3 months ago • 5 comments

🤔 What's changed?

Add support for calling DisposeAsync on objects managed by ObjectContainer. This supplements the existing Dispose support:

  • If an object implements IAsyncDisposable, its DisposeAsync method will be called when the container is disposed
  • If an object implements IDisposeable its Dispose method will continue to be called unless it was cleaned up via DisposeAsync

⚡️ What's your motivation?

I am tired of trying to manually clean up IAsyncDisposable objects myself in my test suites. 😅

🏷️ What kind of change is this?

  • :zap: New feature (non-breaking change which adds new behaviour)
  • :boom: Breaking change (incompatible changes to the API)

📋 Checklist:

  • [x] I've changed the behaviour of the code
    • [x] I have added/updated tests to cover my changes.
  • [x] My change requires a change to the documentation.
    • [x] I have updated the documentation accordingly.
  • [x] Users should know about my change
    • [x] I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

Code-Grump avatar Sep 01 '25 21:09 Code-Grump

Because this is breaking, we have to wait for the v4 line to merge this, right?

gasparnagy avatar Sep 04 '25 10:09 gasparnagy

So this is an interesting position we find ourselves in. This change is unlikely to be breaking for general consumers - they just get a better lifecycle experience if they want to add it. Tests should continue to run without changes.

For direct users of the API though, there are breaking changes to contracts: all the sync calls moving to async. If our position is that any API changes have to be backwards compatible or require a major version, this has to be a major version.

Code-Grump avatar Sep 04 '25 11:09 Code-Grump

For direct users of the API though, there are breaking changes to contracts: all the sync calls moving to async. If our position is that any API changes have to be backwards compatible or require a major version, this has to be a major version.

Yes. The direct users we need to take care of are the plugins. It's very hard to track the plugin compatibility if we change the API that the plugins use within major versions.

BUT: we don't have to wait 6 month to release a new major version. I think there is absolutely no problem releasing a new major version even every month. The v3 was maybe a bit special because the formatters were a single huge chunk.

So changes like this could be accumulated and maybe early October we could release v4.

gasparnagy avatar Sep 04 '25 12:09 gasparnagy

Repointed this PR to the branch it's dependant off.

Code-Grump avatar Sep 13 '25 15:09 Code-Grump

Thank you for your effort. I am looking forward to using this feature. On a few dependencies we currently wrap DisposeAsync in a DisposeAsync().GetAwaiter().GetResult()-call in Dispose(), which is not very efficient.

borgiman avatar Oct 13 '25 17:10 borgiman