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

feature: Adds collections & uploads Apollo to version 3 #455

Merged
merged 18 commits into from
Feb 4, 2021

Conversation

kierangillen
Copy link
Collaborator

@kierangillen kierangillen commented Jan 15, 2021

Screen Shot 2021-01-27 at 6 38 20 PM

@kierangillen kierangillen requested a review from l2succes January 19, 2021 21:49
@kierangillen kierangillen marked this pull request as ready for review January 19, 2021 21:49
@@ -129,18 +129,8 @@ export const Home = screenTrack()(({ navigation, route }) => {
if (!isFetchingMoreFitPics && fitPicsReceived > 0) {
fetchMore({
variables: { firstFitPics: 8, skipFitPics: fitPicsReceived },
updateQuery: (prev: { fitPics: Homepage_fitPics[]; fitPicsCount: any }, { fetchMoreResult }) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updateQuery is going to be deprecated next major release, this PR updates the homepage to use the new cache merge function as an example.

@@ -112,25 +111,12 @@ export const setupApolloClient = async () => {
}
})

const accessToken = await getAccessTokenFromSession()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We're already storing the useSession in the context provider, no need for it in the cache layer

Comment on lines 12 to 22
typePolicies: {
Query: {
fields: {
fitPicsCount: {
read(newCount) {
return newCount
},
},
fitPics: offsetLimitPagination(),
},
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the new method for updateQuery - updating the query directly in the cache instead of calling updateQuery

@kierangillen kierangillen marked this pull request as draft January 19, 2021 22:12
@kierangillen kierangillen changed the title feature: Adds collections feature: Adds collections & uploads Apollo to version 3 Jan 20, 2021
@kierangillen kierangillen marked this pull request as ready for review January 20, 2021 19:33
})
const httpLink = new HttpLink({
uri: process.env.MONSOON_ENDPOINT || "http://localhost:4000/", // Server URL (must be absolute)
}) as any
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should figure out the proper types for this

},
fitPics: {
merge(existing = [], incoming = [], { args: { skipFitPics = 0 } }) {
const merged = existing ? existing.slice(0) : []
Copy link
Collaborator

Choose a reason for hiding this comment

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

@kierangillen Do we actually need to do this for every paginated query? seems a bit odd they would add that much complexity

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes this is the new method for handling merging arrays now that updateQuery is deprecated: https://www.apollographql.com/docs/react/caching/cache-field-behavior/#merging-arrays

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -48,7 +48,7 @@ export const Bag = screenTrack()((props) => {
}, [])
)

const { data, refetch } = useQuery(GET_BAG)
const { previousData, data = previousData, refetch } = useQuery(GET_BAG)
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's that about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For more info on the issue and fix: apollographql/apollo-client#6603

I brought it up in slack too: https://seasonsnyc.slack.com/archives/CMJE9QVA5/p1611168937010600

@kierangillen kierangillen merged commit c6f3795 into master Feb 4, 2021
@kierangillen kierangillen deleted the feature/collection-rail branch February 4, 2021 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants