From 957ebd205d3853984c6c5d1ac2b93e3523fb0667 Mon Sep 17 00:00:00 2001 From: Alex Florisca Date: Tue, 9 Nov 2021 16:49:31 +0000 Subject: [PATCH 1/6] Fix error on the Cart block --- .../base/context/hooks/use-customer-data.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/assets/js/base/context/hooks/use-customer-data.ts b/assets/js/base/context/hooks/use-customer-data.ts index 98dcce60c2d..d9cad0627d8 100644 --- a/assets/js/base/context/hooks/use-customer-data.ts +++ b/assets/js/base/context/hooks/use-customer-data.ts @@ -95,6 +95,39 @@ export const useCustomerData = (): { // Store values last sent to the server in a ref to avoid requests unless important fields are changed. const previousCustomerData = useRef< CustomerData >( customerData ); + // Need to sync shipping and billing address from wp/store/cart to local state, because `customerData` is + // populated before the wc/store/cart is hydrated. We only need to run this the first time they're out of sync + // because subsequent times will be the result of this effect or client side changes. + const [ hasCustomerDataSynced, setHasCustomerDataSynced ] = useState< + boolean + >( false ); + + useEffect( () => { + if ( + ! hasCustomerDataSynced && + ( ! isShallowEqual( + customerData.billingData, + initialBillingAddress + ) || + ! isShallowEqual( + customerData.shippingAddress, + initialShippingAddress + ) ) + ) { + setHasCustomerDataSynced( true ); + const newCustomerData = { + shippingAddress: initialShippingAddress, + billingData: initialBillingAddress, + }; + previousCustomerData.current = newCustomerData; + setCustomerData( newCustomerData ); + } + }, [ + initialBillingAddress, + initialShippingAddress, + customerData, + hasCustomerDataSynced, + ] ); // Debounce updates to the customerData state so it's not triggered excessively. const [ debouncedCustomerData ] = useDebounce( customerData, 1000, { // Default equalityFn is prevData === newData. From b2b03c7fc262a9ef04de8547b92c04b25b58a5f2 Mon Sep 17 00:00:00 2001 From: Alex Florisca Date: Wed, 10 Nov 2021 16:17:00 +0000 Subject: [PATCH 2/6] Created a cartIsHydrated variable in useStoreCart hook and used this to update the billing address in the internal state of the useCustomerData hook --- .../base/context/hooks/cart/use-store-cart.ts | 12 ++++++++++++ .../base/context/hooks/use-customer-data.ts | 19 +++++-------------- assets/js/types/type-defs/hooks.ts | 1 + 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/assets/js/base/context/hooks/cart/use-store-cart.ts b/assets/js/base/context/hooks/cart/use-store-cart.ts index c59602fed09..552899a91df 100644 --- a/assets/js/base/context/hooks/cart/use-store-cart.ts +++ b/assets/js/base/context/hooks/cart/use-store-cart.ts @@ -116,6 +116,7 @@ export const defaultCartData: StoreCart = { paymentRequirements: EMPTY_PAYMENT_REQUIREMENTS, receiveCart: () => undefined, extensions: EMPTY_EXTENSIONS, + cartIsHydrated: false, }; /** @@ -138,6 +139,7 @@ export const useStoreCart = ( const previewCart = previewData?.previewCart; const { shouldSelect } = options; const currentResults = useRef(); + const cartIsHydrated = useRef< boolean >( false ); // This will keep track of jQuery and DOM events triggered by other blocks // or components and will invalidate the store resolution accordingly. @@ -174,6 +176,7 @@ export const useStoreCart = ( typeof previewCart?.receiveCart === 'function' ? previewCart.receiveCart : () => undefined, + cartIsHydrated: true, }; } @@ -184,6 +187,14 @@ export const useStoreCart = ( const cartIsLoading = ! store.hasFinishedResolution( 'getCartData' ); + + if ( + ! cartIsHydrated.current && + store.hasFinishedResolution( 'getCartData' ) + ) { + cartIsHydrated.current = true; + } + const shippingRatesLoading = store.isCustomerDataUpdating(); const { receiveCart } = dispatch( storeKey ); const billingAddress = decodeValues( cartData.billingAddress ); @@ -230,6 +241,7 @@ export const useStoreCart = ( cartHasCalculatedShipping: cartData.hasCalculatedShipping, paymentRequirements: cartData.paymentRequirements, receiveCart, + cartIsHydrated: cartIsHydrated.current, }; }, [ shouldSelect ] diff --git a/assets/js/base/context/hooks/use-customer-data.ts b/assets/js/base/context/hooks/use-customer-data.ts index d9cad0627d8..f5deba47bcc 100644 --- a/assets/js/base/context/hooks/use-customer-data.ts +++ b/assets/js/base/context/hooks/use-customer-data.ts @@ -82,8 +82,10 @@ export const useCustomerData = (): { const { billingAddress: initialBillingAddress, shippingAddress: initialShippingAddress, + cartIsHydrated, }: Omit< CustomerData, 'billingData' > & { billingAddress: CartResponseBillingAddress; + cartIsHydrated: boolean; } = useStoreCart(); // State of customer data is tracked here from this point, using the initial values from the useStoreCart hook. @@ -103,29 +105,18 @@ export const useCustomerData = (): { >( false ); useEffect( () => { - if ( - ! hasCustomerDataSynced && - ( ! isShallowEqual( - customerData.billingData, - initialBillingAddress - ) || - ! isShallowEqual( - customerData.shippingAddress, - initialShippingAddress - ) ) - ) { - setHasCustomerDataSynced( true ); + if ( cartIsHydrated && ! hasCustomerDataSynced ) { const newCustomerData = { shippingAddress: initialShippingAddress, billingData: initialBillingAddress, }; - previousCustomerData.current = newCustomerData; setCustomerData( newCustomerData ); + setHasCustomerDataSynced( true ); } }, [ + cartIsHydrated, initialBillingAddress, initialShippingAddress, - customerData, hasCustomerDataSynced, ] ); // Debounce updates to the customerData state so it's not triggered excessively. diff --git a/assets/js/types/type-defs/hooks.ts b/assets/js/types/type-defs/hooks.ts index efe3d40dee9..d03ceec7c80 100644 --- a/assets/js/types/type-defs/hooks.ts +++ b/assets/js/types/type-defs/hooks.ts @@ -51,4 +51,5 @@ export interface StoreCart { cartHasCalculatedShipping: boolean; paymentRequirements: Array< string >; receiveCart: ( cart: CartResponse ) => void; + cartIsHydrated: boolean; } From 216bd8a3f849ae7d5cfe9a2be2a4a15b2184a94d Mon Sep 17 00:00:00 2001 From: Alex Florisca Date: Thu, 11 Nov 2021 13:01:24 +0000 Subject: [PATCH 3/6] Fix the country is required error on the Cart page using refs --- .../base/context/hooks/cart/use-store-cart.ts | 11 ---- .../base/context/hooks/use-customer-data.ts | 50 ++++++++++--------- assets/js/types/type-defs/hooks.ts | 1 - 3 files changed, 27 insertions(+), 35 deletions(-) diff --git a/assets/js/base/context/hooks/cart/use-store-cart.ts b/assets/js/base/context/hooks/cart/use-store-cart.ts index 552899a91df..7430f0978bc 100644 --- a/assets/js/base/context/hooks/cart/use-store-cart.ts +++ b/assets/js/base/context/hooks/cart/use-store-cart.ts @@ -116,7 +116,6 @@ export const defaultCartData: StoreCart = { paymentRequirements: EMPTY_PAYMENT_REQUIREMENTS, receiveCart: () => undefined, extensions: EMPTY_EXTENSIONS, - cartIsHydrated: false, }; /** @@ -139,7 +138,6 @@ export const useStoreCart = ( const previewCart = previewData?.previewCart; const { shouldSelect } = options; const currentResults = useRef(); - const cartIsHydrated = useRef< boolean >( false ); // This will keep track of jQuery and DOM events triggered by other blocks // or components and will invalidate the store resolution accordingly. @@ -176,7 +174,6 @@ export const useStoreCart = ( typeof previewCart?.receiveCart === 'function' ? previewCart.receiveCart : () => undefined, - cartIsHydrated: true, }; } @@ -188,13 +185,6 @@ export const useStoreCart = ( 'getCartData' ); - if ( - ! cartIsHydrated.current && - store.hasFinishedResolution( 'getCartData' ) - ) { - cartIsHydrated.current = true; - } - const shippingRatesLoading = store.isCustomerDataUpdating(); const { receiveCart } = dispatch( storeKey ); const billingAddress = decodeValues( cartData.billingAddress ); @@ -241,7 +231,6 @@ export const useStoreCart = ( cartHasCalculatedShipping: cartData.hasCalculatedShipping, paymentRequirements: cartData.paymentRequirements, receiveCart, - cartIsHydrated: cartIsHydrated.current, }; }, [ shouldSelect ] diff --git a/assets/js/base/context/hooks/use-customer-data.ts b/assets/js/base/context/hooks/use-customer-data.ts index f5deba47bcc..e1cf0d1e352 100644 --- a/assets/js/base/context/hooks/use-customer-data.ts +++ b/assets/js/base/context/hooks/use-customer-data.ts @@ -82,43 +82,47 @@ export const useCustomerData = (): { const { billingAddress: initialBillingAddress, shippingAddress: initialShippingAddress, - cartIsHydrated, }: Omit< CustomerData, 'billingData' > & { billingAddress: CartResponseBillingAddress; - cartIsHydrated: boolean; } = useStoreCart(); + // We need to store a ref to the shipping and billing addresses in order to re-populate the local state + // here with the hydrated values once they change. The refs keep track of the changing values of the addresses + // as they are hydrated. + const billingDataRef = useRef( {} ); + const shippingAddressRef = useRef( {} ); + billingDataRef.current = initialBillingAddress; + shippingAddressRef.current = initialShippingAddress; + // State of customer data is tracked here from this point, using the initial values from the useStoreCart hook. const [ customerData, setCustomerData ] = useState< CustomerData >( { billingData: initialBillingAddress, shippingAddress: initialShippingAddress, } ); - // Store values last sent to the server in a ref to avoid requests unless important fields are changed. - const previousCustomerData = useRef< CustomerData >( customerData ); - - // Need to sync shipping and billing address from wp/store/cart to local state, because `customerData` is - // populated before the wc/store/cart is hydrated. We only need to run this the first time they're out of sync - // because subsequent times will be the result of this effect or client side changes. + // We only want to update the local state once, otherwise the data on the checkout page gets overwritten + // with the initial state of the addresses here const [ hasCustomerDataSynced, setHasCustomerDataSynced ] = useState< boolean >( false ); - useEffect( () => { - if ( cartIsHydrated && ! hasCustomerDataSynced ) { - const newCustomerData = { - shippingAddress: initialShippingAddress, - billingData: initialBillingAddress, - }; - setCustomerData( newCustomerData ); - setHasCustomerDataSynced( true ); - } - }, [ - cartIsHydrated, - initialBillingAddress, - initialShippingAddress, - hasCustomerDataSynced, - ] ); + if ( + ! hasCustomerDataSynced && + shouldUpdateAddressStore( + billingDataRef.current, + customerData.billingData + ) + ) { + setCustomerData( { + billingData: billingDataRef.current, + shippingAddress: shippingAddressRef.current, + } ); + setHasCustomerDataSynced( true ); + } + + // Store values last sent to the server in a ref to avoid requests unless important fields are changed. + const previousCustomerData = useRef< CustomerData >( customerData ); + // Debounce updates to the customerData state so it's not triggered excessively. const [ debouncedCustomerData ] = useDebounce( customerData, 1000, { // Default equalityFn is prevData === newData. diff --git a/assets/js/types/type-defs/hooks.ts b/assets/js/types/type-defs/hooks.ts index d03ceec7c80..efe3d40dee9 100644 --- a/assets/js/types/type-defs/hooks.ts +++ b/assets/js/types/type-defs/hooks.ts @@ -51,5 +51,4 @@ export interface StoreCart { cartHasCalculatedShipping: boolean; paymentRequirements: Array< string >; receiveCart: ( cart: CartResponse ) => void; - cartIsHydrated: boolean; } From cdb36f13d33923a452ebbcec97e6fd09eb51a556 Mon Sep 17 00:00:00 2001 From: Alex Florisca Date: Fri, 12 Nov 2021 17:09:34 +0000 Subject: [PATCH 4/6] Separate api calls to update shipping and billingUpdate billing and shipping addresses only when needed in API calls --- .../base/context/hooks/use-customer-data.ts | 61 +++++++++++++------ assets/js/data/cart/actions.ts | 10 +-- assets/js/types/type-defs/cart.ts | 5 ++ 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/assets/js/base/context/hooks/use-customer-data.ts b/assets/js/base/context/hooks/use-customer-data.ts index e1cf0d1e352..9cd83d267d9 100644 --- a/assets/js/base/context/hooks/use-customer-data.ts +++ b/assets/js/base/context/hooks/use-customer-data.ts @@ -11,9 +11,13 @@ import { pluckAddress, pluckEmail, } from '@woocommerce/base-utils'; -import type { +import { CartResponseBillingAddress, CartResponseShippingAddress, + objectHasProp, + BillingAddressShippingAddress, + CartBillingAddress, + CartShippingAddress, } from '@woocommerce/types'; declare type CustomerData = { @@ -89,8 +93,8 @@ export const useCustomerData = (): { // We need to store a ref to the shipping and billing addresses in order to re-populate the local state // here with the hydrated values once they change. The refs keep track of the changing values of the addresses // as they are hydrated. - const billingDataRef = useRef( {} ); - const shippingAddressRef = useRef( {} ); + const billingDataRef = useRef( initialBillingAddress ); + const shippingAddressRef = useRef( initialShippingAddress ); billingDataRef.current = initialBillingAddress; shippingAddressRef.current = initialShippingAddress; @@ -109,8 +113,8 @@ export const useCustomerData = (): { if ( ! hasCustomerDataSynced && shouldUpdateAddressStore( - billingDataRef.current, - customerData.billingData + customerData.shippingAddress, + shippingAddressRef.current ) ) { setCustomerData( { @@ -174,23 +178,46 @@ export const useCustomerData = (): { */ useEffect( () => { // Only push updates when enough fields are populated. + const shouldUpdateBillingAddress = shouldUpdateAddressStore( + previousCustomerData.current.billingData, + debouncedCustomerData.billingData + ); + + const shouldUpdateShippingAddress = shouldUpdateAddressStore( + previousCustomerData.current.shippingAddress, + debouncedCustomerData.shippingAddress + ); + + if ( ! shouldUpdateBillingAddress && ! shouldUpdateShippingAddress ) { + return; + } + + const customerDataToUpdate: + | Partial< BillingAddressShippingAddress > + | Record< + keyof BillingAddressShippingAddress, + CartBillingAddress | CartShippingAddress + > = {}; + + if ( shouldUpdateBillingAddress ) { + customerDataToUpdate.billing_address = + debouncedCustomerData.billingData; + } + if ( shouldUpdateShippingAddress ) { + customerDataToUpdate.shipping_address = + debouncedCustomerData.shippingAddress; + } + if ( - ! shouldUpdateAddressStore( - previousCustomerData.current.billingData, - debouncedCustomerData.billingData - ) && - ! shouldUpdateAddressStore( - previousCustomerData.current.shippingAddress, - debouncedCustomerData.shippingAddress - ) + ! objectHasProp( customerDataToUpdate, 'billing_address' ) && + ! objectHasProp( customerDataToUpdate, 'shipping_address' ) ) { return; } previousCustomerData.current = debouncedCustomerData; - updateCustomerData( { - billing_address: debouncedCustomerData.billingData, - shipping_address: debouncedCustomerData.shippingAddress, - } ) + updateCustomerData( + customerDataToUpdate as Partial< BillingAddressShippingAddress > + ) .then( () => { removeNotice( 'checkout' ); } ) diff --git a/assets/js/data/cart/actions.ts b/assets/js/data/cart/actions.ts index 7b276a1aba6..253c8b3ffd7 100644 --- a/assets/js/data/cart/actions.ts +++ b/assets/js/data/cart/actions.ts @@ -6,9 +6,8 @@ import type { Cart, CartResponse, CartResponseItem, - CartBillingAddress, - CartShippingAddress, ExtensionCartUpdateArgs, + BillingAddressShippingAddress, } from '@woocommerce/types'; import { ReturnOrGeneratorYieldUnion } from '@automattic/data-stores'; import { camelCase, mapKeys } from 'lodash'; @@ -463,11 +462,6 @@ export function* selectShippingRate( return true; } -type BillingAddressShippingAddress = { - billing_address: CartBillingAddress; - shipping_address: CartShippingAddress; -}; - /** * Updates the shipping and/or billing address for the customer and returns an * updated cart. @@ -476,7 +470,7 @@ type BillingAddressShippingAddress = { * billing_address and shipping_address. */ export function* updateCustomerData( - customerData: BillingAddressShippingAddress + customerData: Partial< BillingAddressShippingAddress > ): Generator< unknown, boolean, { response: CartResponse } > { yield updatingCustomerData( true ); diff --git a/assets/js/types/type-defs/cart.ts b/assets/js/types/type-defs/cart.ts index c04cb3a0e24..ce8cbe83ed0 100644 --- a/assets/js/types/type-defs/cart.ts +++ b/assets/js/types/type-defs/cart.ts @@ -198,3 +198,8 @@ export interface ExtensionCartUpdateArgs { data: Record< string, unknown >; namespace: string; } + +export interface BillingAddressShippingAddress { + billing_address: CartBillingAddress; + shipping_address: CartShippingAddress; +} From 23a584e2a29d45999b9857a654120685a53ee132 Mon Sep 17 00:00:00 2001 From: Alex Florisca Date: Mon, 15 Nov 2021 13:15:38 +0000 Subject: [PATCH 5/6] Remove redundant check for customerDataToUpdate --- assets/js/base/context/hooks/use-customer-data.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/assets/js/base/context/hooks/use-customer-data.ts b/assets/js/base/context/hooks/use-customer-data.ts index 9cd83d267d9..c3dea2b9c77 100644 --- a/assets/js/base/context/hooks/use-customer-data.ts +++ b/assets/js/base/context/hooks/use-customer-data.ts @@ -14,7 +14,6 @@ import { import { CartResponseBillingAddress, CartResponseShippingAddress, - objectHasProp, BillingAddressShippingAddress, CartBillingAddress, CartShippingAddress, @@ -208,12 +207,6 @@ export const useCustomerData = (): { debouncedCustomerData.shippingAddress; } - if ( - ! objectHasProp( customerDataToUpdate, 'billing_address' ) && - ! objectHasProp( customerDataToUpdate, 'shipping_address' ) - ) { - return; - } previousCustomerData.current = debouncedCustomerData; updateCustomerData( customerDataToUpdate as Partial< BillingAddressShippingAddress > From ee84975ce2d40a741cb081c14df18c6e7812f500 Mon Sep 17 00:00:00 2001 From: Nadir Seghir Date: Tue, 16 Nov 2021 12:36:16 +0100 Subject: [PATCH 6/6] remove use of refs in initial values --- assets/js/base/context/hooks/use-customer-data.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/assets/js/base/context/hooks/use-customer-data.ts b/assets/js/base/context/hooks/use-customer-data.ts index c3dea2b9c77..2f7fec296e5 100644 --- a/assets/js/base/context/hooks/use-customer-data.ts +++ b/assets/js/base/context/hooks/use-customer-data.ts @@ -89,14 +89,6 @@ export const useCustomerData = (): { billingAddress: CartResponseBillingAddress; } = useStoreCart(); - // We need to store a ref to the shipping and billing addresses in order to re-populate the local state - // here with the hydrated values once they change. The refs keep track of the changing values of the addresses - // as they are hydrated. - const billingDataRef = useRef( initialBillingAddress ); - const shippingAddressRef = useRef( initialShippingAddress ); - billingDataRef.current = initialBillingAddress; - shippingAddressRef.current = initialShippingAddress; - // State of customer data is tracked here from this point, using the initial values from the useStoreCart hook. const [ customerData, setCustomerData ] = useState< CustomerData >( { billingData: initialBillingAddress, @@ -113,12 +105,12 @@ export const useCustomerData = (): { ! hasCustomerDataSynced && shouldUpdateAddressStore( customerData.shippingAddress, - shippingAddressRef.current + initialShippingAddress ) ) { setCustomerData( { - billingData: billingDataRef.current, - shippingAddress: shippingAddressRef.current, + billingData: initialBillingAddress, + shippingAddress: initialShippingAddress, } ); setHasCustomerDataSynced( true ); }