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.

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

Remove bad fragment recommendations in best practices docs #11390

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

jerelmiller
Copy link
Member

Our best practices doc includes some recommendations on fragments that I do not deem as good advice. This PR removes them from the documentation.

There are additional recommendations that I would like to add in the future regarding fragments, including deemphasizing splitting queries at the component boundary, but for now this is the quick and dirty change I would like to see removed to avoid future Apollo Client users from following these patterns.

@jerelmiller jerelmiller requested a review from a team as a code owner November 27, 2023 17:54
Copy link

changeset-bot bot commented Nov 27, 2023

⚠️ No Changeset found

Latest commit: be9dffc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

size-limit report 📦

Path Size
dist/apollo-client.min.cjs 37.08 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" 43.5 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/main.cjs" (production) 42 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" 32.61 KB (0%)
import { ApolloClient, InMemoryCache, HttpLink } from "dist/index.js" (production) 31.27 KB (0%)
import { ApolloProvider } from "dist/react/index.js" 1.22 KB (0%)
import { ApolloProvider } from "dist/react/index.js" (production) 1.2 KB (0%)
import { useQuery } from "dist/react/index.js" 4.28 KB (0%)
import { useQuery } from "dist/react/index.js" (production) 4.1 KB (0%)
import { useLazyQuery } from "dist/react/index.js" 4.59 KB (0%)
import { useLazyQuery } from "dist/react/index.js" (production) 4.41 KB (0%)
import { useMutation } from "dist/react/index.js" 2.55 KB (0%)
import { useMutation } from "dist/react/index.js" (production) 2.53 KB (0%)
import { useSubscription } from "dist/react/index.js" 2.24 KB (0%)
import { useSubscription } from "dist/react/index.js" (production) 2.2 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" 4.62 KB (0%)
import { useSuspenseQuery } from "dist/react/index.js" (production) 4.05 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" 4.14 KB (0%)
import { useBackgroundQuery } from "dist/react/index.js" (production) 3.56 KB (0%)
import { useReadQuery } from "dist/react/index.js" 2.99 KB (0%)
import { useReadQuery } from "dist/react/index.js" (production) 2.94 KB (0%)
import { useFragment } from "dist/react/index.js" 2.09 KB (0%)
import { useFragment } from "dist/react/index.js" (production) 2.04 KB (0%)

@@ -160,87 +160,6 @@ query GetGlobalStatus {
* If you have collections of components that _are_ always rendered together, you can use fragments to distribute the structure of a single query between them. See [Colocating fragments](./fragments/#colocating-fragments).
* If you're querying a list field that returns more items than your component needs to render, you should [paginate that field](../pagination/overview/).



## Use fragments to encapsulate related sets of fields
Copy link
Member Author

Choose a reason for hiding this comment

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

Going forward, we want fragments to be used by components so that components can declare their data needs. It is rare that you should need/want to use fragments as a means to DRY up a query, or share data between multiple operations. This tends to result in overfetching over time.

}
```

### Avoid excessive or illogical fragments
Copy link
Member Author

Choose a reason for hiding this comment

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

When using fragments to declare data needs for a component, queries start to look more like the "less readable" example here. We much prefer for devs to use fragments as the unit of composition in components rather than queries, so this recommendation makes no sense anymore.

Copy link

netlify bot commented Nov 27, 2023

Deploy Preview for apollo-client-docs ready!

Name Link
🔨 Latest commit be9dffc
🔍 Latest deploy log https://app.netlify.com/sites/apollo-client-docs/deploys/6564d7cdcc282c0008114a6c
😎 Deploy Preview https://deploy-preview-11390--apollo-client-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@alessbell alessbell left a comment

Choose a reason for hiding this comment

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

💯

@github-actions github-actions bot added the auto-cleanup 🤖 label Nov 27, 2023
Copy link
Contributor

@Meschreiber Meschreiber left a comment

Choose a reason for hiding this comment

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

Thank you for clean-up and commenting on what we'll want to recommend in the future!

@jerelmiller jerelmiller merged commit 4adbb2e into main Nov 27, 2023
19 checks passed
@jerelmiller jerelmiller deleted the change-some-recs branch November 27, 2023 18:52
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants