Shopify / js-buy-sdk

The JS Buy SDK is a lightweight library that allows you to build ecommerce into any website. It is based on Shopify's API and provides the ability to retrieve products and collections from your shop, add products to a cart, and checkout.
https://shopify.github.io/js-buy-sdk
MIT License
989 stars 261 forks source link

[RFC] Cart Interface #1

Closed minasmart closed 8 years ago

minasmart commented 8 years ago

Preface

Hey all! This issue exists to get your input on some interface design for the js-buy-sdk. I'm asking y'all since some of you are likely to be early SDK consumers. First, I'd like to mention that the code in this repo is still under active development and is a bit of a prototype. Right now it supports fetching from the publications API and fetching and persisting to the checkouts anywhere API (TBD). This has been done in an extensible way so that at some point in the future other data types or APIs may integrated if that serves a need.

This SDK exists as the basis for moving Buy Button away from iframes and into a more customizable and usable open source SDK. Buy button will be replatformed onto this SDK.

If you're not familiar with this project, what purpose it serves, and the motivations behind it, please checkout out:

NOTE: Project name and exported global are still TBD.

User interaction

The following are some examples of interactions around adding items to a cart.

Functional modeling

const client = ShopifyBuy.createClient({ ... });
const $buyButton = $('#buy-button');

let cart = client.createCart();

client.fetchProduct(123).then(product => {

  setUpUIForVariant(product.variants[1]);

  $buyButton.on('click', () => {
    shopClient.addVariantToCart(cart, { id: product.variants[1].id, quantity: 1 });
    shopClient.save(cart).then(updateCart => {
      cart = updateCart;
    });
  });
});

In this example, mutator functions are attached to the shopClient, which is what's responsible for interacting with a specific API.

In this case addVariantToCart could just augments the carts state. Persistence is handled in a separate call. Another option would be to have addVariantToCart make the call to persist the cart, and return a promise that resolves with the new cart. If it resolves a new cart, instead of a mutated cart this eliminates the possibility of race conditions (checking cart state while network requests are in flight could produce inconsistent or unreliable state).

OO Modelling

const client = ShopifyBuy.createClient({ ... });
const cart = client.createCart();
const $buyButton = $('#buy-button');

client.fetchProduct(123).then(product => {

  setUpUIForVariant(product.variants[1]);

  $buyButton.on('click', () => {
    cart.addVariant({ id: product.variants[1].id, quantity: 1 });
    cart.save();
  });
});

This approach implies that the existing cart would be altered by the call, and necessitates more state management. Whether or not addVariant saves or we require an explicit save call, this approach necessitates state management (the cart would have to be marked as in-flight, and subsequent calls to save would have to be queued after the initial save resolves). We could also just not worry about race conditions and just assume last-write-wins and the carts internal state will eventually resolve.

How do we represent the cart?

As shown above, we can use a factory method that can create a cart, that an SDK consumer is responsible for maintaining.

An alternative would be to have a singleton cart, that's created, resolved, or fetched:

shopClient.myCart().then(cart => {
  // Do some work with the cart
});

myCart would check for an instance attached to the cart client; failing that it would check a cookie or local storage for a cart token, fetch the cart and resolve; and failing that make a create call to the server and resolve the newly created cart. I think this approach is pretty convenient, but it may be more opinionated than what we want to build with this API.

Simpler use cases

In other cases where a cart object isn't necessary, we can create checkout permalinks based on variants and quantities.


const client = ShopifyBuy.createClient({ ... });
const $buyButton = $('#buy-button');

client.fetchProduct(123).then(product => {

  setUpUIForVariant(product.variants[1]);

  $buyButton.on('click', () => {
    const checkout = window.open();

    const link = shopClient.checkoutPermalink([
      { id: product.variants[1], quantity: 1 }
    ]);

    checkout.location.href = link;
  });
});

In the case of buy button, this approach would work in some cases. Variant ID and quantity would be pre-set by the buy-button builder, and the product fetch would only be used to get product name, price and image to fill in the display.

Other recommendations!

I'm looking for all of your input so we can come up with a decent public-facing API. The fundamentals for either of these approaches are built, it's just a matter of wrapping them up in some convenience methods so nobody has to work directly with hashes.

This is just the starting point of a discussion. All of your feedback questions and proposed use-cases are welcomed.

I look forward to hearing from you all!

minasmart commented 8 years ago

CC: @richgilbank @tessalt @m-ux @AWaselnuk @lemonmade @jamesmacaulay @mpiotrowicz @stevenmichaelthomas @nwtn

nwtn commented 8 years ago

Unfortunately the links for the brief and design doc aren’t working for me.

Another option would be to have addVariantToCart make the call to persist the cart, and return a promise that resolves with the new cart. If it resolves a new cart, instead of a mutated cart this eliminates the possibility of race conditions (checking cart state while network requests are in flight could produce inconsistent or unreliable state).

I really like this

myCart would check for an instance attached to the cart client; failing that it would check a cookie or local storage for a cart token, fetch the cart and resolve; and failing that make a create call to the server and resolve the newly created cart. I think this approach is pretty convenient, but it may be more opinionated than what we want to build with this API.

To me this seems like a really good place to aim for a possible library built on top of the API, rather than having it baked into the API itself.

minasmart commented 8 years ago

@nwtn Yeah, that part is sort of opinionated. I don't thing it's something the SDK needs to get by, but it may turn out that that's the simplest use case for an api consumer? I could just bundle it as an example maybe?

nwtn commented 8 years ago

@minasmart that would be cool, ya!

minasmart commented 8 years ago

links fixed

minasmart commented 8 years ago

One thing about the addVariantToCart and the persistence returning a new cart: Do you think it would be better to require an explicit save that returns a new cart? Or have each addVariantToCart immediately persist?

tessalt commented 8 years ago

I agree with both @nwtn's points.

I'm on the fence about whether or not to require an explicit save. Leaning towards yes to preserve some flexibility.

nwtn commented 8 years ago

ya i think i like the idea of an explicit save

jamesmacaulay commented 8 years ago

This is great, thanks for putting this together.

For the issue of explicit saves, I think in almost 100% of cases, the client will want to make immediate individual updates via the UI. One option would be to have the main action methods auto-save because that's almost always what you want, but provide a mechanism for batching operations if you need that.

About how much state-tracking the SDK should do: I don't know, but I think the most important thing is that it should be easy to bypass that state-tracking and still be left with an ergonomic interface if you are using an architecture that doesn't benefit from it.

minasmart commented 8 years ago

@jamesmacaulay totally agree. Underpinning whichever direction we take is a very basic interface for CRUD operations, where state is the consumer's concern. Not that of the SDK. So either way, it's very easy for the user to skip all of the convenience methods.

lemonmade commented 8 years ago

While I really like the functional approach to updating the cart, I worry that it will be error prone (particularly in the case where state is never stored in the cart, and a mutated cart is eventually resolved from every operation). All it takes is forgetting to assign the car in one save promise resolution to have a cart that is not what you expected. It's not too hard to imagine forgetting to do the assignment, particularly if the catch case also needs to be handled. I also find it a little awkward to read with all of the boilerplate reassignment (using async/await to flatten it out a bit):

const product = await client.fetchProduct(123)
setUpUIForVariant(product.variants[1]);

client.someCartOperation(cart, {});
const newCart = await shopClient.save(cart);
cart = newCart;

client.someOtherOperation(cart, {});
const newCart = await shopClient.save(cart);
cart = newCart;

What's the general level of skill you expect from developers using this API?

I really like @jamesmacaulay's idea of saving by default but having the escape hatch of not autosaving in cases where you want to batch in a way not provided by the API.

Awesome job with this, I love the way you're presenting the API options, makes it really easy to compare and contrast.

minasmart commented 8 years ago

(particularly in the case where state is never stored in the cart, and a mutated cart is eventually resolved from every operation). All it takes is forgetting to assign the car in one save promise resolution to have a cart that is not what you expected.

State is stored in the cart, but it's state at a time. I don't disagree about the reassignment, but the moment we persist state back to the existing reference, we need to start managing the model's in-flight state, and queueing up subsequent requests. All that may be a trivial thing to do, but it adds a much more complex surface area as to where bugs may come up. So I don't know if we'd rather err on the side of just clearly documenting how the code works (resolves a new cart. Nothing old gets mutated), or make the SDK undeniably more convenient to use, but own that complexity internally.

lemonmade commented 8 years ago

Sorry, I didn't mean that state wasn't stored in the cart, just that with that approach, the internal state would be completely immutable (is that correct)?

I do feel like the right choice comes down to your audience. Are you writing for an audience of strong JavaScript developers? If so, the issues with the more functional API are actually features in that they match closely with the way something like Immutable.js works. If the audience is more broad, I think it's our responsibility to bear the complexity internally, because it's much better than developers introducing bugs that affect merchants' customers.

AWaselnuk commented 8 years ago

Some initial thoughts:

jmwind commented 8 years ago

@gmalette is a cart expert and can verify the semantics of the cart operations. For example, are you allowing multiple of the save variant to be added to the cart?

The reality is that the skill level in the digital agencies who will be using these APIs is lowish, so target junior web developers as customers of the API. The simpler the better.

richgilbank commented 8 years ago

@jmwind To answer some of your questions,

I agree about trying to keep the barrier fairly low for devs - while I prefer the more functional approach, it might be better for us to provide more abstraction over the behaviour to make it easier to adopt. Our primary target for the audience is devs working for Plus shops, in the medium-advanced realm. That said, we also expect to have a long tail of devs working for smaller shops who are less experienced with JS.

gmalette commented 8 years ago

@richgilbank I think what @jmwind was talking about is adding a variant that results in another line in the cart. We allow people to add properties to their line items, so you can add the same variant twice, resulting in two lines. e.g.:

cart.addVariant(snowboard, {quantity: 1, properties: {engraving: "Hello"}})
cart.addVariant(snowboard, {quantity: 1, properties: {engraving: "World"}})

This would result in two distinct lines in the cart.


While I agree that remote calls for updates aren't ideal because they add delay. I have two arguments to offer for having them, at least before we give the cart back to rendering.

1) Automated Discounts & Scripts: The only way to know if a discount applies to the line item in the cart is to actually run them through the scripts and automated discounts. It's likely we'll add the possibility to limit the quantity of items in the cart, too. Failing to run them, the price in the cart will not result the ongoing sales and promotions a merchant is running, and possibly even the quantities would be off.

2) Line item folding rules: We define folding as how we sum similar variants to produce the quantity. See the code here. We currently fold items if they have identical applied_discounts, variant_id, properties, and the more obscure source_indices which you can ignore.

It's almost impossible to have identical folding properties in both the JS and Ruby world.

Example:

cart = new Cart()
cart = cart.addVariant(snowboard, {quantity: 1})
cart = cart.addVariant(snowboard, {quantity: 1})

How many lines does this cart contain? We don't know!

richgilbank commented 8 years ago

@gmalette To clarify, we're not launching with a cart that's persisted to Shopify. We built it out using the Checkout API, but the decision was made to not use Shopify for cart management until we have a proper Cart API.

As for multiple line items for one variant, we will indeed be supporting them. Our behaviour was modelled on Shopify, so we can determine what we think is going to be the expected behaviour to use out of the box, and expose more granular controls over the different resources (variants, line items) as well.

gmalette commented 8 years ago

To clarify, we're not launching with a cart that's persisted to Shopify. We built it out using the Checkout API

Is the cart displayed in any way on the host website? How and when are you displaying discounts that apply to line items?

richgilbank commented 8 years ago

Developers would be implementing their own UI for it, but they would be rendering the local cart onto the page. Discounts wouldn't be visible until users arrive at checkout, as we currently do with Buy Buttons.

gmalette commented 8 years ago

Discounts wouldn't be visible until users arrive at checkout, as we currently do with Buy Buttons.

I don't think we should take this approach from a product perspective. Discounts and promotions play a central role in how merchants acquire customers, and if the prices don't reflect them where people check their cart, merchants see a big drop in conversion.

Buy Buttons currently don't have discounts because it's currently not possible to publish scripts other than on the online store, but we'll be investigating how to increase the channels we cover once we're done fixing the current issues with scripts. Automated discounts is also coming soon, so it's not just for Plus.

jamesmacaulay commented 8 years ago

I don't think we should take this approach from a product perspective. Discounts and promotions play a central role in how merchants acquire customers, and if the prices don't reflect them where people check their cart, merchants see a big drop in conversion.

Fair enough; it's just a question of how soon a carts API with these features can be launched. There were seemingly too many concerns with using the checkout API "as a cart API" for the project to move forward with that approach.

Right now the plan is to implement local carts in the first public release, then have a Shopify-backed cart in a subsequent release. Ideally the SDK interface can be built such that client code can be written against the local cart implementation and then upgrade to SDKv2 and suddenly the same code is working against a Shopify-backed cart (plus or minus a config setting). The local cart implementation would presumably be used as a cache at that point. To make that work, the interface for the local cart would need to be async and accommodate a future where augmented/modified cart data is being sent back in the response.

gmalette commented 8 years ago

Right now the plan is to implement local carts in the first public release

Sounds good. I guess we'll want to have everything async to start with then, and have an API that reflects those "unpredictable" changes.