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.

Sign up for GitHub

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

Jump to bottom

Fragment referencing/Query composition support #343

Merged
merged 11 commits into from
Jul 1, 2016
Merged

Fragment referencing/Query composition support #343

merged 11 commits into from
Jul 1, 2016

Conversation

Poincare
Copy link
Contributor

Adding a way to reference fragments from different components of your application and compose queries using those fragments. See #338

TODO:

  • Update CHANGELOG.md with your change
  • Make sure all of the significant new logic is covered by tests
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

@Poincare Poincare changed the title [WIP] Fragment referencing Fragment referencing/Query composition support Jun 30, 2016
@Poincare Poincare self-assigned this Jun 30, 2016
stubailo
stubailo reviewed Jun 30, 2016
View reviewed changes
src/QueryManager.ts
@@ -202,7 +204,7 @@ export class QueryManager {
mutation: Document,
variables?: Object,
resultBehaviors?: MutationBehavior[],
}): Promise<GraphQLResult> {
}, fragments: FragmentDefinition[] = []): Promise<GraphQLResult> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just add this to the existing list of options?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds better - will do.

@stubailo
Copy link
Contributor

@Poincare one question I just realized - how will this work with the view integrations? Perhaps it should be a static export of apollo-client, since it doesn't really require an instance?

Like:

import { createFragments } from 'apollo-client';

createFragments(gql`
  ...
`);

Or something.

@jbaxleyiii do you have any thoughts on how this might or should interact with react-apollo? My thought is that maybe we don't need anything React-specific here if it's just a static property you can pass around.

@Poincare
Copy link
Contributor Author

Poincare commented Jul 1, 2016 •
edited
Loading

I thought the client instance was available to react-apollo and angular-apollo through the provider. If that's the case, we might not need a static export although it would probably be easier for an app developer to use fragments with the static export.

Secondly, I think holding the state that describes the fragment names should be specific to a particular instance of ApolloClient rather than a global across the package. I don't think I have a clear understanding of how the package/import system works so I'm not sure if there's some way around holding package-global state if we use a static export.

(P.S. Moved fragments into WatchQueryOptions and made the other fixes you mentioned)

stubailo
stubailo reviewed Jul 1, 2016
View reviewed changes
src/index.ts

// This function disables the warnings printed about fragment names. One place where this chould be
// is called when writing unit tests that depend on Apollo Client and use named fragments that may
// have the Nsame name across different unit tests.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra capital N

@stubailo
Copy link
Contributor

stubailo commented Jul 1, 2016

Looks great other than some small comments

@Poincare
Copy link
Contributor Author

Poincare commented Jul 1, 2016

Fixed @stubailo

@Poincare
added fragments to query, watchQuery and startQuery
87a9831
@Poincare
in progress commit for the nested array structure
8406de5
@Poincare
added support for fragment referencing within QueryManager and added …
707f512 
…a method to ApolloClient to construct fragments
@Poincare
Added tests for mutate, watchQuery and query on client - all passing …
b52112c 
…for fragment referencing
@Poincare
added a test for polling queries and fragment references
156a073
@Poincare
updated changelog
67d7c4b
@Poincare
stylistic changes + more clear comments
da77d99
@Poincare
moved fragments into WatchQueryOptions and made stylistic fixes
2d7c817
@Poincare
added a few tests and made it possible to disable warnings
55e7d81
@Poincare
replaced getFragmentDefinitions with getFragmentDefinition
661e1bd
@Poincare
fixed style issues + fixed a problem where warning messages where pri…
fa22a34 
…nting where they were not supposed to
@stubailo stubailo merged commit 72114a2 into master Jul 1, 2016
@zol zol removed the in progress label Jul 1, 2016
@Poincare Poincare mentioned this pull request Jul 5, 2016
Closed
@Poincare Poincare mentioned this pull request Jul 20, 2016
Closed
@stubailo stubailo deleted the fragment_referencing branch September 20, 2016 03:43
@snyk-bot snyk-bot mentioned this pull request May 3, 2021
Open
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@Poincare Poincare

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Poincare @stubailo @zol