gateway: Update gateway.load() to support AS2 and AS3 #819
Conversation
This file contains hidden or 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
Starting with AS 2.25.0, gateway.load receives `graphRef` in addition to `graphId` and `graphVariant`; `graphVariant` is declared as always provided. In AS3, only `graphRef` will be provided; `graphVariant` will not be provided. In order for this code to work with both versions, we declare a version of ApolloConfig that could come from either ourselves, and parse it into an internal representation based on the AS3 version.
glasser
force-pushed
the
glasser/as3-compat
branch
from
93bc8c9 to
ff9f07d
Compare
trevor-scheer
approved these changes
Jun 16, 2021
Comment on lines
+111
to
+116
| // The protocol for talking to GCS requires us to split the graph ref | ||
| // into ID and variant; sigh. | ||
| const at = graphRef.indexOf('@'); | ||
| const graphId = at === -1 ? graphRef : graphRef.substring(0, at); | ||
| const graphVariant = at === -1 ? 'current' : graphRef.substring(at + 1); | ||
|
|
There was a problem hiding this comment.
Suggested change
| // The protocol for talking to GCS requires us to split the graph ref | |
| // into ID and variant; sigh. | |
| const at = graphRef.indexOf('@'); | |
| const graphId = at === -1 ? graphRef : graphRef.substring(0, at); | |
| const graphVariant = at === -1 ? 'current' : graphRef.substring(at + 1); | |
| // The protocol for talking to GCS requires us to split the graph ref | |
| // into ID and variant; sigh. | |
| const [graphId, graphVariant = "current"] = graphRef.split("@"); |
There was a problem hiding this comment.
That's cool syntax I hadn't thought of. But if there are multiple @s it just silently drops whatever is after the second, right? Not really legal variant but still. (Super messed up that the limit arg to JS string.split isn't "stop splitting after N pieces" but "throw away all bit the first N pieces".)
|
I think the legacyNetworkRequests tests test the old "pass in graphId/graphVariant" way reasonably well, though they aren't explicitly called out like a unit test of it. |
glasser
added a commit
that referenced
this pull request
Jun 16, 2021
GraphQLServiceEngineConfig is being removed in AS3, so we can't import it and use it in our public API if we want to work with both AS2 and AS3.
glasser
added a commit
that referenced
this pull request
Jun 16, 2021
GraphQLServiceEngineConfig is being removed in AS3, so we can't import it and use it in our public API if we want to work with both AS2 and AS3.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Starting with AS 2.25.0, gateway.load receives
graphRefin addition tographIdandgraphVariant;graphVariantis declared as alwaysprovided.
In AS3, only
graphRefwill be provided;graphVariantwill not beprovided.
In order for this code to work with both versions, we declare a version
of ApolloConfig that could come from either ourselves, and parse it into
an internal representation based on the AS3 version.