Skip to content

feat: Remove caching on fetching product inventory data #2801

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.

Sign up for GitHub

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

Open
Tharaae wants to merge 1 commit into
base: canary
Choose a base branch
from
Open

feat: Remove caching on fetching product inventory data #2801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/eager-nails-bake.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"@bigcommerce/catalyst-core": minor
---

Fetch product inventory data with a separate GQL query with no caching
Comment on lines +2 to +5
Copy link
Collaborator

@migueloller migueloller Dec 31, 2025

Choose a reason for hiding this comment

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

Is the rationale for marking this as a minor the impact it could have on performance and/or costs for the PDP?

This is the primary question that I have when introducing queries that aren't cached. Are we good with every single PDP load invoking the API? Are we certain merchants won't run into rate limits?


## Migration
The files to be rebased for this change to be applied are:
- core/app/[locale]/(default)/product/[slug]/page-data.ts
- core/app/[locale]/(default)/product/[slug]/page.tsx
46 changes: 37 additions & 9 deletions core/app/[locale]/(default)/product/[slug]/page-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ export const getStreamableProductVariant = cache(
document: StreamableProductVariantBySkuQuery,
variables,
customerAccessToken,
fetchOptions: customerAccessToken ? { cache: 'no-store' } : { next: { revalidate } },
fetchOptions: { cache: 'no-store' },
Copy link
Collaborator

@migueloller migueloller Dec 31, 2025

Choose a reason for hiding this comment

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

This seems to be removing caching from the existing query for product variants. Was this intentional? If the query is specifically for variant inventory quantities, can we update the query name to be explicit about that.

});

return data.site.product?.variants;
Expand Down Expand Up @@ -306,6 +306,36 @@ const StreamableProductQuery = graphql(
minPurchaseQuantity
maxPurchaseQuantity
warranty
...ProductViewedFragment
...ProductSchemaFragment
}
}
}
`,
[ProductViewedFragment, ProductSchemaFragment],
);

type Variables = VariablesOf<typeof StreamableProductQuery>;

export const getStreamableProduct = cache(
async (variables: Variables, customerAccessToken?: string) => {
const { data } = await client.fetch({
document: StreamableProductQuery,
variables,
customerAccessToken,
fetchOptions: customerAccessToken ? { cache: 'no-store' } : { next: { revalidate } },
});

return data.site.product;
},
);

const StreamableProductInventoryQuery = graphql(
`
query StreamableProductInventoryQuery($entityId: Int!) {
site {
product(entityId: $entityId) {
sku
inventory {
hasVariantInventory
isInStock
Expand All @@ -320,25 +350,23 @@ const StreamableProductQuery = graphql(
availabilityV2 {
status
}
...ProductViewedFragment
...ProductVariantsInventoryFragment
...ProductSchemaFragment
}
}
}
`,
[ProductViewedFragment, ProductSchemaFragment, ProductVariantsInventoryFragment],
[ProductVariantsInventoryFragment],
);

type Variables = VariablesOf<typeof StreamableProductQuery>;
type ProductInventoryVariables = VariablesOf<typeof StreamableProductQuery>;

export const getStreamableProduct = cache(
async (variables: Variables, customerAccessToken?: string) => {
export const getStreamableProductInventory = cache(
async (variables: ProductInventoryVariables, customerAccessToken?: string) => {
const { data } = await client.fetch({
document: StreamableProductQuery,
document: StreamableProductInventoryQuery,
variables,
customerAccessToken,
fetchOptions: customerAccessToken ? { cache: 'no-store' } : { next: { revalidate } },
fetchOptions: { cache: 'no-store' },
Copy link
Collaborator

@migueloller migueloller Dec 31, 2025

Choose a reason for hiding this comment

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

Is no-store the appropriate value here or would a very small revalidation period be desirable? There's is always a chance for there to be stale inventory data, even if it's always fetched without caching since there could be race conditions when loading the page. This could be addressed with polling, but I question whether that's a good approach. Instead, a warning when stock is very low might be good enough to let the user know that the item product might be out of stock soon.

});

return data.site.product;
Expand Down
25 changes: 20 additions & 5 deletions core/app/[locale]/(default)/product/[slug]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
getProductPricingAndRelatedProducts,
getStreamableInventorySettingsQuery,
getStreamableProduct,
getStreamableProductInventory,
getStreamableProductVariant,
} from './page-data';

Expand Down Expand Up @@ -117,8 +118,22 @@ export default async function Product({ params, searchParams }: Props) {

const streamableProductSku = Streamable.from(async () => (await streamableProduct).sku);

const streamableProductInventory = Streamable.from(async () => {
const variables = {
entityId: Number(productId),
};

const product = await getStreamableProductInventory(variables, customerAccessToken);

if (!product) {
return notFound();
}

return product;
});

const streamableProductVariant = Streamable.from(async () => {
const product = await streamableProduct;
const product = await streamableProductInventory;
Comment on lines 135 to +136
Copy link
Collaborator

@migueloller migueloller Dec 31, 2025

Choose a reason for hiding this comment

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

Shouldn't the variable name change if this is specifically tracking product inventory?


if (!product.inventory.hasVariantInventory) {
return undefined;
Expand Down Expand Up @@ -188,7 +203,7 @@ export default async function Product({ params, searchParams }: Props) {
});

const streameableCtaLabel = Streamable.from(async () => {
const product = await streamableProduct;
const product = await streamableProductInventory;

if (product.availabilityV2.status === 'Unavailable') {
return t('ProductDetails.Submit.unavailable');
Expand All @@ -206,7 +221,7 @@ export default async function Product({ params, searchParams }: Props) {
});

const streameableCtaDisabled = Streamable.from(async () => {
const product = await streamableProduct;
const product = await streamableProductInventory;

if (product.availabilityV2.status === 'Unavailable') {
return true;
Expand Down Expand Up @@ -253,7 +268,7 @@ export default async function Product({ params, searchParams }: Props) {

const streamableStockDisplayData = Streamable.from(async () => {
const [product, variant, inventorySetting] = await Streamable.all([
streamableProduct,
streamableProductInventory,
streamableProductVariant,
streamableInventorySettings,
]);
Expand Down Expand Up @@ -343,7 +358,7 @@ export default async function Product({ params, searchParams }: Props) {

const streamableBackorderDisplayData = Streamable.from(async () => {
const [product, variant, inventorySetting] = await Streamable.all([
streamableProduct,
streamableProductInventory,
streamableProductVariant,
streamableInventorySettings,
]);
Expand Down