Upgrade to @apollo/client@3

[sirugh]

https://www.apollographql.com/docs/react/migrating/apollo-client-3-migration/

Notes from upgrade

• Apollo freezes response objects. Must copy/clone before modifying.
• read functions replace local resolvers
• merge functions handle ambiguous normalization (ie. arrays of data, objects with no keys)
  • Be on the lookout for apollo client warnings about problems merging. These are usually found through manual steps and I've tried to capture and fix all that I could but it is possible I have missed some.

TODO:

• Any PRs merged that use react-hooks or v2 syntax will need to get updated on this PR.
• Upgrade to 3.2.0 for global next fetch policy (or open a followup ticket).
• Double check package.json to ensure new client is added and old deps are removed.
• Migrate gift option resolvers to type policiesDone
• Migrate payment information resolvers to type policies Done
• Convert to possibleTypes from dataFromObject - Done
• Fix PDP Overfetching - Resolved by removing manual cache lookups in useProduct
• Fix Cart Overfetching (and/or blowing away old data ie region/country). Could not repro anymore.
• Release/Update apollo-link-mutation-queue since operation.toKey is no longer a function. Done.
• Handle skip bug. - [email protected] resolved this.
• graphiql schema view breaks now This appears broken on develop.

[PWAStudioBot]

	Fails
🚫	Missing "Description" section. Please add it back, with detail.
🚫	Missing "Verification Steps" section. Please add it back, with detail.
🚫	No linked issue found. Please link a relevant open issue by adding the text "closes #<issue_number>" or "closes JIRA-<issue_number>" in your PR.

	Warnings
⚠️	Found the word "TODO" in the PR description. Just letting you know incase you forgot :)

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

If your PR is missing information, check against the original template here. At a minimum you must have the section headers from the template and provide some information in each section.

Generated by 🚫 dangerJS against d1877ca

[devops-pwa-codebuild]

Performance Test Results

The following fails have been reported by WebpageTest. These numbers indicates a possible performance issue with the PR which requires further manual testing to validate.

https://pr-2560.pwa-venia.com : LH Performance Expected 0.85 Actual 0.35, LH Accessibility Expected 1 Actual 0.97, LH Best Practices Expected 1 Actual 0.93, WPT Cache Expected 90 Actual 35.666666666667
https://pr-2560.pwa-venia.com/venia-tops.html : LH Performance Expected 0.75 Actual 0.31, LH Best Practices Expected 1 Actual 0.93
https://pr-2560.pwa-venia.com/valeria-two-layer-tank.html : LH Performance Expected 0.8 Actual 0.57, LH Accessibility Expected 0.9 Actual 0.89, LH Best Practices Expected 1 Actual 0.93, WPT Cache Expected 65 Actual 44

[supernova-at]

This answers one of my previous questions, thanks for the context 👍

[supernova-at]

Interesting that they chose to make gql the default export of @apollo/client.

[supernova-at]

After looking at the other imports, this one needs to be changed.

import { gql } from '@apollo/client';

But actually, I don't even know what this file is. Is this generated code?

[sirugh]

Honestly, not sure if this file is even used at all. I'll poke around, but yes this does need to be changed to at least:

const apolloClient = require('apollo/client');
const { gql } = apolloClient;

[sirugh]

8eafc91

[supernova-at]

This is actually pretty readable

[supernova-at]

May want to drop a comment where POSSIBLE_TYPES comes from since it's not otherwise present in this file

[sirugh]

Done 8eafc91

[supernova-at]

Makes me think we should update our Mutation PII Cheat Sheet.

[sirugh]

Not sure if the use of the policy for the Cart field warrants the update, but I think I get where you're going with this. We could preempt this and move all the connection directives from mutations to this policies file. For example this mutation:

export const REMOVE_GIFT_CARD_MUTATION = gql`
    mutation removeGiftCard($cartId: String!, $giftCardCode: String!) {
        removeGiftCardFromCart(
            input: { cart_id: $cartId, gift_card_code: $giftCardCode }
        ) @connection(key: "removeGiftCardFromCart") {
            ...
       }
    }
`;

would become

{
   Mutation: {
       fields: {
           removeGiftCardFromCart: {
               keyArgs: () => 'removeGiftCardFromCart'
           }
       }
   }
}

Then the rule in the cheatsheet, and something we'd have to enforce in reviews, would be to add a field keyArg rule to the policies file when adding a new mutation.

I'd like to get opinions on that though.

[sirugh]

[sirugh]

Thoughts:

1. Have each component use the useTypePolicies hook to add type policy keys for their mutations at runtime. This would prevent initial bundle bloat and aid in chunking.

2. Declare all mutation keys up front. Adds bytes to bundle and hurts chunking if we aren't using all mutations.
   note: We could generate this automatically from the schema if we knew what fields we consider "pii".

3. Use a global no-cache policy for mutations. This would remove the need to do any of this, but would force us to refactor our app because we use mutation results to auto update cache. We would have to switch to using refetchQueries after each mutation.

[supernova-at]

I never know where to draw that line.

1. This is going to be a major change anyway, right?
2. When do we actually remove this code?

[jimbo]

This is good from my end. There are still minor things to resolve, but I'm good when others' feedback is resolved.

[jimbo]

1. Yes, it's absolutely going to be a major change.
2. We haven't actually formalized a policy for removal, only deprecation.

I think the pacing of our major releases is a bit too fast for us to change code from "canonical" to "deprecated" to "removed" just like that. Now that we're properly marking things with @deprecated, we can start the conversation around an actual removal timeframe.

[tjwiebell]

Nice improvements with making type policies composable, this looks ready for QA 👍