-
Notifications
You must be signed in to change notification settings - Fork 25
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
Fix 'queueMessages' expected behaviour #1005
Conversation
46a54ba
to
bc662e1
Compare
Breadcrumbs: internal discussion around failing tests which may be blocking @ricardopereira from fully testing this, which may be why this PR is still in Draft state. |
Yes, exactly. Thanks @QuintinWillison. |
bc662e1
to
40d1102
Compare
Publish and presence state changes should fail immediately if not in the connected state and shouldn't change the channel state.
Use the Realtime queue array for every message.
… connection state is INITIALIZED, CONNECTING or DISCONNECTED
b3ee004
to
d6b07d2
Compare
The CI is getting better but even so I had to restart some of them. |
@QuintinWillison BTW, this fixes #894 too 🙌 |
[channel setFailed:channelStatus]; | ||
break; | ||
default: | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is all of this still gated under a condition that fails if options.queueMessages
is false? This is RTL3, right? That spec item doesn't mention anything about queueMessages, what do they have to do with each other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 586c255
#1004