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

Bring whenState in line with spec #1640

Merged
merged 5 commits into from
Mar 6, 2024

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Feb 27, 2024

Updates the whenState methods to conform to ably/specification#183. See commit messages for more details.

Resolves #1597.
https://ably.atlassian.net/browse/ECO-4662

@github-actions github-actions bot temporarily deployed to staging/pull/1640/features February 27, 2024 13:33 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/bundle-report February 27, 2024 13:34 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/typedoc February 27, 2024 13:34 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/features February 28, 2024 11:46 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/bundle-report February 28, 2024 11:47 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/typedoc February 28, 2024 11:47 Inactive
@lawrence-forooghian lawrence-forooghian changed the title 1597 implement when state spec Bring whenState in line with spec Feb 28, 2024
@lawrence-forooghian lawrence-forooghian marked this pull request as ready for review February 28, 2024 12:38
@VeskeR VeskeR mentioned this pull request Mar 1, 2024
10 tasks
src/common/lib/util/eventemitter.ts Outdated Show resolved Hide resolved
src/common/lib/util/eventemitter.ts Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to staging/pull/1640/features March 6, 2024 13:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/bundle-report March 6, 2024 13:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/typedoc March 6, 2024 13:20 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/features March 6, 2024 13:22 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/bundle-report March 6, 2024 13:22 Inactive
@github-actions github-actions bot temporarily deployed to staging/pull/1640/typedoc March 6, 2024 13:23 Inactive
I think that onceOrIfAfter was the original name for whenState, but
5d05530 forgot to update the test name accordingly.
Missed this in 7c465d6.

I’ve had to remove the assertion from channelattachWhenState which
checks that whenState calls its listener immediately, because there’s no
easy way to check for this (because promise listeners are always invoked
asynchronously and there’s no way to synchronously check whether a
promise is pending).
Until spec commit d5e8fc6, these methods were exclusive to ably-js and
were not tested. This commit brings the ably-js implementations in line
with the aforementioned spec commit and tests that they conform to the
spec.

Documentation based on sdk-api-reference commit 4a06b46.

Note the addition of the async/await adaptations of closeAndFinish and
monitorConnection. These new tests are the first realtime tests that
we’ve written on the v2 branch — i.e. since we dropped support for the
callbacks API, and so I’ve written them using async/await properly
instead of the whenPromiseSettles stopgap solution of 43a2d1d. Hence the
new helper methods.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants