Feat [Shopify]: Internationalization Support for Shopify Loaders and Queries #1371
Conversation
WalkthroughThreads optional Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as App
participant Loader as Page Loader
participant SF as Storefront API
User->>UI: Navigate to page
UI->>Loader: call loader({ languageCode?, countryCode? })
note right of Loader #E8F8F5: defaults language=PT, country=BR
Loader->>SF: GraphQL query @inContext(languageCode, countryCode)
SF-->>Loader: Localized response (includes buyerIdentity when cart)
Loader-->>UI: Return localized data
UI-->>User: Render localized UI
sequenceDiagram
autonumber
participant CartLoader as Cart Loader
participant SF as Storefront API
CartLoader->>SF: CreateCart(countryCode?) or GetCart(id, languageCode?, countryCode?)
SF-->>CartLoader: cart { id, buyerIdentity, lines... }
CartLoader->>SF: CartBuyerIdentityUpdate(cartId, buyerIdentity)
SF-->>CartLoader: updated cart + userErrors
CartLoader-->>Client: Cart with buyerIdentity context
sequenceDiagram
autonumber
participant ProxyLoader as Proxy Loader
participant Builder as buildProxyRoutes
participant Routes as Route Output
ProxyLoader->>Builder: { extraPathsToProxy?, extraPaths?, excludePathsFromShopifySiteMap? }
Builder-->>ProxyLoader: route definitions (use finalExtraPathsToProxy)
note right of Routes #F7F9F9: `/sitemap.xml` route includes `exclude` and sets __resolveType based on pathTemplate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Points needing extra attention:
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧬 Code graph analysis (7)shopify/utils/types.ts (1)
shopify/handlers/sitemap.ts (4)
shopify/loaders/cart.ts (3)
shopify/loaders/ProductList.ts (2)
shopify/loaders/proxy.ts (4)
shopify/loaders/ProductListingPage.ts (3)
shopify/utils/storefront/queries.ts (1)
🪛 ast-grep (0.39.6)shopify/handlers/sitemap.ts[warning] 17-20: Manual HTML sanitization detected using string replacement methods. Manual sanitization is error-prone and can be bypassed. Use dedicated HTML sanitization libraries like 'sanitize-html' or 'DOMPurify' instead. (manual-html-sanitization) [warning] 17-19: Manual HTML sanitization detected using string replacement methods. Manual sanitization is error-prone and can be bypassed. Use dedicated HTML sanitization libraries like 'sanitize-html' or 'DOMPurify' instead. (manual-html-sanitization) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Tagging OptionsShould a new tag be published when this PR is merged?
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
shopify/utils/transform.ts (1)
353-370: Bug: URLSearchParams API misused; toggle logic never works
URLSearchParams.hasand.deletedon’t accept a value argument. As written, removing a single selected filter never happens.Apply:
const label = getFilterValue(value); - - if (params.has(filter.id, label)) { - params.delete(filter.id, label); - } else { - params.append(filter.id, label); - } + const existing = params.getAll(filter.id); + const idx = existing.indexOf(label); + if (idx >= 0) { + existing.splice(idx, 1); + params.delete(filter.id); + for (const v of existing) params.append(filter.id, v); + } else { + params.append(filter.id, label); + }shopify/loaders/ProductListingPage.ts (1)
249-251: Canonical uses zero-basedpage.Use
currentPage(orpage + currentPageoffset) to avoid emitting ?page=1 for the second page request of ?page=2.Apply:
- canonical: `${url.origin}${url.pathname}${ - page >= 1 ? `?page=${page}` : "" - }`, + canonical: `${url.origin}${url.pathname}${ + currentPage > 1 ? `?page=${currentPage}` : "" + }`,shopify/loaders/RelatedProducts.ts (1)
99-109: Include locale in cacheKey to prevent cross-locale cache mixupsEven with
no-cachetoday, adding these guards against future caching or upstream overrides.-export const cacheKey = (props: Props, req: Request): string => { - const { slug, count } = props; +export const cacheKey = (props: Props, req: Request): string => { + const { slug, count, languageCode, countryCode } = props; const searchParams = new URLSearchParams({ slug, count: count.toString(), + languageCode: (languageCode ?? "PT"), + countryCode: (countryCode ?? "BR"), });
🧹 Nitpick comments (22)
shopify/utils/transform.ts (2)
165-173: Use type guards instead of casts when reading metafield reference imagesThe current casts can hide shape issues at runtime. Narrow by checking for an
imageproperty on the union node to avoid undefined access.- const referenceImageUrl = - (reference as { image?: { url?: string } } | undefined)?.image?.url ?? null; + let referenceImageUrl: string | null = null; + const refAny = reference as unknown; + if (refAny && typeof refAny === "object" && "image" in refAny) { + referenceImageUrl = + (refAny as { image?: { url?: string } }).image?.url ?? null; + } @@ - const edgeImages = Array.isArray(references?.edges) - ? references.edges.map( - (edge) => - (edge?.node as { image?: { url?: string } } | undefined)?.image - ?.url ?? null - ) - : null; + const edgeImages = Array.isArray(references?.edges) + ? references.edges.map((edge) => { + const node = edge?.node as unknown; + if (node && typeof node === "object" && "image" in node) { + return (node as { image?: { url?: string } }).image?.url ?? null; + } + return null; + }) + : null;
176-183: Joining multiple image URLs into a comma string can be brittleDownstream code may expect a single string; comma-joining also makes parsing ambiguous. Consider serializing arrays as JSON strings for multi-image values, or add a separate field for images while preserving the original
value.If you choose JSON, a minimal change is:
- const valueToReturn = + const valueToReturn = referenceImageUrl ?? (validEdgeImages && validEdgeImages.length > 0 - ? validEdgeImages.join(",") + ? JSON.stringify(validEdgeImages) : value);Please confirm no consumer relies on comma-splitting today. I can scan usages of
additionalProperty/PropertyValue.valueto de-risk before changing.shopify/loaders/proxy.ts (3)
63-64: Avoid substring match for route type selection
includes("sitemap")may misclassify unrelated paths (e.g., custom routes containing “sitemap”). Match only Shopify sitemap endpoints.- __resolveType: pathTemplate.includes("sitemap") ? "shopify/handlers/sitemap.ts" : "website/handlers/proxy.ts", + __resolveType: /^\/sitemap(\.xml.*|\/.*)$/i.test(pathTemplate) + ? "shopify/handlers/sitemap.ts" + : "website/handlers/proxy.ts",
127-134: Doc titles don’t match actual routes
- Deco sitemap path is
/sitemap/deco.xml(not/deco-sitemap.xml).- Shopify sitemap lives at
/sitemap.xml(title says/shopify-sitemap.xml)./** - * @title If deco site map should be exposed at /deco-sitemap.xml + * @title If deco site map should be exposed at /sitemap/deco.xml */ @@ /** - * @title Exclude paths from /shopify-sitemap.xml + * @title Exclude paths from /sitemap.xml */
6-25: Duplicate entry in PATHS_TO_PROXY
"/services/*"appears twice."/services/*", - "/.well-known/*", - "/services/*", + "/.well-known/*",shopify/utils/types.ts (1)
193-196: Make LanguageContextArgs optional to match GraphQL InputMaybe and improve reuseYour queries pass defaults, but keeping these optional aligns with codegen types and lets other operations omit them without widening intersections.
-export interface LanguageContextArgs { - languageCode: LanguageCode; - countryCode: CountryCode; -} +export interface LanguageContextArgs { + languageCode?: LanguageCode; + countryCode?: CountryCode; +}Side note: CartBuyerIdentity.countryCode is a string in this file; consider CountryCode for consistency.
shopify/loaders/ProductList.ts (3)
16-18: Imports look good; consider aliasing Metafield to avoid name collisionMetafield here represents identifiers (namespace/key) while GraphQL also defines a Metafield type. Aliasing can reduce confusion.
-import { LanguageContextArgs, Metafield } from "../utils/types.ts"; +import { LanguageContextArgs, Metafield as MetafieldIdentifier } from "../utils/types.ts";Then update type uses (metafields?: MetafieldIdentifier[]; and identifiers: metafields).
Also applies to: 26-26
75-87: Include locale in cacheKey to prevent cross-locale collisionsEven with cache = "no-cache" today, future changes or upstream caching could serve the wrong locale. Add languageCode/countryCode to cacheKey.
TypeScript snippet (outside the changed range) showing the minimal addition:
export const cacheKey = (expandedProps: Props, req: Request): string => { // ...existing code... const languageCode = expandedProps.languageCode ?? "PT"; const countryCode = expandedProps.countryCode ?? "BR"; searchParams.append("languageCode", languageCode); searchParams.append("countryCode", countryCode); // ...existing code... }
109-111: Narrow types on defaults for stronger inferenceExplicitly annotate to keep these as LanguageCode/CountryCode (not widened unions).
-const languageCode = expandedProps?.languageCode ?? "PT"; -const countryCode = expandedProps?.countryCode ?? "BR"; +const languageCode: LanguageCode = expandedProps.languageCode ?? "PT"; +const countryCode: CountryCode = expandedProps.countryCode ?? "BR";shopify/loaders/ProductDetailsPage.ts (2)
22-34: Add locale to cacheKey to avoid mixing pages across localesSlug-only cache keys risk serving the wrong localized PDP later if caching is enabled.
TypeScript snippet (outside the changed range):
export const cacheKey = (props: Props, req: Request): string => { const { slug } = props; const languageCode = props.languageCode ?? "PT"; const countryCode = props.countryCode ?? "BR"; const searchParams = new URLSearchParams({ slug, languageCode, countryCode }); const url = new URL(req.url); url.search = searchParams.toString(); return url.href; };
46-47: Preserve literal types on defaultsMinor: explicitly annotate to keep these as the enum literal types.
-const { slug, languageCode = "PT", countryCode = "BR" } = props; +const { slug } = props; +const languageCode: LanguageCode = props.languageCode ?? "PT"; +const countryCode: CountryCode = props.countryCode ?? "BR";shopify/loaders/shop.ts (2)
17-28: Document defaults on new locale props.Add JSDoc @default tags so CMS/UI surfaces the implicit PT/BR defaults.
Apply:
/** * @title Language Code * @description Language code for the storefront API * @example "EN" for English, "FR" for French, etc. + * @default "PT" */ languageCode?: LanguageCode; /** * @title Country Code * @description Country code for the storefront API * @example "US" for United States, "FR" for France, etc. + * @default "BR" */ countryCode?: CountryCode;
39-39: Default locale derivation.Optional: derive defaults from ctx (storefront/shop defaults or request Accept-Language) to avoid surprising PT/BR fallback in non-BR stores.
shopify/loaders/ProductListingPage.ts (4)
74-86: Include locale in cache key or confirm no-cache.If caching is enabled later, include languageCode/countryCode in cacheKey; otherwise different locales can collide. Current export sets cache="no-cache", but please confirm this won’t change.
Proposed cacheKey patch:
- const searchParams = new URLSearchParams({ + const searchParams = new URLSearchParams({ count, query, page, endCursor, startCursor, sort, + languageCode: (props.languageCode ?? "PT"), + countryCode: (props.countryCode ?? "BR"), });
111-113: Locale defaults OK; consider specificity.Minor: with countryCode "BR", using languageCode "PT_BR" may better match most Brazilian stores.
158-161: Handle edge cases in last-segment collection handle extraction.Guard against trailing “collections” or “products” segments to avoid querying invalid handles.
Apply:
-const pathname = props.collectionName || url.pathname.split("/").filter(Boolean).pop(); +const segments = (props.collectionName + ? [props.collectionName] + : url.pathname.split("/").filter(Boolean)); +let pathname = segments.at(-1); +if (pathname === "collections" || pathname === "products") { + pathname = segments.at(-2); +}
227-227: Breadcrumb @id should be a page URL, not a Shopify GID.Use a stable canonical URL so search engines can reconcile entities.
Apply:
- "@id": collectionId, + "@id": isSearch ? url.href : `${url.origin}${url.pathname}`,shopify/loaders/cart.ts (2)
30-35: Locale defaults OK; consider specificity.Optional: prefer PT_BR when country BR to better align language/country.
34-45: Optional: normalize existing carts to new country.If a cookie cart exists with a different buyerIdentity.countryCode than props.countryCode, consider issuing cartBuyerIdentityUpdate to keep pricing consistent with the chosen locale.
I can draft the mutation wiring if you want it added in this PR.
shopify/loaders/RelatedProducts.ts (2)
56-57: Consider sourcing defaults from config, not hardcoding "PT"/"BR"If multiple shops/locales coexist, pulling defaults from a central config (or ctx) avoids drift.
63-69: Avoid intersecting generated variables with local types
GetProductQueryVariablesandProductRecommendationsQueryVariablesalready includeidentifiers,languageCode, andcountryCode. The intersections withHasMetafieldsMetafieldsArgsandLanguageContextArgsunnecessarily tighten the contract (e.g., making optionals required) and can drift from the generated schema.Apply:
- const query = await storefront.query< - GetProductQuery, - GetProductQueryVariables & HasMetafieldsMetafieldsArgs & LanguageContextArgs - >({ + const query = await storefront.query< + GetProductQuery, + GetProductQueryVariables + >({ variables: { handle, identifiers: metafields, languageCode, countryCode }, ...GetProduct, }); - const data = await storefront.query< - ProductRecommendationsQuery, - ProductRecommendationsQueryVariables & HasMetafieldsMetafieldsArgs & LanguageContextArgs - >({ + const data = await storefront.query< + ProductRecommendationsQuery, + ProductRecommendationsQueryVariables + >({ variables: { productId: query.product.id, identifiers: metafields, languageCode, countryCode }, ...ProductRecommendations, });Also applies to: 75-87
shopify/utils/storefront/queries.ts (1)
273-282: CreateCart: optionalReturning only
idis fine; consider addingbuyerIdentity.countryCodeif you plan to display locale-specific cart state immediately after creation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
shopify/loaders/ProductDetailsPage.ts(4 hunks)shopify/loaders/ProductList.ts(6 hunks)shopify/loaders/ProductListingPage.ts(10 hunks)shopify/loaders/RelatedProducts.ts(5 hunks)shopify/loaders/cart.ts(1 hunks)shopify/loaders/proxy.ts(6 hunks)shopify/loaders/shop.ts(2 hunks)shopify/utils/storefront/queries.ts(2 hunks)shopify/utils/storefront/storefront.graphql.gen.ts(6 hunks)shopify/utils/transform.ts(1 hunks)shopify/utils/types.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
shopify/utils/types.ts (1)
shopify/utils/storefront/storefront.graphql.gen.ts (2)
LanguageCode(3911-4199)CountryCode(2138-2628)
shopify/loaders/shop.ts (2)
shopify/utils/types.ts (2)
Metafield(188-191)LanguageContextArgs(193-196)shopify/utils/storefront/storefront.graphql.gen.ts (3)
LanguageCode(3911-4199)CountryCode(2138-2628)ShopMetafieldsArgs(7233-7235)
shopify/loaders/RelatedProducts.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (5)
LanguageCode(3911-4199)CountryCode(2138-2628)GetProductQueryVariables(7745-7750)HasMetafieldsMetafieldsArgs(3752-3754)ProductRecommendationsQueryVariables(7801-7806)shopify/utils/types.ts (1)
LanguageContextArgs(193-196)
shopify/loaders/ProductDetailsPage.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (4)
LanguageCode(3911-4199)CountryCode(2138-2628)GetProductQueryVariables(7745-7750)HasMetafieldsMetafieldsArgs(3752-3754)shopify/utils/types.ts (1)
LanguageContextArgs(193-196)
shopify/loaders/ProductListingPage.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (2)
LanguageCode(3911-4199)CountryCode(2138-2628)shopify/utils/types.ts (1)
LanguageContextArgs(193-196)
shopify/loaders/cart.ts (3)
shopify/utils/storefront/storefront.graphql.gen.ts (6)
LanguageCode(3911-4199)CountryCode(2138-2628)GetCartQuery(7743-7743)CreateCartMutation(7734-7734)CreateCartMutationVariables(7729-7731)GetCartQueryVariables(7736-7740)shopify/utils/storefront/queries.ts (1)
CreateCart(272-282)shopify/utils/types.ts (1)
LanguageContextArgs(193-196)
shopify/loaders/proxy.ts (2)
website/handlers/proxy.ts (1)
Props(44-89)shopify/handlers/sitemap.ts (1)
Props(31-33)
shopify/loaders/ProductList.ts (2)
shopify/utils/storefront/storefront.graphql.gen.ts (5)
LanguageCode(3911-4199)CountryCode(2138-2628)QueryRoot(6476-6579)QueryRootSearchArgs(6798-6810)HasMetafieldsMetafieldsArgs(3752-3754)shopify/utils/types.ts (1)
LanguageContextArgs(193-196)
shopify/utils/storefront/storefront.graphql.gen.ts (1)
wake/utils/graphql/storefront.graphql.gen.ts (7)
CreateCartMutationVariables(5028-5028)Exact(12-12)InputMaybe(11-11)CreateCartMutation(5031-5031)GetCartQueryVariables(5021-5023)Scalars(18-33)GetCartQuery(5026-5026)
shopify/utils/storefront/queries.ts (1)
shopify/utils/storefront/storefront.graphql.gen.ts (6)
Cart(527-578)Product(5933-6018)ProductVariant(6333-6386)Collection(1881-1912)Filter(3596-3605)Customer(3022-3061)
🔇 Additional comments (22)
shopify/utils/storefront/storefront.graphql.gen.ts (3)
7725-7744: Verify that all GraphQL callers handle the new optional i18n variables
I didn’t find any direct references to the generated query hooks or documents (e.g.useGetCartQuery,GetCartDocument), so please manually confirm that every GraphQL call now provides the optionalcountryCode/languageCodearguments or correctly falls back to defaults.
7729-7731: Confirm CreateCart country scoping is actually used
CreateCartMutationVariablesgainedcountryCode, but many implementations instead setbuyerIdentity.countryCodein theinput. Ensure the mutation or its wrapper applies the variable (via@inContextorinput.buyerIdentity), otherwise it’s unused.#!/bin/bash rg -nP -C3 'CreateCartMutation\W' && rg -nP -C3 'cartCreate\W' shopify | sed -n '1,200p'
7862-7869: New CartBuyerIdentityUpdate mutation: wire through loaders and add basic error handlingEnsure a public function wraps this mutation and maps
userErrorsinto your app’s error model. Also add a timeout/retry policy in the caller.#!/bin/bash rg -nP -C2 'CartBuyerIdentityUpdateMutation' && rg -nP -C2 'buyerIdentityUpdate' shopifyshopify/utils/types.ts (1)
7-7: LGTM: source-of-truth for locale enumsImporting LanguageCode and CountryCode from the generated GraphQL types removes drift risk vs hand-rolled enums. No concerns.
shopify/loaders/ProductList.ts (1)
141-142: Locale threading verified All storefront queries declare and use$languageCodeand$countryCodewith@inContextacross search, collection, and recommendation paths.shopify/loaders/ProductDetailsPage.ts (2)
9-11: Imports align with codegen and shared typesBringing LanguageCode/CountryCode from codegen and LanguageContextArgs from utils is consistent with the rest of the PR.
Also applies to: 13-13
56-56: Approve locale variables usageVerified that the GetProduct query in shopify/utils/storefront/queries.ts declares both $languageCode and $countryCode and applies them in the @incontext(language: $languageCode, country: $countryCode) directive.
shopify/loaders/shop.ts (2)
4-9: Imports look good.Types are sourced from the generated schema and shared types. No issues.
41-47: Approve code changesGetShopInfo query declares $languageCode and $countryCode and applies @incontext; the type intersection with ShopMetafieldsArgs & LanguageContextArgs is correct and future-proof.
shopify/loaders/ProductListingPage.ts (4)
133-145: Search query: locale variables threaded correctly.Intersection with LanguageContextArgs looks good; variable bag is consistent.
167-168: Collection query: locale variables—LGTM.Types and variable wiring are correct.
Also applies to: 177-179
192-192: Storing collectionId—LGTM.Used later for breadcrumb; see note below on @id value.
230-230: Breadcrumb name derivation—LGTM.Using the last path segment for non-search pages is reasonable.
shopify/loaders/cart.ts (3)
14-27: Props + docs—LGTM.Public API looks clear and consistent with other loaders.
47-53: GetCart with locale—LGTM.Variable typing via intersection ensures both codes are required at callsite.
55-55: Verify cookie encoding consistency.We decode before querying and then write raw cartId back. Ensure getCartCookie/setCartCookie agree on encoding to avoid double-encoding or invalid IDs.
Run:
#!/bin/bash rg -nC2 'getCartCookie|setCartCookie' shopify | sed -n '1,200p'shopify/loaders/RelatedProducts.ts (2)
32-44: Props for locale look goodOptional props + docs read well.
18-19: ConfirmingMetafieldaligns with GraphQLHasMetafieldsIdentifier; no changes needed. TheMetafieldinterface (namespace: string; key: string) exactly matches the identifier shape expected by the query.shopify/utils/storefront/queries.ts (4)
286-297: Consistent @incontext adoption across queriesNice job threading
languageCode/countryCodeand moving to variable-based@inContext. This will play well with tooling and typed vars.Also applies to: 301-313, 316-333, 335-379, 380-426, 428-442, 444-502
172-181: Cart fragment enrichment looks goodIncluding
buyerIdentity.countryCode/
506-513: Customer query doesn’t need @incontextFields returned aren’t localized; keeping it simple here makes sense.
528-556: Good UX: surface user errorsExplicitly returning
customerUserErrorson registration and sign-in is 👍.Also applies to: 608-623
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
shopify/handlers/sitemap.ts (3)
64-68: Use a resilient insertion anchorRelying on an exact opening-tag string is brittle. Insert before the closing instead.
- const finalXml = includeBlock - ? excludedXml.replace(XML_HEADER, `${XML_HEADER}\n${includeBlock}`) - : excludedXml; + const finalXml = includeBlock + ? excludedXml.replace("</sitemapindex>", `${includeBlock}\n</sitemapindex>`) + : excludedXml;
59-63: Simplify origin handlingURL.origin never ends with “/”; the extra normalization and the variable name are misleading.
- const originWithSlash = reqOrigin.endsWith("/") ? reqOrigin.slice(0, -1) : reqOrigin; - const originReplacedXml = originalXml.replaceAll(shopifyUrl, originWithSlash); + const originReplacedXml = originalXml.replaceAll(shopifyUrl, reqOrigin);
38-38: Document exclude semanticsAdd JSDoc to clarify expected values (path prefixes, leading “/”, examples).
export interface Props { include?: string[]; - exclude?: string[]; + /** @description Path prefixes to remove from the Shopify sitemap index (e.g., "/sitemap_blogs", "/sitemap_pages"). Leading "/" optional. */ + exclude?: string[]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shopify/handlers/sitemap.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
shopify/handlers/sitemap.ts (3)
shopify/mod.ts (2)
Props(13-41)AppContext(10-10)website/handlers/proxy.ts (2)
Props(44-89)Proxy(94-235)shopify/utils/password.ts (1)
withDigestCookie(3-9)
🔇 Additional comments (3)
shopify/handlers/sitemap.ts (3)
45-45: Signature change LGTMThreading exclude into the handler signature looks good and is backward compatible.
50-56: Proxy integration looks correctForwarding digest cookie via customHeaders and preserving upstream headers/status is aligned with existing proxy behavior.
57-57: Early return on non-OK is appropriateAvoids touching body/headers for error responses.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
shopify/handlers/sitemap.ts (1)
8-8: Remove unused TODAY constant.The module-level
TODAYconstant is declared but never used. Line 13 correctly computes a freshtodayvalue at runtime insidebuildIncludeSitemaps, which is the proper approach to avoid staleness on long-lived workers.Apply this diff to remove the unused constant:
const XML_HEADER = '<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">'; -const TODAY = new Date().toISOString().substring(0, 10);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
shopify/handlers/sitemap.ts(1 hunks)shopify/loaders/cart.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
shopify/loaders/cart.ts (3)
shopify/utils/storefront/storefront.graphql.gen.ts (6)
LanguageCode(3911-4199)CountryCode(2138-2628)GetCartQuery(7743-7743)CreateCartMutation(7734-7734)CreateCartMutationVariables(7729-7731)GetCartQueryVariables(7736-7740)shopify/utils/storefront/queries.ts (1)
CreateCart(272-282)shopify/utils/types.ts (1)
LanguageContextArgs(193-196)
shopify/handlers/sitemap.ts (3)
shopify/mod.ts (2)
Props(13-41)AppContext(10-10)website/handlers/proxy.ts (2)
Props(44-89)Proxy(94-235)shopify/utils/password.ts (1)
withDigestCookie(3-9)
🪛 ast-grep (0.39.5)
shopify/handlers/sitemap.ts
[warning] 14-17: Manual HTML sanitization detected using string replacement methods. Manual sanitization is error-prone and can be bypassed. Use dedicated HTML sanitization libraries like 'sanitize-html' or 'DOMPurify' instead.
Context: s
.replaceAll("&", "&")
.replaceAll("<", "<")
.replaceAll(">", ">")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html
- https://cwe.mitre.org/data/definitions/79.html
(manual-html-sanitization)
[warning] 14-16: Manual HTML sanitization detected using string replacement methods. Manual sanitization is error-prone and can be bypassed. Use dedicated HTML sanitization libraries like 'sanitize-html' or 'DOMPurify' instead.
Context: s
.replaceAll("&", "&")
.replaceAll("<", "<")
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html
- https://cwe.mitre.org/data/definitions/79.html
(manual-html-sanitization)
🪛 Biome (2.1.2)
shopify/loaders/cart.ts
[error] 39-39: Expected an expression but instead found 'const'.
Expected an expression here.
(parse)
[error] 39-39: Shouldn't redeclare 'cartId'. Consider to delete it or rename it.
'cartId' is defined here:
(lint/suspicious/noRedeclare)
🪛 GitHub Actions: ci
shopify/loaders/cart.ts
[error] 39-39: Deno fmt failed: Expression expected at cart.ts:39:3 during 'deno task check'.
🔇 Additional comments (8)
shopify/loaders/cart.ts (4)
14-27: LGTM! Well-documented Props interface.The Props interface is well-structured with clear JSDoc annotations and examples. The optional nature of both properties maintains backward compatibility.
30-30: LGTM! Loader signature correctly accepts Props.The signature change from
_props: unknowntoprops: Propsproperly enables typed access to the new locale parameters.
34-34: LGTM! Locale defaults are consistent with PR objectives.The default values (PT/BR) align with the stated internationalization strategy and the pattern used across other loaders in this PR.
49-55: LGTM! GetCart query properly includes locale context.The query correctly extends
GetCartQueryVariableswithLanguageContextArgsand passes all required variables including the new locale parameters. The typing is explicit and correct.shopify/handlers/sitemap.ts (4)
10-29: LGTM! XML escaping and fresh timestamp correctly implemented.The function properly addresses previous review concerns:
- Computes
todayfresh at runtime (line 13) to avoid staleness- Correctly escapes XML entities in proper order (& first, lines 14-20)
- Uses escaped URLs in
<loc>elements (line 25)Note: The static analysis warning about "manual HTML sanitization" is a false positive. This code performs standard XML entity escaping for sitemap
<loc>elements, which is the correct approach for this use case. The escaping sequence properly handles all required XML entities.
31-53: LGTM! Robust URL handling and prefix normalization.The function correctly implements all previously requested improvements:
- Normalizes exclude prefixes to ensure they start with "/" (line 35)
- Safely parses URLs with try-catch, using origin as base to support both absolute and relative URLs (lines 41-44)
- Falls back gracefully on parse errors, leaving entries untouched (lines 45-47)
- Uses normalized prefixes for consistent matching (line 50)
55-58: LGTM! Clean API extension.The
excludeproperty follows the same pattern asinclude, maintaining consistency and backward compatibility.
63-101: LGTM! Proper header handling and clean XML pipeline.The implementation correctly addresses the previous header passthrough concerns:
- Response headers are cloned (line 88)
- Body-specific headers are removed to prevent mismatches (lines 89-92):
content-length,content-encoding,etag,accept-ranges- Content-type is set appropriately for XML (lines 93-95)
The XML processing pipeline is well-structured:
- Proxy to Shopify
- Replace origin URLs (line 80)
- Apply exclusions (line 81)
- Inject inclusions (lines 84-86)
The origin normalization (line 79) correctly handles trailing slashes.
1 participant
What is this Contribution About?
This PR introduces internationalization (i18n) support for the main Shopify queries and loaders used in Deco.cx. With this update, it's now possible to dynamically pass
languageCodeandcountryCodevalues via the@inContextdirective, allowing the Shopify Storefront API to return localized content for different languages and regions.Additionally, this PR enhances the sitemap handling logic by introducing the ability to exclude specific URLs from the sitemap response using a new
excludeprop in the proxy handler.🧩 How It Works
✅ Loaders
Most loaders now accept the following optional props:
These values are passed as variables to GraphQL queries and injected using the
@inContextdirective.✅ Queries
All core Shopify queries have been refactored to use GraphQL variables in the
@inContextdirective (instead of interpolated JavaScript), making them fully compatible with GraphQL tooling and static code analysis like GraphQL Codegen.Example:
On the TypeScript side:
🗺️ Sitemap Enhancements
The sitemap proxy handler now accepts two optional props:
This enables:
include)exclude)Example usage:
{ "include": ["/sitemap_pages_custom.xml"], "exclude": [ "/sitemap_products_1.xml", "/sitemap_pages_1.xml", "/sitemap_collections_1.xml", "/sitemap_blogs_1.xml" ] }This resolves issues where Shopify includes non-localized or duplicated sitemap entries that should not be indexed, allowing more control over SEO structure.
🛠 Affected Queries
This pattern has been applied consistently across:
ListProductsSearchProductsProductsByCollectionProductRecommendationsGetProductand related queriesShopCart🌍 Impact
🎥 Loom Video
https://www.loom.com/share/8a8f204de9a745528ac2f84f44f428fa
Summary by CodeRabbit
New Features
Improvements