-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Merge Apollo Link common + HTTP functionality into core #5412
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
And remove the unused isomorphic fetch dep.
Introductory import of the Apollo Link core, common and HTTP code that is going to be integrated into the Apollo Client core. This code is going to change, but this commit gets the initial code moved over, and ensures that all Link tests still pass.
Now that the HTTP link code is part of AC, we can open up the constructor to allow additional options, like Apollo Boost did. Most people will just want to use `uri`, but `credentials` and `headers` use is common enough that it probably makes sense to expose them as well. Other HTTP link options can still be accessed by instantiating an `HttpLink` and passing it in via AC's constructor `link` option. Defining a custom link overrides the `uri`, `credentials`, and `headers` options.
The Apollo Link project will be updated to re-use common and HTTP based link code from Apollo Client, so we need to make sure that code is reachable externally.
This util was adding very little additional value over just using `ApolloLink` directly.
Also removes link test utilities from the public API.
This should help make future refactoring a bit easier, and gets us away from re-exporting all link utility functions, when very few of them need to be made available outside of this project.
Smaller, more focused files should make it easier to refactor AC's link support further. Link based re-exports from Apollo Client have been adjusted at the same time, to make sure we're only exporting items that are trully needed by 3rd party links (and by the `apollo-link` repo).
I'm not sure what the historical reason was for exposing link utility functions like `concat`, `empty`, `from`, `split` and `execute` both as static functions on `ApolloLink` and as separate importable functions. Since it might be a bit jarring for people if we remove one of these options, for now lets move all `ApolloLink` functionality back into the `ApolloLink` class, and instead re-export that functionality as separate utility functions. Using the utility functions through `ApolloLink` seems to be the most popular approach, and by aligning this in the code we'll make future maintenance a bit easier.
benjamn
approved these changes
Oct 4, 2019
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.
Looks great! I will push a couple commits with minor tweaks that I spotted while reviewing.
@hwillson Before we merge this, can you update |
Sub-classing then re-exporting `Observable` leads to the `@types/zen-observable` types not being available, when `Observable` is exported from `@apollo/client`. This is because the `@types/zen-observable` class declaration (`declare class Observable<T>`) is replaced during TS compilation with the `class Observable extends LinkObservable` declaration, which doesn't expose any of original `Observable` methods. This means projects like Apollo Link that are using the re-exported `Observable` from `@apollo/client`, don't see any of the `Observable` methods when using Typescript. Instead of creating an `Observable` sub-class with the additional RxJS interop functionality, this commit uses TS global module augmentation to essentially add to the `@types/zen-observable` `Observable` type. It then adds the needed interop function to `Observable` directly. This allows external projects like Apollo Link to find all `@types/zen-observable` types, and keeps RxJS interop functionality in place. And just to add - method creation using `$$observable` was removed as `zen-observable` already does this: https://github.com/zenparsing/zen-observable/blob/f63849a8c60af5d514efc8e9d6138d8273c49ad6/src/Observable.js#L396
Make sure all parts of Apollo Client import `Observable` from a local module, instead of from `zen-observable` directly. This decoupling will make things easier as we investigate letting people use their own observables implementation.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Background
As part of Apollo Client 3's incremental steps towards improving developer experience, portions of the Apollo Link infrastructure are being moved into core. This will help ensure developers have much of what they need to build awesome GraphQL based applications out of the box, with Apollo Client. This will also help make future Apollo Link optimizations easier, due to its closer integration with the Apollo Client core.
Implementation
This PR introduces functionality into
@apollo/client
that is equivalent to the following Apollo Link packages:apollo-link
apollo-link-http
apollo-link-http-common
This means installing and referencing the previously separate
apollo-link
and/orapollo-link-http
packages is no longer necessary.ApolloLink
andHttpLink
are now exported directly from@apollo/client
, and HTTP links can be created automatically using theApolloClient
uri
constructor option:This approach is very similar to the now deprecated Apollo Boost project, but instead of offering a limited set of capabilities from a separate project, the goal is to simplify use of the main (and only) Apollo Client project (while providing options to incrementally add in more sophisticated functionality).
Apollo Link Project
Associated PR: apollographql/apollo-link#1158
PR apollographql/apollo-link#1158 updates the Apollo Link project to remove the capabilities that will be part of the AC core, after this PR is merged in. Packages in the
apollo-link
repo will have a dependency on Apollo Client, to pull in the common Apollo Link functionality they need.Documentation
Apollo Client documentation changes for this work are coming in a separate PR. We're going to merge the separate Apollo Link documentation into the Apollo Client docs, which means there will be quite a few changes.