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

Update the WritableIterable behavior #724

Merged
merged 12 commits into from
Jul 31, 2023
Merged

Update the WritableIterable behavior #724

merged 12 commits into from
Jul 31, 2023

Conversation

srikrsna-buf
Copy link
Member

@srikrsna-buf srikrsna-buf commented Jul 20, 2023

Update the WritableIterable behavior to improve the developer experience. Writes behave as if they are synchronous and resolve when there is one more next call (indicating the previous one is processed) or rejected when consumers call throw.

Comment on lines +269 to +274
.then((result) =>
expect(result).toEqual({ done: true, value: undefined })
)
.catch(() =>
fail("expected successful done result but unexpectedly rejected")
);
Copy link
Member Author

@srikrsna-buf srikrsna-buf Jul 20, 2023

Choose a reason for hiding this comment

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

Changed the behavior for next to return {done: true} when the writer is closed either due to an error or with call toclose. This is what I think should be the behavior of Iterables outlined here, based on these two paragraphs:

If an iterator returns a result with done: true, any subsequent calls to next() are expected to return done: true as well, although this is not enforced on the language level.

throw(exception) Optional
A function that accepts zero or one argument and returns an object conforming to the IteratorResult interface, typically with done equal to true. Calling this method tells the iterator that the caller detects an error condition, and exception is typically an Error instance.

@srikrsna-buf srikrsna-buf marked this pull request as ready for review July 20, 2023 08:05
Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

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

👍 Thank you for the work on this.

Once we are sure the DX is solid, we should export the writable from @bufbuild/connect.

@srikrsna-buf srikrsna-buf merged commit 1fe3501 into main Jul 31, 2023
@srikrsna-buf srikrsna-buf deleted the TCN-2005 branch July 31, 2023 14:50
@timostamm timostamm mentioned this pull request Aug 17, 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.

2 participants