aws-mobile-appsync-sdk-android icon indicating copy to clipboard operation
aws-mobile-appsync-sdk-android copied to clipboard

Use Android apollo-client as a dependency instead of modifying its code

Open PSIXO opened this issue 5 years ago • 5 comments

While looking at AppSync code it is obvious that large part of it is just a fork of https://github.com/apollographql/apollo-android Also that fact is stated in the doc on the first page. For example class AppSyncResponseFetchers is exactly the same like ApolloResponseFetchers down to the docs. Only class name and Licence header are different in this case. My guess is that team behind the Android Sdk has more legal knowledge than me so this is probably aligned with MIT licenced source. But from the technical point of view it is a complete puzzle why it's done this way.

Could the Apollo lib be a dependency of this project and AWS added classes another artifact? Having it setup like this would allow users of this library to chose the Apollo client version they want to use, would allow them to more easily read the docs/tutorials and to know on which project to report bugs.

PSIXO avatar Apr 23 '19 10:04 PSIXO

@PSIXO Although you're correct that large parts of the code are the same, we did modify some of the core behaviors. I haven't done an analysis for Android yet, but for iOS, a non-comprehensive list of changes includes:

  • Expose whether a result was served from service or cache
  • Trigger watches when writing to cache within a transaction
  • Allow null result values to be propagated in selection sets as long as the underlying type is optional

Please let us know if you have other questions.

palpatim avatar Apr 23 '19 20:04 palpatim

100% percent agree, it is even more problematic now, when android studio 3.4 is stable and gradle 5.1+ needs to be used (issue here https://github.com/awslabs/aws-mobile-appsync-sdk-android/issues/91 ) .. and this prevents using appsync because it is not compilable, although apollo library already has fix for that.

mlykotom avatar Apr 30 '19 11:04 mlykotom

I'm going to recharacterize this as a Feature Request for importing the latest Apollo client. As I mentioned above, I'm not sure how realistic this request is since the base client at the time we forked it had some behaviors that were incompatible with AppSync, but we'll add to the feature backlog and update here when we have more information.

palpatim avatar Aug 05 '19 15:08 palpatim

Any update on this? I finds this particularly jaring as I don't use AppSync but a 3rd Party library has dependencies on it. Now I use Apollo's latest library directly (2.1.0) and it seems the 3rd party transitive dependencies are making my build fail. e.g.

com.apollographql.apollo.cache.normalized.sql

I do understand forking of the Apollo client - but there are tricks one can use, renaming class and modules etc or using Gradle build system to stop the exposure of internal libraries API??

teopeurt avatar May 27 '20 15:05 teopeurt

@palpatim Since there were some modifications introduced to the original code - is there any way to at least change the package names of the embedded classes? This should be faster than integrating the apollo library the proper way.

Because of this embedded code, we are totally unable to use AppSync SDK in our app.

EDIT: I could give it a try and make a PR for it if there is any chance that this could be released soon(ish)

scana avatar Jun 09 '20 15:06 scana