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

STRF-5640 - Load cart quantity in FE #1379

Merged
merged 1 commit into from Dec 11, 2018

Conversation

Projects
None yet
5 participants
@bookernath
Copy link
Contributor

bookernath commented Nov 7, 2018

What?

This is the first step in improving storefront performance by moving most/all Cart interactions to the frontend.

This change moves all functionality related to calculating the cart quantity in the header to FE API calls, after the initial HTML loads.

In future PRs, we'll be able to actually turn off the loading of Cart details in the HTML in Cornerstone, which will improve performance, especially for large carts. However, this PR does not disable Cart data, it simply introduces the FE behavior.

The "last known" cart quantity is cached in localStorage so it loads quickly when the page loads and then it's quietly updated if new information is received from the API. There is a graceful fallback if localStorage is unavailable in the environment or otherwise fails (e.g. if the 5MB storage limit is already reached for the domain).

Tickets / Documentation

@bigbot

This comment has been minimized.

Copy link

bigbot commented Nov 7, 2018

@bookernath bookernath changed the title Load cart quantity in FE (RFC) Load cart quantity in FE Nov 7, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch 2 times, most recently from 25a8d79 to 97fe643 Nov 7, 2018

@bookernath bookernath changed the title (RFC) Load cart quantity in FE Load cart quantity in FE Nov 7, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 97fe643 to 3513b5f Nov 7, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 3513b5f to 5b40815 Nov 19, 2018

Show resolved Hide resolved assets/js/theme/global/cart-preview.js Outdated
Show resolved Hide resolved assets/js/theme/global/cart-preview.js Outdated
}

// Get updated cart quantity from the Cart API
const cartQtyPromise = new Promise(resolve => {

This comment has been minimized.

@mattolson

mattolson Nov 27, 2018

Member

I just noticed that the getCartQuantity function in stencil-utils does not handle api errors properly.

https://github.com/bigcommerce/stencil-utils/blob/master/src/api/cart.js#L26 should be modified to check for an error and then call callback(err). When successful, it should call callback(null, quantity). The standard for JS callback functions is that the first param the error and the second param is the result.

Once that is done, this PR should be modified to handle the error and call the reject function in the Promise.

This comment has been minimized.

@bookernath

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 5b40815 to 1acaf81 Dec 3, 2018

Show resolved Hide resolved assets/js/theme/global.js Outdated

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 6b862d3 to 6501ee1 Dec 3, 2018

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 3, 2018

@mattolson ♻️

Please note that this requires stencil-utils 3.0.0 (containing bigcommerce/stencil-utils#88) to be released to NPM before we can merge this.

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 6501ee1 to 997eaa1 Dec 3, 2018

@mattolson

This comment has been minimized.

Copy link
Member

mattolson commented Dec 3, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 997eaa1 to 7d87cd3 Dec 3, 2018

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 3, 2018

Now depends on bigcommerce/stencil-utils#90 and I've targeted this PR at 3.1.0

@bookernath bookernath changed the title Load cart quantity in FE STRF-5640 - Load cart quantity in FE Dec 3, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 7d87cd3 to 20315aa Dec 3, 2018

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 3, 2018

@mattolson ♻️ once more :)

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 3, 2018

This can't be merged yet; found some weird behavior on HTTP/shared SSL storefronts.

@bookernath

This comment has been minimized.

Copy link
Contributor Author

bookernath commented Dec 7, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch from 217c006 to f168949 Dec 7, 2018

@bookernath bookernath force-pushed the bookernath:fe-cart branch from f168949 to db3ad5a Dec 10, 2018

@junedkazi junedkazi merged commit 7386c29 into bigcommerce:master Dec 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@bookernath bookernath deleted the bookernath:fe-cart branch Dec 11, 2018

@carsonreinke

This comment has been minimized.

Copy link

carsonreinke commented Dec 12, 2018

For your consideration #1405 (comment)

@carsonreinke

This comment has been minimized.

Copy link

carsonreinke commented Dec 12, 2018

@bookernath

@carsonreinke can you give an example of what you mean?

GTM is our biggest use case. Delaying pushing data for the data layer changes what trigger executes the tag. The tag cannot fire on page load, DOM load, or window loaded. A custom event would need to be added to trigger once the API call came back.

Just to make sure I'm articulating this clearly, we don't plan to do anything with the existing {{cart}} resource - we intend to keep supporting it and all the existing themes and stores that might be using it.

However, towards the goal of making the storefront faster, we plan to move the interactions to the frontend and disable the cart object by default in Cornerstone which will result in a faster response time for the page, especially for stores with certain configurations like a lot of discount rules, or shoppers who have a lot of items in their cart.

Anyone who still wants to use the Cart resource can keep it enabled.

Oh I see, so it is opt-in. Well, that kind of solves my problem, so maybe this is just a non-issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment