Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[chore]: Move .graphql files from queries/ directory to the component directory where they are used #1928

Closed
sirugh opened this issue Oct 22, 2019 · 6 comments
Labels
help wanted Eligible for community contribution. hold On hold until another condition is fulfilled.

Comments

@sirugh
Copy link
Contributor

sirugh commented Oct 22, 2019

Queries (and mutations) are supposed to be highly tailored to the component using them; it's not considered an optimization to reuse a query, so there's no value in keeping them in a shared folder.

Goal

  • Move all .graphql from /queries directory into the directory where they are used. For example, queries/createAccount.graphql is used by the CreateAccount component so we should move the graphql file to venia-ui/lib/components/CreateAccount/.
  • Ensure all tooling around queries is updated if necessary. There may be paths hardcoded to look in queries/ and if so, it needs to be updated to check all locations for .graphql instead.

Expected Structure

  • Component Operations: <componentName>.gql.js - for ease of use, the default export of this file should be an object of queries and mutations. Example here.

It is expected that since multiple components imported these .graphql files, this will result in multiple components having identical operations; this is okay. Our current patterns outline that GraphQL reuse is driven by fragments, not operations, which is not a requirement in this scope. If you would like to tackle breaking response shapes into fragments, you can follow the patterns in some newer components on CartPage.

  • Fragment Structure: <componentName>Fragments.gql.js

See here for more context.

@supernova-at
Copy link
Contributor

supernova-at commented Oct 22, 2019

Just chiming in from the Apollo caching perspective: I think this is fine, but we will want to use and share fragments to ensure we get cache hits, which may mean overfetching in some particular cases.

A good example is the Category page requesting details about a Product. To render its UI, the Category page only really needs a subset of that data (like name, small_image, price), but the PDP (a likely place the user will navigate to next) will likely want more details (media_gallery_entries, etc.).

We can create a "ProductDetails" fragment that looks something like

fragment ProductDetails on ProductInterface {
  media_gallery_entries
  name
  price
  small_image
}

and then we can re-use it in the Category and Product queries:

products {
  ... ProductDetails
}

The Category page will technically be overfetching data, but it'll make it so the PDP can serve data from the cache instead of having to make another request.

All that said, I opened an issue (magento/graphql-ce#1027) because it seems like support for fragments has some bugs in Magento GraphQL currently.

@sirugh
Copy link
Contributor Author

sirugh commented Oct 22, 2019

Yea, so fragments may be a shareable thing but optimizing right now to create a fragment/ directory may be overkill (especially since they don't quite work). Good call out though @supernova-at !

@sirugh sirugh added the Progress: good first issue Good for newcomers label Oct 22, 2019
@brendanfalkowski
Copy link
Contributor

There's no value in keeping them in a shared folder.

Caveat: improves learning + onboarding new devs.

It was super nice having all queries in the same folder so I could easily compare their structure, naming, etc.

Also, I never would have noticed #1913 if the files were spread out in components.

@robdunn
Copy link

robdunn commented Nov 21, 2019

Happy to take this on. Seems like a good one to get familiar with the project. Thanks!

@awilcoxa
Copy link

awilcoxa commented Mar 3, 2020

@tjwiebell to fill in detail on expected pattern to follow for this issue. Also provide an example via a branch to provide a starting place.

@tjwiebell tjwiebell added the hold On hold until another condition is fulfilled. label Mar 10, 2020
@tjwiebell tjwiebell added the Event: Global-Contribution-Day Groomed for GCD 2020 label Apr 3, 2020
@tjwiebell tjwiebell removed their assignment Apr 3, 2020
@awilcoxa awilcoxa removed Event: Global-Contribution-Day Groomed for GCD 2020 Progress: good first issue Good for newcomers labels Apr 9, 2020
@awilcoxa awilcoxa removed the hold On hold until another condition is fulfilled. label May 5, 2020
@larsroettig larsroettig self-assigned this May 5, 2020
@larsroettig larsroettig removed their assignment Jun 17, 2020
@tjwiebell tjwiebell added the hold On hold until another condition is fulfilled. label Jul 22, 2020
@tjwiebell
Copy link
Contributor

Given the scope of this, we're going to pull this internal as we work out the final resting place of queries and fragments.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
help wanted Eligible for community contribution. hold On hold until another condition is fulfilled.
Projects
None yet
Development

No branches or pull requests

7 participants