Skip to content

Async naming updates and doc fixes #2504

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Apr 17, 2025

After implementing foreign futures these in JS, I wanted to make some updates. The FFI is staying the same, but some names have been changed.

UniffiForeignFutureFree is now named UniffiForeignFutureDroppedCallback and UniffiForeignFuture is now UniffiForeignFutureDroppedCallbackStruct. This reflects the fact that we don't really need to use these to manage the future, the real reason they're here nowadays is to support cancellation. For languages like JS, where we can't implement cancellation, we can just ignore the param.

Updated some of the code that deals with this to treat it like the out parameter it is, rather than a return value. I think that was leftover from previous implementations.

There's also one change for RustFuture: RustFuturePoll::MaybeReady is now RustFuturePoll::Wake. "MaybeReady" was always a confusing/ambiguous name. "Wake" seems reasonable, since it's what the foreign bindings should do and reflects how the Rust futures code works.

Updated some outdated docstrings.

@bendk bendk requested a review from a team as a code owner April 17, 2025 16:08
@bendk bendk requested review from jeddai and removed request for a team April 17, 2025 16:08
Copy link
Member

@mhammond mhammond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically a breaking change, so we'll land after decide to force one.

@bendk bendk force-pushed the push-monqzlyovvpo branch from f0d2f54 to 174b379 Compare May 2, 2025 19:15
After implementing foreign futures these in JS, I wanted to make some
updates.  The FFI is staying the same, but some names have been changed.

`UniffiForeignFutureFree` is now named `UniffiForeignFutureDroppedCallback` and
`UniffiForeignFuture` is now `UniffiForeignFutureDroppedCallbackStruct`.
This reflects the fact that we don't really need to use these to manage
the future, the real reason they're here nowadays is to support
cancellation. For languages like JS, where we can't implement
cancellation, we can just ignore these structs and the param.

Updated some of the code that deals with this to treat it like the out
parameter it is, rather than a return value.  I think that was leftover
from previous implementations.

There's also one change for RustFuture: `RustFuturePoll::MaybeReady` is
now `RustFuturePoll::Wake`.  "MaybeReady" was always a
confusing/ambiguous name.  "Wake" seems reasonable, since it's what the
foreign bindings should do and reflects how the Rust futures code works.

Updated some outdated docstrings.
@bendk bendk force-pushed the push-monqzlyovvpo branch from 174b379 to 41df0c3 Compare May 2, 2025 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants