Query merging at top level #277
Conversation
| import cloneDeep = require('lodash.clonedeep'); | ||
|
|
||
| // A handler that takes a selection, variables, and a directive to apply to that selection. | ||
| export type DirectiveResolver = (selection: Selection, |
There was a problem hiding this comment.
This alignment is pretty odd - we don't do it anywhere else in the code this way.
|
Are the changes in Also, related to the previous question, does this PR introduce the batching ability in Network Interface? If so how does it relate to the batching PR? It looks like I think the only files that should be in there are |
|
The changes in The |
|
Would it be possible to make all the batching related changes in the batching PR, and then rebase this PR on top of that one? Or is the batching PR made on top of this one? |
|
The batching PR is made on top of this one. If this one can is merged before the batching one, we can just apply the batching PR on top of this PR on |
|
Removed |
|
Ok, that's fine. I would have thought that logically this PR comes on top of the batching one (because it implements a particular way of batching), but it doesn't matter all that much, as long as the end-result is what we need. |
| } | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Why is this test no longer needed? Is this related to query merging?
There was a problem hiding this comment.
Since we now use the query document for the Request, this test no longer makes any sense (i.e. "gql" should throw the same error before anything is sent to the server).
… spreads within result unpacking
| renameVariables(operationOrFragmentDef.selectionSet, aliasName); | ||
| return operationOrFragmentDef; | ||
| } else { | ||
| return definition; |
There was a problem hiding this comment.
This branch isn't covered by any test. Should be tested, or removed if unreachable.
|
Alright, that all makes sense. Thanks for making those changes so fast. It looks good to me now, except that a few things aren't covered by tests yet. After adding the couple of tests that are missing, we should be good to go! |
|
🎉 |
update docs: PropTypes is now a separate package
This PR (originally part of PR #266) implements query merging at the top level. It provides functions that can take multiple queries and place them under the same query while aliasing the names within the AST to prevent collisions. The point of doing this is to implement query batching without requiring any changes to the server (see issue #164).
The CHANGELOG has not been updated since this is not a user-facing change (yet).
TODO: