Skip to content

Add overloads to InvokeAsync with a return value#1305

Merged
linkdotnet merged 3 commits intobUnit-dev:mainfrom
Jcparkyn:invokeasync-overloads
Dec 10, 2023
Merged

Add overloads to InvokeAsync with a return value#1305
linkdotnet merged 3 commits intobUnit-dev:mainfrom
Jcparkyn:invokeasync-overloads

Conversation

@Jcparkyn
Copy link
Copy Markdown
Contributor

@Jcparkyn Jcparkyn commented Dec 1, 2023

Closes #1296

Pull request description

Didn't include tests since there's essentially no logic changes here and the second overload already doesn't have any, but I can add them if wanted.

PR meta checklist

  • Pull request is targeted at main branch for code
    or targeted at stable branch for documentation that is live on bunit.dev.
  • Pull request is linked to all related issues, if any.
  • I have read the CONTRIBUTING.md document.

Code PR specific checklist

  • My code follows the code style of this project and AspNetCore coding guidelines.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • I have updated the appropriate sub section in the CHANGELOG.md.
  • I have added, updated or removed tests to according to my changes.
    • All tests passed.

@Jcparkyn
Copy link
Copy Markdown
Contributor Author

Jcparkyn commented Dec 1, 2023

@dotnet-policy-service agree

@linkdotnet
Copy link
Copy Markdown
Collaborator

Thanks for filing the PR @Jcparkyn !
It might be worth extending the documentation, shedding some light on the new use cases you enabled.

@linkdotnet
Copy link
Copy Markdown
Collaborator

Hey @Jcparkyn,

if you have any documentation locally - just push it. The build server can take care of it.
Otherwise, we can also flatten out some rough edges.

@linkdotnet
Copy link
Copy Markdown
Collaborator

@Jcparkyn the docs should be totally fixed.

@Jcparkyn
Copy link
Copy Markdown
Contributor Author

Jcparkyn commented Dec 9, 2023

Thanks @linkdotnet, just hadn't got around to writing it yet 😀.

Do you think it's worth adding a full example, or just a sentence or two? I'd think that most people who need this feature would discover it from the existing docs - maybe not for the Task use case though, since that's a bit more subtle.

Edit: Another thing I never figured out is whether it's necessary (or best practice) to always await calls to InvokeAsync? I would've thought so, but the example in the docs doesn't?

@Jcparkyn Jcparkyn force-pushed the invokeasync-overloads branch from 781133a to 602f664 Compare December 9, 2023 03:33
@Jcparkyn
Copy link
Copy Markdown
Contributor Author

Jcparkyn commented Dec 9, 2023

Added examples for both use cases, feel free to shorten it if you think they're not necessary. Also fixed a couple of incorrect line numbers in the docs.

By the way, the docs build and run properly now, but I think the readme instructions need updating again to use the docfx CLI.

Comment thread docs/site/docs/interaction/trigger-renders.md
Comment thread docs/site/docs/interaction/trigger-renders.md Outdated
@linkdotnet
Copy link
Copy Markdown
Collaborator

linkdotnet commented Dec 9, 2023

By the way, the docs build and run properly now, but I think the readme instructions need updating again to use the docfx CLI.

Yes, you are right, and we missed updating this again :D
I will do that later today.!

Edit: Updates the README.MD. Thnaks for the hint @Jcparkyn

@linkdotnet
Copy link
Copy Markdown
Collaborator

LGTM! Thanks for your efforts and time @Jcparkyn

@linkdotnet linkdotnet merged commit 08ea910 into bUnit-dev:main Dec 10, 2023
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.

IRenderedComponent.InvokeAsync should support return values

2 participants