Add SEO support: meta tags, OpenGraph, and JSON-LD - #21
Merged
Conversation
Implement all SEO improvements from issue #9: - Unique titles and meta descriptions for every page - Canonical URLs for products and taxons - OpenGraph support (basic, images, prices) - JSON-LD structured data (Product, BreadcrumbList, Organization) - Twitter Card meta tags - React cache() wrappers for request-level data deduplication - ensureProtocol() helper for store URLs without scheme Closes #9 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds server-side metadata generators, SEO helpers, JSON‑LD component, and cached data wrappers; integrates metadata generation and conditional JSON‑LD rendering into layouts and storefront pages with guarded cached data fetching. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ServerPage as Server Page
participant MetadataGen as Metadata Generator
participant Cache as Cached Data Layer
participant JsonLdComponent as JsonLd Renderer
Client->>ServerPage: Request page (country/locale[/slug|/permalink])
ServerPage->>MetadataGen: generateMetadata(params)
MetadataGen->>Cache: getCachedStore / getCachedProduct / getCachedTaxon
Cache-->>MetadataGen: cached data (or throws → handled)
MetadataGen-->>ServerPage: Metadata (title, alternates, openGraph)
ServerPage->>Cache: fetch page data for render (try/catch)
Cache-->>ServerPage: page data (product/taxon/store or null)
ServerPage->>JsonLdComponent: build JSON‑LD via seo helpers (if data & canonical)
JsonLdComponent-->>ServerPage: <script type="application/ld+json">...</script>
ServerPage-->>Client: HTML + metadata + optional JSON‑LD
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/components/seo/JsonLd.tsx`:
- Around line 5-11: In JsonLd, avoid raw JSON.stringify output to prevent XSS
from sequences like "</script>": serialize the data (JSON.stringify(data)) into
a variable (e.g., serialized) then sanitize it by escaping angle-brackets (for
example replace all "<" with "\u003C" or specifically replace "</script" with an
escaped form) and use that sanitized string for dangerouslySetInnerHTML; update
the JsonLd function to produce the sanitized __html value before returning the
<script> element.
In `@src/lib/data/cached.ts`:
- Around line 1-6: React.cache is currently wrapping
getProduct/getTaxon/getStore which accept object args, so callers like
getCachedStore({ locale }) always pass new object references and the cache never
hits; replace these exports with thin primitive-arg wrappers that construct the
option/params objects internally (e.g. export getCachedStore = cache((locale) =>
getStore({ locale })) and similarly for getCachedTaxon and getCachedProduct) so
callers pass primitives (locale, id, etc.) and React.cache can deduplicate by
Object.is.
In `@src/lib/metadata/product.ts`:
- Around line 68-75: The product price keys currently put under Metadata.other
(the "product:price:amount" and "product:price:currency" entries generated from
product.price in src/lib/metadata/product.ts) produce <meta name="..."> tags
which crawlers won't recognize; remove those price entries from Metadata.other
and instead emit proper <meta property="..."> tags at the route level by adding
a head.tsx (or a Head component) for the product page (e.g.,
app/products/[id]/head.tsx) that returns <meta property="product:price:amount"
content={product.price.amount} /> and <meta property="product:price:currency"
content={product.price.currency} /> along with <meta property="og:type"
content="product" /> so crawlers see property attributes rather than name
attributes.
In `@src/lib/seo.ts`:
- Around line 7-12: ensureProtocol currently returns "https://" for empty or
whitespace-only input causing downstream new URL(...) failures; update
ensureProtocol(url: string) to trim the input, and if the trimmed string is
empty throw a clear error (e.g. "Invalid empty url") or return null so callers
can handle it; keep the existing behavior for non-empty values (preserve
detection for "http://" and "https://" and prepend "https://" otherwise). Locate
the function ensureProtocol and adjust its parameter handling and return/error
semantics, and then update callers (e.g., where metadataBase/new URL is
constructed) to handle the thrown error or null return accordingly.
🧹 Nitpick comments (1)
src/lib/metadata/home.ts (1)
26-45: Minor:buildCanonicalUrlcalled twice with identical arguments.The canonical URL is computed on Line 32 and again on Line 40. Consider reusing the variable already assigned on Line 21-23 (
canonicalUrlis available but computed conditionally). Actually, looking again,canonicalUrlis defined on Line 21 and already used on Line 32 — but theopenGraph.urlon Line 40 recomputes it. You could use the existingcanonicalUrlvariable:♻️ Suggested simplification
openGraph: { title: storeName, description, - ...(store?.url - ? { url: buildCanonicalUrl(store.url, `/${country}/${locale}`) } - : {}), + ...(canonicalUrl ? { url: canonicalUrl } : {}), type: "website",
- Sanitize JsonLd output to prevent XSS via </script> injection - Fix React.cache wrappers to use primitive args (Object.is comparison) - Add trim() to ensureProtocol for whitespace-padded URLs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/app/[country]/[locale]/(storefront)/products/[slug]/page.tsx (1)
27-39: PreferPromise.allfor parallel product and store fetches.Neither fetch depends on the other's result; both can be issued concurrently. Although
React.cachemakes these cache hits in practice (both were called duringgenerateMetadata), expressing parallel intent aligns with the codebase's convention.♻️ Proposed refactor
- let product; - try { - product = await getCachedProduct(slug, "images", locale); - } catch { - product = null; - } - - let store; - try { - store = await getCachedStore(locale); - } catch { - store = null; - } + const [product, store] = await Promise.all([ + getCachedProduct(slug, "images", locale).catch(() => null), + getCachedStore(locale).catch(() => null), + ]);As per coding guidelines: "Use Promise.all() to fetch data in parallel from multiple sources in server components instead of sequential awaits".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[country]/[locale]/(storefront)/products/[slug]/page.tsx around lines 27 - 39, Both fetches should run in parallel: replace the sequential try/await blocks for getCachedProduct(slug, "images", locale) and getCachedStore(locale) with a Promise.all (or Promise.allSettled) call so both requests are issued concurrently; then assign product and store from the results, converting any failed result to null (preserve variable names product and store) and keep error swallowing behavior per original by mapping rejected results to null. Ensure you reference the existing functions getCachedProduct and getCachedStore and preserve the slug and locale parameters when invoking them.src/lib/metadata/product.ts (1)
16-28: PreferPromise.allfor parallel product and store fetches.The sequential awaits add unnecessary latency when both requests are independent. The guideline requires
Promise.allfor parallel fetches in server-side code.♻️ Proposed refactor
- let product; - try { - product = await getCachedProduct(slug, "images", locale); - } catch { - return { title: "Product Not Found" }; - } - - let store; - try { - store = await getCachedStore(locale); - } catch { - store = null; - } + const [product, store] = await Promise.all([ + getCachedProduct(slug, "images", locale).catch(() => null), + getCachedStore(locale).catch(() => null), + ]); + + if (!product) return { title: "Product Not Found" };As per coding guidelines: "Use Promise.all() to fetch data in parallel from multiple sources in server components instead of sequential awaits".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/product.ts` around lines 16 - 28, Replace the sequential awaits for getCachedProduct(slug, "images", locale) and getCachedStore(locale) with a parallel fetch using Promise.allSettled (or equivalent) so both requests run concurrently; then inspect the results: if the getCachedProduct promise is rejected return { title: "Product Not Found" } (preserving existing behavior), and if getCachedStore is rejected set store = null. Reference the existing symbols product, store, getCachedProduct, getCachedStore, slug, and locale when making the change.src/lib/data/cached.ts (1)
1-1: Prefer importing from local data wrappers for consistency.Importing
getProduct,getStore,getTaxondirectly from@spree/nextbypassessrc/lib/data/{products,store,taxonomies}.ts, which are the intended SDK abstraction layer. Any future additions to those wrappers (error handling, telemetry, type narrowing) won't apply to the cached versions.♻️ Suggested change
-import { getProduct, getStore, getTaxon } from "@spree/next"; +import { getProduct } from "@/lib/data/products"; +import { getStore } from "@/lib/data/store"; +import { getTaxon } from "@/lib/data/taxonomies";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/data/cached.ts` at line 1, Replace direct imports of getProduct, getStore, and getTaxon from `@spree/next` with imports from the local data wrapper modules (the project's products, store, and taxonomies data wrappers) so the cached code uses the SDK abstraction layer; update the import statement that currently references getProduct, getStore, getTaxon to import the same named functions from the corresponding local wrapper modules to ensure shared error handling/telemetry/type narrowing is applied.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/metadata/category.ts`:
- Around line 33-36: The meta fallback uses raw taxon.description which may
contain HTML; update the assignment for the description variable so that when
taxon.meta_description is falsy you use a sanitized/plain-text version of
taxon.description (strip HTML tags, decode entities and trim/collapse
whitespace) before falling back to `Browse ${taxon.name} products.`; refer to
the description variable and the taxon.meta_description / taxon.description
fields and either call an existing utility (e.g., a sanitize/stripHtml helper)
or add a small strip-html utility and use it in the expression.
In `@src/lib/metadata/store.ts`:
- Line 22: The current object spread uses new URL(ensureProtocol(store.url))
directly which can throw for malformed store.url; wrap the URL construction in a
safe check so metadataBase is only set when the URL is valid—use a try/catch (or
a URL-validation helper) around new URL(...) and fall back to omitting
metadataBase on error, and log or ignore the error as appropriate; update the
spread expression that sets metadataBase (the ...(store?.url ? { metadataBase:
new URL(ensureProtocol(store.url)) } : {}) code) to perform this guarded
construction referencing ensureProtocol and metadataBase.
---
Nitpick comments:
In `@src/app/`[country]/[locale]/(storefront)/products/[slug]/page.tsx:
- Around line 27-39: Both fetches should run in parallel: replace the sequential
try/await blocks for getCachedProduct(slug, "images", locale) and
getCachedStore(locale) with a Promise.all (or Promise.allSettled) call so both
requests are issued concurrently; then assign product and store from the
results, converting any failed result to null (preserve variable names product
and store) and keep error swallowing behavior per original by mapping rejected
results to null. Ensure you reference the existing functions getCachedProduct
and getCachedStore and preserve the slug and locale parameters when invoking
them.
In `@src/lib/data/cached.ts`:
- Line 1: Replace direct imports of getProduct, getStore, and getTaxon from
`@spree/next` with imports from the local data wrapper modules (the project's
products, store, and taxonomies data wrappers) so the cached code uses the SDK
abstraction layer; update the import statement that currently references
getProduct, getStore, getTaxon to import the same named functions from the
corresponding local wrapper modules to ensure shared error
handling/telemetry/type narrowing is applied.
In `@src/lib/metadata/product.ts`:
- Around line 16-28: Replace the sequential awaits for getCachedProduct(slug,
"images", locale) and getCachedStore(locale) with a parallel fetch using
Promise.allSettled (or equivalent) so both requests run concurrently; then
inspect the results: if the getCachedProduct promise is rejected return { title:
"Product Not Found" } (preserving existing behavior), and if getCachedStore is
rejected set store = null. Reference the existing symbols product, store,
getCachedProduct, getCachedStore, slug, and locale when making the change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/metadata/store.ts (2)
22-30: Replace the IIFE with a pre-computedmetadataBasevariable.The self-invoking function nested inside a conditional spread (
store?.url ? (() => { try/catch })() : {}) is hard to read. Hoisting the URL construction into a plainletblock above thereturnproduces identical behaviour with far less cognitive overhead.♻️ Proposed refactor
+ let metadataBase: URL | undefined; + if (store?.url) { + try { + metadataBase = new URL(ensureProtocol(store.url)); + } catch { + // ignore malformed store URL + } + } + return { - ...(store?.url - ? (() => { - try { - return { metadataBase: new URL(ensureProtocol(store.url)) }; - } catch { - return {}; - } - })() - : {}), + ...(metadataBase ? { metadataBase } : {}), title: {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/store.ts` around lines 22 - 30, Hoist the IIFE by precomputing a metadataBase variable before the returned object: declare a let metadataBase = undefined; if (store?.url) { try { metadataBase = { metadataBase: new URL(ensureProtocol(store.url)) }; } catch { metadataBase = {}; } } then replace the spread ...(store?.url ? (() => { try { return { metadataBase: new URL(ensureProtocol(store.url)) }; } catch { return {}; } })() : {}) with ...metadataBase in the return; ensure you reference ensureProtocol and new URL exactly as in the original code and preserve the original try/catch behavior and resulting empty object on failure.
12-17: Consider explicitly typingstorefor clarity.
let store;relies on TypeScript's control-flow inference from the try/catch assignments. While it works, an explicit type keeps the contract clear ifgetCachedStore's return type ever changes.♻️ Proposed change
- let store; + let store: Awaited<ReturnType<typeof getCachedStore>> | null = null; try { store = await getCachedStore(locale); } catch { store = null; }As per coding guidelines,
**/*.ts{,x}: use strict TypeScript type checking and avoid implicit typing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/store.ts` around lines 12 - 17, Declare an explicit type for the local variable `store` instead of `let store;` — e.g., set it to the return type of `getCachedStore` (or the known store interface) unioned with null (like ReturnType<typeof getCachedStore> | null or Store | null), then keep the existing try/catch logic (`store = await getCachedStore(locale)` and `store = null` on catch); ensure you import or reference the correct Store type if needed so the declaration provides a clear contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/metadata/category.ts`:
- Around line 33-36: The meta description can contain HTML entities because
stripHtml (used in category.ts when building description) only removes tags;
update stripHtml in src/lib/seo.ts to also decode HTML entities after stripping
tags (or add a small helper decodeHtmlEntities and call it from stripHtml) so
that taxon.description like "<p>Apples & Oranges</p>" yields "Apples &
Oranges"; ensure stripHtml still trims and returns null/empty behavior unchanged
and keep the call site in category.ts (where description is computed) intact.
- Around line 42-56: The category metadata generator currently returns an object
with openGraph but no twitter key; update the returned object (the same place
where title, description, canonicalUrl, and openGraph are composed) to add a
twitter property after openGraph with card: "summary_large_image", title,
description, and conditionally include images when taxon.image_url exists (use
images: [taxon.image_url]); ensure you mirror the existing logic used for
openGraph images and for canonicalUrl so the new twitter object references the
same title/description and conditional image from taxon.
---
Nitpick comments:
In `@src/lib/metadata/store.ts`:
- Around line 22-30: Hoist the IIFE by precomputing a metadataBase variable
before the returned object: declare a let metadataBase = undefined; if
(store?.url) { try { metadataBase = { metadataBase: new
URL(ensureProtocol(store.url)) }; } catch { metadataBase = {}; } } then replace
the spread ...(store?.url ? (() => { try { return { metadataBase: new
URL(ensureProtocol(store.url)) }; } catch { return {}; } })() : {}) with
...metadataBase in the return; ensure you reference ensureProtocol and new URL
exactly as in the original code and preserve the original try/catch behavior and
resulting empty object on failure.
- Around line 12-17: Declare an explicit type for the local variable `store`
instead of `let store;` — e.g., set it to the return type of `getCachedStore`
(or the known store interface) unioned with null (like ReturnType<typeof
getCachedStore> | null or Store | null), then keep the existing try/catch logic
(`store = await getCachedStore(locale)` and `store = null` on catch); ensure you
import or reference the correct Store type if needed so the declaration provides
a clear contract.
damianlegawiec
requested changes
Feb 18, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/lib/metadata/category.ts (1)
5-8: Consider exportingCategoryMetadataParams.The interface is used as the parameter type of the exported function. Not exporting it prevents consumers from referencing or extending the type without duplicating it.
♻️ Proposed refactor
-interface CategoryMetadataParams { +export interface CategoryMetadataParams {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/category.ts` around lines 5 - 8, The interface CategoryMetadataParams should be exported so consumers can reference or extend it; modify the declaration to export interface CategoryMetadataParams { country: string; locale: string; permalink: string[] } and leave the exported function signature that uses CategoryMetadataParams unchanged so external callers can import this type instead of duplicating it.src/lib/metadata/store.ts (2)
22-29: Optional: simplify themetadataBaseSpreadtype annotation.
Record<string, never>is an unconventional type for an empty-object placeholder.Partial<{ metadataBase: URL }>communicates the intent more directly and collapses the union.♻️ Proposed simplification
- let metadataBaseSpread: { metadataBase: URL } | Record<string, never> = {}; + let metadataBaseSpread: Partial<{ metadataBase: URL }> = {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/store.ts` around lines 22 - 29, The type for metadataBaseSpread uses an odd union with Record<string, never>; change it to a clearer Partial<{ metadataBase: URL }> and keep the same initialization logic: set metadataBaseSpread to {} by default and when store?.url exists assign { metadataBase: new URL(ensureProtocol(store.url)) } inside the try/catch (preserving the catch to reset to {}). Update the variable declaration of metadataBaseSpread to use Partial<{ metadataBase: URL }> to reflect the intent.
22-29: Optional: use a cleaner type formetadataBaseSpread.
Record<string, never>is an atypical way to express an empty-object placeholder.Partial<{ metadataBase: URL }>communicates the intent more directly and removes the union.♻️ Proposed simplification
- let metadataBaseSpread: { metadataBase: URL } | Record<string, never> = {}; + let metadataBaseSpread: Partial<{ metadataBase: URL }> = {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/store.ts` around lines 22 - 29, Replace the awkward union type for metadataBaseSpread with a clearer Partial type: change the declaration from "let metadataBaseSpread: { metadataBase: URL } | Record<string, never> = {}" to "let metadataBaseSpread: Partial<{ metadataBase: URL }> = {}". Keep the runtime logic unchanged (the try/catch that sets metadataBaseSpread = { metadataBase: new URL(ensureProtocol(store.url)) } or {}), this simplifies the intent while preserving behavior and references to metadataBaseSpread, ensureProtocol, and store?.url.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/metadata/category.ts`:
- Around line 33-36: The fallback meta description uses taxon.description raw
(variable description), which may contain HTML; import and call stripHtml before
using taxon.description so the meta description is plain text; update the
construction of the description constant (alongside buildCanonicalUrl import) to
use stripHtml(taxon.description) instead of taxon.description, preserving
taxon.meta_description precedence.
In `@src/lib/metadata/store.ts`:
- Line 42: Normalize openGraph.locale before emitting by converting BCP 47 to
Open Graph underscore format: split the incoming locale (e.g., the variable
named locale / openGraph.locale) on '-' or '_', make the language subtag
lowercase and the region/territory subtag uppercase, then join with '_' (e.g.,
"en-US" -> "en_US", "pt-br" -> "pt_BR"); update the assignment/return in
src/lib/metadata/store.ts where locale is passed to openGraph (the symbol
openGraph.locale / locale) to apply this transformation and ensure only the
normalized value is emitted.
---
Nitpick comments:
In `@src/lib/metadata/category.ts`:
- Around line 5-8: The interface CategoryMetadataParams should be exported so
consumers can reference or extend it; modify the declaration to export interface
CategoryMetadataParams { country: string; locale: string; permalink: string[] }
and leave the exported function signature that uses CategoryMetadataParams
unchanged so external callers can import this type instead of duplicating it.
In `@src/lib/metadata/store.ts`:
- Around line 22-29: The type for metadataBaseSpread uses an odd union with
Record<string, never>; change it to a clearer Partial<{ metadataBase: URL }> and
keep the same initialization logic: set metadataBaseSpread to {} by default and
when store?.url exists assign { metadataBase: new URL(ensureProtocol(store.url))
} inside the try/catch (preserving the catch to reset to {}). Update the
variable declaration of metadataBaseSpread to use Partial<{ metadataBase: URL }>
to reflect the intent.
- Around line 22-29: Replace the awkward union type for metadataBaseSpread with
a clearer Partial type: change the declaration from "let metadataBaseSpread: {
metadataBase: URL } | Record<string, never> = {}" to "let metadataBaseSpread:
Partial<{ metadataBase: URL }> = {}". Keep the runtime logic unchanged (the
try/catch that sets metadataBaseSpread = { metadataBase: new
URL(ensureProtocol(store.url)) } or {}), this simplifies the intent while
preserving behavior and references to metadataBaseSpread, ensureProtocol, and
store?.url.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/metadata/store.ts (2)
6-10:normalizeOpenGraphLocalesilently drops the region subtag for 3-part BCP 47 locales.For a locale like
zh-Hans-CN,partsis['zh', 'Hans', 'CN']andparts[1]isHans, so the function emitszh_HANSand discardsCN. Most Spree setups use 2-part locales so this is a low-probability edge case, but it produces a non-conformantog:localesilently.🛠️ Proposed fix — use last subtag as the region
function normalizeOpenGraphLocale(locale: string): string { const parts = locale.split(/[-_]/); if (parts.length < 2) return locale; - return `${parts[0].toLowerCase()}_${parts[1].toUpperCase()}`; + // language + last subtag as region (e.g. zh-Hans-CN → zh_CN) + return `${parts[0].toLowerCase()}_${parts[parts.length - 1].toUpperCase()}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/store.ts` around lines 6 - 10, normalizeOpenGraphLocale currently drops the region for 3+ part BCP 47 tags (e.g. "zh-Hans-CN") because it always uses parts[1] as the region; update normalizeOpenGraphLocale to use the last subtag as the region (parts[parts.length - 1]) and the first subtag as the language, so it returns language lowercased + "_" + region uppercased (and still returns the input early if there are fewer than 2 parts). Locate the function normalizeOpenGraphLocale and replace the region selection logic to pick the final subtag from the split array instead of parts[1], preserving the same casing rules.
6-10:normalizeOpenGraphLocalesilently drops the region subtag for 3-part BCP 47 locales.
locale.split(/[-_]/)onzh-Hans-CNproduces['zh', 'Hans', 'CN'];parts[1]isHans, so the function emitszh_HANSand discardsCN. Most Spree setups use 2-part locales, so this is an edge case, but if a 3-part locale is ever configured the emittedog:localewill be malformed.🛠️ Proposed fix
function normalizeOpenGraphLocale(locale: string): string { const parts = locale.split(/[-_]/); if (parts.length < 2) return locale; - return `${parts[0].toLowerCase()}_${parts[1].toUpperCase()}`; + // Use language + last subtag as region (e.g. zh-Hans-CN → zh_CN) + return `${parts[0].toLowerCase()}_${parts[parts.length - 1].toUpperCase()}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/store.ts` around lines 6 - 10, normalizeOpenGraphLocale currently splits on -/_ and uses parts[0] and parts[1], which drops the region for 3-part BCP47 tags (e.g., "zh-Hans-CN"); update the function so it uses the first subtag as the language and the last subtag as the region (i.e., return `${parts[0].toLowerCase()}_${parts[parts.length - 1].toUpperCase()}` when parts.length >= 2), preserving original input when there is no region; change the logic in normalizeOpenGraphLocale accordingly to avoid emitting malformed og:locale values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/metadata/store.ts`:
- Around line 43-51: The openGraph block in the metadata builder does not set
description, so og:description is never emitted; update the openGraph object
(the openGraph property in src/lib/metadata/store.ts) to include description:
store?.meta_description || "Online store powered by Spree Commerce" (or same
fallback used for the top-level description) so that openGraph.description is
explicitly provided as a fallback for child pages; ensure you add this to the
existing openGraph literal alongside siteName, locale, type, and images.
- Around line 43-51: The openGraph object is missing a description field so
og:description won't be emitted or inherited; add description:
store?.meta_description || "Online store powered by Spree Commerce" to the
openGraph literal (next to siteName/locale/type/images) so
metadata.openGraph.description has the same fallback as the top-level
description, ensuring child segments that override openGraph still get the
fallback; update the code that builds openGraph (the openGraph object where
siteName, locale, type, images are set and normalizeOpenGraphLocale is used) to
include this description property.
---
Duplicate comments:
In `@src/lib/metadata/category.ts`:
- Around line 33-36: The meta description currently uses taxon.description raw
which may include HTML; update the logic in the const description assignment to
sanitize taxon.description by calling stripHtml(taxon.description). Ensure
stripHtml is imported/available and use its returned plain text (falling back to
the template string if stripHtml yields empty). Reference the existing symbol
const description and the taxon.description value when applying stripHtml.
In `@src/lib/metadata/store.ts`:
- Around line 52-61: No changes required—the twitter card and `@-prefix`
normalization in the twitter property of the metadata store (the twitter: {...}
block using store?.twitter and startsWith("@") logic) are correct; leave this
code as-is and approve the change.
- Around line 52-61: No changes required: the twitter card construction and
handle normalization in the twitter object (using store.twitter and the
conditional spread) are correct as written—keep the current logic that prefixes
an "@" when store.twitter does not startWith("@") and retain the existing
structure alongside normalizeOpenGraphLocale usage for locale handling.
- Around line 28-35: The metadataBase construction already guards against
malformed URLs by checking store?.url and wrapping new
URL(ensureProtocol(store.url)) in a try/catch, so no change is necessary; keep
the current logic around the metadataBaseSpread variable and the use of
ensureProtocol and the try/catch block as-is to prevent unhandled errors from
invalid store.url values.
- Around line 28-35: The code safely guards metadataBase construction by
checking store?.url and wrapping new URL(ensureProtocol(store.url)) in a
try/catch; keep this pattern but ensure you return or spread metadataBaseSpread
where used (reference metadataBaseSpread, ensureProtocol, and the new URL
creation) so callers receive either a valid metadataBase: URL or an empty
object; if any upstream logic expects a specific shape, update that consumer to
handle the empty case instead of assuming metadataBase exists.
---
Nitpick comments:
In `@src/lib/metadata/store.ts`:
- Around line 6-10: normalizeOpenGraphLocale currently drops the region for 3+
part BCP 47 tags (e.g. "zh-Hans-CN") because it always uses parts[1] as the
region; update normalizeOpenGraphLocale to use the last subtag as the region
(parts[parts.length - 1]) and the first subtag as the language, so it returns
language lowercased + "_" + region uppercased (and still returns the input early
if there are fewer than 2 parts). Locate the function normalizeOpenGraphLocale
and replace the region selection logic to pick the final subtag from the split
array instead of parts[1], preserving the same casing rules.
- Around line 6-10: normalizeOpenGraphLocale currently splits on -/_ and uses
parts[0] and parts[1], which drops the region for 3-part BCP47 tags (e.g.,
"zh-Hans-CN"); update the function so it uses the first subtag as the language
and the last subtag as the region (i.e., return
`${parts[0].toLowerCase()}_${parts[parts.length - 1].toUpperCase()}` when
parts.length >= 2), preserving original input when there is no region; change
the logic in normalizeOpenGraphLocale accordingly to avoid emitting malformed
og:locale values.
…arams Resolve merge conflict in category page by keeping SEO features (metadata, JSON-LD, cached data helpers) while adopting the new SDK parameter format: `expand: string[]` replaces `includes: string`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove all store.url usage and getCachedStore dependency (getStore was removed from @spree/next). Store SEO settings (URL, title, description, keywords, twitter, social links) now come from environment variables. Add dummy public/social-image.png placeholder for OG image. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # src/app/[country]/[locale]/(storefront)/page.tsx # src/app/[country]/[locale]/(storefront)/t/[...permalink]/page.tsx # src/app/[country]/[locale]/(storefront)/taxonomies/page.tsx # src/app/[country]/[locale]/layout.tsx
- Add NEXT_PUBLIC_STORE_NAME and NEXT_PUBLIC_STORE_DESCRIPTION env vars - Expose storeName/storeDescription via StoreContext for client components - Add getStoreDescription() helper in seo.ts - Replace all hardcoded "Spree Store" with env-driven values across layout, Header, Footer, checkout layout, home page, and metadata helpers - Fix build error: migrate cached.ts from deleted getTaxon to getCategory - Add generateMetadata + JsonLd breadcrumbs to new c/[...permalink] page - Update buildBreadcrumbJsonLd: StoreTaxon → Category, /t/ → /c/ paths - Fix stale /taxonomies canonical URL in categories metadata Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 27, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
<title>and<meta name="description">for every page (home, products, product detail, category, taxonomies)store.urlfrom Spree APIog:image), and product prices (product:price:amount,product:price:currency)summary_large_image)Product(with offers/availability),BreadcrumbList(for categories), andOrganization(on all pages)ensureProtocol()helper to handle store URLs without a scheme prefixcache()wrappers for request-level data deduplication betweengenerateMetadataand page componentssrc/lib/metadata/*.tsfor clean separationthumbnail_urlwhenproduct.imagesis not populatedCloses #9
Test plan
<title>and<meta name="description">on: home, products list, product detail, category, taxonomies pages<link rel="canonical">with full URL on product and category pagesog:title,og:description,og:url,og:image,og:type) in page sourceproduct:price:amountandproduct:price:currencymeta tags on product pagesProductschema on product detail pages (name, description, image, offers)BreadcrumbListschema on category pages (Home → Categories → ...)Organizationschema on all pages (name, url, contactPoint)npm run buildpasses without errorsnpm run check(Biome) passes without new errors🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor