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

Discussion: What would an Apollo Link implementation look like? #173

Closed
nubbel opened this issue Oct 27, 2017 · 6 comments
Closed

Discussion: What would an Apollo Link implementation look like? #173

nubbel opened this issue Oct 27, 2017 · 6 comments

Comments

@nubbel
Copy link

nubbel commented Oct 27, 2017

Hey!

Modifying/intercepting the way the Client communicates with the network is a popular request by the community. Solutions have been proposed in #131 and #172, but have not been accepted. Instead a more generic design akin to Apollo Link was suggested.

I'm interested in having a stab at implementing Apollo Link for apollo-ios.
Before I get down to coding, I'd like to discuss a few points:

  1. The Javascript version of Apollo Link heavily relies on Observables. Currently, there is no notion of Observables in Swift nor in apollo-ios. I understand that Observables are useful for things like automatic retrying etc., but would you consider that a strict requirement or would an implementation based on Promises also be acceptable for you?
  2. In case a design based on Observables is a strict requirement, would you prefer an implementation of Observables for Swift from scratch or could we use an already existing solution (similar to how the Javascript version depends on zen-observables)? I'm not sure if that would cause any licensing issues though. Implementing Observables from scratch is doable, but definitely not trivial. You can check out my first experiments in this gist (neglecting all concurrency). The most popular implementation of Observables in Swift seems to be RxSwift. It is extremely feature-rich and well-tested, but I would prefer a more lightweight solution if there is any.
  3. The Javascript version uses a context object for link-to-link communication. Could anyone think of a more Swifty and typesafe solution?
  4. Should Apollo Link be a separate Pod (just like Apollo/SQLite) or should it be integrated into Core?

Let me know what you think!

@martijnwalraven, @MrAlek, @DarkDust

@martijnwalraven
Copy link
Contributor

Really happy you're opening this issue, and I think you're asking exactly the right questions. Would be great to bring more people together to figure this out. I'm traveling right now, but maybe we could set up a meeting for next week.

Let me give you some preliminary answers, but we should probably schedule a call to talk about this in more depth and discuss next steps:

  • I don't think our Link implementation necessarily has to use Observables, and I agree we don't want a dependency on something like RxSwift. If we do, I'd prefer a lightweight built-in implementation as well, similar to what we've done with Promises.
  • For the context object, it would be great if we could make this type-safe instead of relying on a dictionary. I was thinking maybe Links could have an associated input and output context type, and then we could use protocol composition to add our own properties to the input type and specify the result as the output type. Haven't tried anything out, so just an idea that may not work at all.
  • I think we'll want this in core, because it'll be a replacement for the current network transport protocol.

@nubbel
Copy link
Author

nubbel commented Oct 31, 2017

Great to get some good feedback on this!
We‘re having a public holiday today and tomorrow in Germany (or at least in Bavaria), so I won’t be available before Thursday.
Is there a slack channel or something we could use, so we don’t pollute this issue with organizational things?

@martijnwalraven
Copy link
Contributor

@nubbel I just created an #ios-core channel on the Apollo Slack, so let's continue the discussion there!

@martijnwalraven
Copy link
Contributor

I think we can just use this issue to continue the discussion. Here is the gist @nubbel created to discuss a type-safe Context.

@nubbel
Copy link
Author

nubbel commented Nov 20, 2017

Quick update: I've drafted an implementation and like to share some details on some of the design decisions I made.
The changes currently live on my branch: https://github.com/apollographql/apollo-ios/compare/master...nubbel:pr/apollo-link?expand=1#diff-2ada2f86538801e01040a5462880d916

Terminating vs. Non-Terminating Links

ApolloLink has a concept of terminating and non-terminating links. In the Javascript implementation, the link type is determined at runtime.
I thought we could de better and determine it at compile time by using two distinct types TerminatingLink and NonTerminatingLink. This eliminates some compile time checks. For example, concat is only defined on NonTerminatingLink and returns a terminating or non-terminating link, depending on the type of the second link.
However, it makes it difficult to define a split of a terminating and a non-terminating link or vice-versa, because what should the return type be? SchrödingersLink?

Cancellation

Requests need to be cancellable, that means links need to be cancellable. I considered three options to achieve that:

  1. Using Observables. Observables are inherently cancellable, but non-trivial to implement.
  2. Adding a Cancelled state to Promise.
  3. Using a cancellable object, that us to register onCancel handlers. This was inspired by an experimental Javascript feature, AbortController.

I chose the first third option and created an CancelController and CancelSignal similar to AbortController/AbortSignal in Javascript. The signal is passed in the context.
It seemed to be the less invasive solution and I actually really like the design.

LinkOperationDescriptor

This struct is used for the test closure in SplitLink to overcome the Swift limitation that we can't store generic closures as properties. In essence, it is a type-erased wrapper for GraphQLOperation. I don't really like, but couldn't find a better solution.

Caveats

The current design has a few caveats. Most notably, links can only return one result, which renders them unsuitable for subscriptions.

Next steps

As a next step, I would like to implement an HTTPLink that pulls configuration from the context and makes a network request.
Then, I'd like to see how we could implement a RetryLink.

@nubbel
Copy link
Author

nubbel commented Nov 22, 2017

I managed to get rid of LinkOperationDescriptor and filed a PR #186, so let's continue the discussion there.

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

No branches or pull requests

2 participants