Skip to content
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

Add better evicted cart support #341

Merged
merged 5 commits into from
Jan 6, 2023

Conversation

jplhomer
Copy link
Contributor

@jplhomer jplhomer commented Dec 16, 2022

In #325, we attempted to solve an issue that exists today in the SFAPI Cart implementation: carts will be "evicted" from storage after X days of inactivity.

However, as discussed here:https://github.com/Shopify/core-issues/issues/48930 that would be solving the problem at the wrong layer. Clients like Hydrogen should not be concerned about this implementation detail.

Instead, the Cart API will be updated to "soft-404" for mutations against an evicted cart, and return a new Cart ID instead.

This PR:

  • Ensures 404 carts don't break the app at the root loader
  • Ensures cartId is updated in the session after each mutation
  • Updates the cart RFC

@jplhomer jplhomer changed the title jl better evicted cart support Add better evicted cart support Dec 16, 2022
Copy link
Contributor

@juanpprieto juanpprieto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

Copy link
Contributor

@frandiox frandiox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎄

rfc/cart.md Outdated Show resolved Hide resolved
Comment on lines +151 to +155
/**
* The Cart ID may change after each mutation. We need to update it each time in the session.
*/
session.set('cartId', cartId);
headers.set('Set-Cookie', await session.commit());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wrap this in if (storedCartId !== cartId)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably could, but committing it every time is simpler and one less thing to reason about.

Co-authored-by: Fran Dios <frankdiox@gmail.com>
@jplhomer jplhomer merged commit 83096b5 into v0.x-2022-10 Jan 6, 2023
@jplhomer jplhomer deleted the jl-better-evicted-cart-support branch January 6, 2023 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants