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

AC-11 Support AC local state #166

Merged
merged 10 commits into from
Feb 26, 2019
Merged

Conversation

cheapsteak
Copy link
Member

@cheapsteak cheapsteak commented Jan 18, 2019

99.9% of this was done by Hugh, really appreciated the extremely thorough comments
I just moved a few lines around and double checked that it's working against our full stack tutorial (which uses AC local state) and pupstagram (which uses link-state)

I did find that running link-state mutations doesn't work (in the case of pupstagram, running the toggleLikedPhoto mutation results in a getCacheKey is not a function error), but it's broken the same way in the currently published version of devtools as well, so it's likely that it hasn't worked in the past

I'm not sure if there's much else worth saying here. The meat of the changes is in links.js, and Hugh's comments do a great job of explaining what and why

@cheapsteak cheapsteak changed the title Support AC local state AC-11 Support AC local state Jan 18, 2019
Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

This all looks great @cheapsteak - thanks very much for working on this!

@hwillson
Copy link
Member

@cheapsteak Let's leave this un-merged for now, just until we know the official launch date of the Apollo Client local state changes. Thanks again!

// is being used.

const supportsApolloClientLocalState =
typeof apolloClient.getTypeDefs === "function";
Copy link
Member

Choose a reason for hiding this comment

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

@hwillson Just a note that apollographql/apollo-client@d3931b8 may have broken this logic (which is fine as long as we make the logic work again before we ship apollo-client@2.5.0).

Copy link
Member

Choose a reason for hiding this comment

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

With this commit, the new way to get the client typeDefs will be apolloClient.typeDefs: apollographql/apollo-client@adfd9c0

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you @benjamn ! @hwillson had warned me that getTypeDefs was going to be replaced so I've been ready for the breaking change 😃 If this will be released in the latest alpha I'll update the DevTools PR and merge.

@justinanastos justinanastos force-pushed the hugh/chang/ac-local-state-changes branch 3 times, most recently from 32b6113 to 4333c28 Compare February 26, 2019 15:12
src/backend/links.js Outdated Show resolved Hide resolved
@justinanastos justinanastos force-pushed the hugh/chang/ac-local-state-changes branch 2 times, most recently from f4339ca to ac35b34 Compare February 26, 2019 16:27
@justinanastos justinanastos force-pushed the hugh/chang/ac-local-state-changes branch from ac35b34 to 75d24e4 Compare February 26, 2019 16:45
The version we were using before couldn't `parse` the `ObjectTypeExtension`
type.
@justinanastos justinanastos merged commit 11714c9 into master Feb 26, 2019
@justinanastos justinanastos deleted the hugh/chang/ac-local-state-changes branch February 26, 2019 17:55
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.

None yet

4 participants