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

BET-4061: Apollo 0.23.1 #6

Merged
merged 365 commits into from
Mar 3, 2020
Merged

BET-4061: Apollo 0.23.1 #6

merged 365 commits into from
Mar 3, 2020

Conversation

ketenshi
Copy link

@ketenshi ketenshi commented Feb 24, 2020

Updated Apollo Version
Release notes: https://github.com/apollographql/apollo-ios/releases

They made some fixes to the Watcher and Caching, which should fix some issues we are currently facing.


Created this branch off connect and then rebased on top of upstream/master

Afterwards merged connect into this branch.

No new merge conflicts other than from the commits we had before.

designatednerd and others added 30 commits February 11, 2020 14:52
@tapi
Copy link

tapi commented Mar 2, 2020

agreed, Also curios why we have connect/disconnect and the socket public

@ketenshi
Copy link
Author

ketenshi commented Mar 2, 2020

We connect/disconnect when we update the auth token. Otherwise from what I remember we'll have to create a new ApolloClient each time.

@tapi
Copy link

tapi commented Mar 2, 2020

and that is dependency injected all the way down so sending a new one is a PITA.

LOL. we don't use the methods we added. we use the exposed web socket directly

private func disconnectWebsockets(reconnect: Bool = false) {
        let websockets = queryRouter.endpoints.compactMap { $0.websocketTransport?.websocket as? WebSocket }
        websockets.forEach { $0.disconnect() }
        if reconnect {
            websockets.forEach { $0.connect() }
        }
    }

@ketenshi
Copy link
Author

ketenshi commented Mar 2, 2020

Lol we can discard that change then.

@tapi
Copy link

tapi commented Mar 2, 2020

And it looks like that is the only time we use websocket so it's likely the methods would be sufficient and probably an easier sell upstream.

@tapi
Copy link

tapi commented Mar 2, 2020

I wonder if instead of connect/disconnect we can use the existing close and add reopen seems like maybe that would solve our use of refreshInterval as well

@ketenshi
Copy link
Author

ketenshi commented Mar 2, 2020

Looking at their closeConnection() it sends a termination message to the server and clears out subscriptions and queues. I think we'll need to do some investigation and testing there as a separate PR.

@ketenshi
Copy link
Author

ketenshi commented Mar 3, 2020

Created https://jira.thescore.com/browse/BET-4109 to track pushing the changes upstream

@ketenshi ketenshi changed the title BET-4061: Apollo 0.23.0 BET-4061: Apollo 0.23.1 Mar 3, 2020
Kevin Delannoy and others added 9 commits March 3, 2020 10:06
This reverts commit 50d8f27.
We already have the connect() and disconnect() methods.
* origin/connect:
  Extend 'WebSocketTransportDelegate' protocol
  Expose 'reconnectionInterval' property
  Extend 'WebSocketTransportDelegate' protocol
  Made connectingPayload public
  Made the websocket public
  Expose 'security' for SSL pinning
  Exposed a method to connect/disconnect

# Conflicts:
#	Sources/ApolloWebSocket/WebSocketTransport.swift
@ketenshi ketenshi merged commit f2a3075 into connect Mar 3, 2020
@ketenshi ketenshi deleted the BET-4061-update-fork branch March 3, 2020 16:23
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.

6 participants