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

Networking Cleanup #1847

Merged
merged 12 commits into from
Jun 24, 2021
Merged

Networking Cleanup #1847

merged 12 commits into from
Jun 24, 2021

Conversation

AnthonyMDev
Copy link
Contributor

Changelog Additions

  • Breaking: Renamed some of the request chain interceptors object:
    • LegacyInterceptorProvider -> DefaultInterceptorProvider
    • LegacyCacheReadInterceptor -> CacheReadInterceptor
    • LegacyCacheWriteInterceptor -> CacheWriteInterceptor
    • LegacyParsingInterceptor -> JSONResponseParsingInterceptor
  • Breaking: WebSocketTransport is now initialized with an ApolloWebSocket (or other object conforming to the ApolloWebSocketClient protocol.) Previously, the initializer took in the necessary parameters to create the web socket internally. This provides better dependency injection capabilities and makes testing easier.
  • Removed class constraint on ApolloInterceptor and converted to structs for all interceptors that could be structs instead of classes.

Note:

Before releasing, updates to docs need to go out. @AnthonyMDev has a stashed changeset where he started this. (Finish by running script generation and removing all instances of “Legacy”, "FlexibleDecoder", and "Parseable" left in docs!)

@AnthonyMDev AnthonyMDev added the include-in-changelog Indicates that changes from a PR should be noted in the changelog for their release. label Jun 23, 2021
@AnthonyMDev AnthonyMDev added this to the June 2021 milestone Jun 23, 2021
@designatednerd
Copy link
Contributor

For the curious: This came out of an internal discussion about a bunch of stuff that was intended under prior plans that we've changed direction on, so we're renaming things to be more accurate for our current direction.

@@ -19,8 +19,6 @@
9B21FD782424305700998B5C /* ExpectedEnumWithDifferentCases.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B68F05F2416F80C00E97318 /* ExpectedEnumWithDifferentCases.swift */; };
9B21FD792424305E00998B5C /* ExpectedEnumWithSanitizedCases.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B68F063241703B200E97318 /* ExpectedEnumWithSanitizedCases.swift */; };
9B260BEB245A020300562176 /* ApolloInterceptor.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B260BEA245A020300562176 /* ApolloInterceptor.swift */; };
9B260BED245A021300562176 /* Parseable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B260BEC245A021300562176 /* Parseable.swift */; };
9B260BEF245A022E00562176 /* FlexibleDecoder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9B260BEE245A022E00562176 /* FlexibleDecoder.swift */; };
Copy link
Contributor

Choose a reason for hiding this comment

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

goodbye, cockamamie idea for a generalizable parser, you were fun while you lasted

Sources/Apollo/JSONResponseParsingInterceptor.swift Outdated Show resolved Hide resolved
public weak var delegate: WebSocketTransportDelegate?

let connectOnInit: Bool
let reconnect: Atomic<Bool>
var websocket: ApolloWebSocketClient
let websocket: ApolloWebSocketClient
Copy link
Contributor

Choose a reason for hiding this comment

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

boy I wish I could remember why this was a var in the first place...

import Foundation

/// The default interceptor provider for typescript-generated code
open class DefaultInterceptorProvider: InterceptorProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since LegacyInterceptorProvider is something people have subclassed, do you think it's worth it to use @available deprecation stuff to get Xcode to offer a fix-it? Or is that overkill since we're in 0.x territory?
https://www.hackingwithswift.com/example-code/language/how-to-use-available-to-deprecate-old-apis

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that's overkill since we're not at 1.0 yet.

@AnthonyMDev AnthonyMDev merged commit 57c07f1 into main Jun 24, 2021
@AnthonyMDev AnthonyMDev deleted the networking-cleanup branch June 24, 2021 18:48

override class func setUp() {
super.setUp()
swap(&webSocketProvider, &WebSocketTransport.provider)
Copy link
Member

Choose a reason for hiding this comment

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

👍🏻

@hwillson hwillson removed this from the MM-2021-06 milestone Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
include-in-changelog Indicates that changes from a PR should be noted in the changelog for their release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants