Suggestion: use Redux for cart state in react-apollo example

[SachaG]

Since Apollo already uses Redux to store products, I think it'd be a natural fit to also use it to manage the shopping cart's state.

Or would it be better to create a new react-apollo-redux example?

[SachaG]

Actually to be more precise, the checkout state is already stored in the Redux store, but then it's also stored as state on the App component. It seems like the App component's state could be eliminated if the data was fetched in the store directly?

[swalkinshaw]

Fetching from the store directly sounds like an improvement 👍. A PR would be welcome if you want to do it :)

[SachaG]

Looking at the code, it seems like the checkout data is obtained through a createCheckout mutation. So actually, maybe the better idea to simplify the architecture would be to wrap the Cart component with a withCreateCheckout HoC directly instead of doing it in App?

In other words, make App slimmer and redistribute its logic throughout each component at the point where it's needed?

[swalkinshaw]

I'm not too familiar with React design patters and HoC's but

make App slimmer and redistribute its logic throughout each component at the point where it's needed

does sound like a good goal. I think right now these examples automatically create a checkout on first visit. This was a decision to simplify things but it might be possible to delay that as well until it's needed.

[SachaG]

Yeah I was wondering what the idea behind that was. Is it so that checkouts can be saved server-side and reloaded later? Do "normal" Shopify sites always initialize checkout objects right from the start?

I also saw there were some hard-coded values in there (e.g. shippingAddress: {city: 'Toronto', province: 'ON', country: 'Canada'}), I wasn't quite sure how to deal with them.

[swalkinshaw]

Yeah I was wondering what the idea behind that was. Is it so that checkouts can be saved server-side and reloaded later? Do "normal" Shopify sites always initialize checkout objects right from the start?

Yeah so when a return visit happens, the checkout can be retrieved from local storage or whatever. But it still doesn't need to happen automatically on first visit. I personally think it should be deferred from "on load" to "on add item".

This does kind of mirror what Shopify's Online Store does, but it's a different concept and I can't think of a good reason to just copy it.

I also saw there were some hard-coded values in there (e.g. shippingAddress: {city: 'Toronto', province: 'ON', country: 'Canada'}), I wasn't quite sure how to deal with them.

I think that was required before our API was launched. It should definitely be removed. An "empty" checkout can be created.

[SachaG]

I think I'm not going to touch the checkout code for now then, I'll let you change it to what you have in mind before I refactor anything. I can always find other things to do :)

[SachaG]

One more note, I think createCheckout should really be a query, not a mutation. Apollo has a lot of APIs that work a lot better if data comes from a query than a mutation. In fact, mutations are usually used to update existing queries, not create data that didn't exist at all before.

[swalkinshaw]

That's weird to me 🤔

In fact, mutations are usually used to update existing queries, not create data that didn't exist at all before.

Any references about this?

Most of what I've ever read about mutations is using them for operations that cause a write on the backend which is obviously the case with createCheckout. Most public schemas I've seen seem to follow this as well.

[SachaG]

Yeah sorry, I didn't really explained what I meant very well. Basically this is how I see it. We have two possible situations:

1. The user has an existing checkout that needs to be retrieved (based on an id stored in a cookie or localStorage I assume? I don't think that part is implemented in the example yet, right?)
2. The user has no checkout, one needs to be created.

Use case (1) is a query, while (2) is a mutation. But in Apollo, query containers pass their result as a child prop, while mutations pass their results as the results of a promise.

So I'm not entirely sure what's the best way to handle both use cases in the same place (like a single withCheckout container). I'm curious to know how you'd do it (basically, how you would adapt the current example to not use the App state anymore), or else I might also ask the Apollo guys directly for some pointers.

[stubailo]

I think this is a bit interesting because most GraphQL clients right now assume that the result from a mutation isn't that interesting. Apollo Client, Relay, etc allow you to update some existing data in the cache via the mutation result, but don't treat that result the same way as the result from a query.

There are a few ways I might go about this:

1. Use the mutation to load a checkout ID, and then use a query everywhere you actually use the information about that checkout. Is there a query that retrieves data via a checkout ID?
2. Use the mutation to load all checkout data, write it to the store, then use readFragment to get the data out via a checkout ID. Since the data is actually stored, it's just not attached to the root query type anywhere. So you can get at it with readFragment.
3. Same as (2) but you use a custom resolver to map the same query as (1) to the result. This might be the most "correct" one.

There's one thing you can't get around: The checkout object is a per-session value. So it can't really be a query since there can be multiple checkouts per user (I assume). So in any case you need to store that ID somewhere in local storage, or redux, or local state, or whatever. The only question is how do you get the rest of the data.

I'm partial to 3 because it means you can use a query to get the data, while still using the mutation result at first to avoid two requests.

[stubailo]

OK @SachaG showed me the query:

  query checkoutQuery($id: ID!) {
    node(id:$id) {
      id
      ... on Checkout{
        ...CheckoutFragment
      }
    }
  }

So you could define a custom resolver for node that checks if a Node with that ID is already present in the store. Then, you define a container that checks if it has a checkoutId, if not it does a mutation to create one; and use the query with the custom resolver to get the data from the store.

[swalkinshaw]

That sounds good to me 👍

Checkouts are handled in a generic way in our schema (like they are any other resource) so any client specific behaviour should be done on the client.

[SachaG]

Oh, speaking of this: what was the reasoning behind having a single node query that returns a union of every type? I could see how it makes some things like searching across types easier, but the downside is that you lose GraphQL's auto-discoverability. Are you guys planning to also implement checkout, products, etc. query endpoints by any chance?

[swalkinshaw]

That's just part of the Relay spec which is pretty standard now. We did have those query fields at one point but just felt it was duplicative. No plans to add them back as of yet.

[nickdandakis]

@SachaG, I think a withCheckout HOC that exposes everything in react-apollo/src/checkout.js to the composed component props, and handles localStorage persistence is the way to go.

[SachaG]

Yes, I agree. I actually started working on this but had to pause the project. Hoping to restart soon following @stubailo's advice.

[SachaG]

OK, I made some progress: https://github.com/ReactifyJS/Reactify

I haven't yet done the custom resolver thing, so right now I'm in scenario 1 in @stubailo's post.

[lloydh]

I'd be interested to know if there's been any more progress. Obviously Apollo has moved away from Redux but @stubailo's 3rd scenario still seems like a good goal to avoid duplicating state.

In particular I'd be interested in options to query Apollo for a Checkout (or null so that one can be created) on the client without a checkout id?