-
Notifications
You must be signed in to change notification settings - Fork 81
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
Upgrade ApolloClient package to version 3 #424
Conversation
- The product part of the cart query needs to be identical across mutations and queries
Codecov Report
@@ Coverage Diff @@
## master #424 +/- ##
============================================
+ Coverage 84.96% 85.17% +0.20%
Complexity 1054 1054
============================================
Files 216 219 +3
Lines 5428 5416 -12
Branches 781 780 -1
============================================
+ Hits 4612 4613 +1
+ Misses 664 651 -13
Partials 152 152
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
||
const emptyCartId = 'empty'; | ||
const mockCartId = '123ABC'; | ||
const mockShippingAddress = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should improve this mechanism a bit, like have these mocks "collected" at runtime from a file structure instead of just piling them into this file.
}); | ||
} | ||
}); | ||
const client = new ApolloClient(clientConfig); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this client
supposed to come from the custom application?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume, if a customer uses their own ApolloClient
, they wouldn't use CommerceApp
anymore, but add their own ApolloProvider
together with a mix of their own and our contexts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this defies the purpose of the CommerceApp
. As soon as they want to have a custom component that wants to fetch data they have to refactor all their code to drop CommerceApp
and re-create our provider stack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can modify CommerceApp
to receive the client as a prop
. if that prop is not provided, then a default client will be created.
Description
Removed
@magento/peregrine
dependencyUpgraded Apollo client to version 3
Related Issue
CIF-1633
Motivation and Context
Apollo GraphQL client version 3 is required for the GET requests
How Has This Been Tested?
Unit tests
Screenshots (if appropriate):
Types of changes
Checklist: