Skip to content

Add sitemap.xml and robots.txt generation#19

Open
Cichorek wants to merge 15 commits intomainfrom
sitemap
Open

Add sitemap.xml and robots.txt generation#19
Cichorek wants to merge 15 commits intomainfrom
sitemap

Conversation

@Cichorek
Copy link
Contributor

@Cichorek Cichorek commented Feb 13, 2026

Summary

Implements dynamic sitemap and robots.txt generation for the storefront, resolving #10.

  • src/app/sitemap.ts — generates XML sitemap with all products (including image sitemaps), taxon/category pages, and static pages (homepage, products listing, taxonomies). Supports multi-locale via SITEMAP_LOCALE_MODE env var (default, selected, all). Automatically splits into multiple sitemap files when exceeding Google's 50k URL limit via generateSitemaps().
  • src/app/robots.ts — generates robots.txt with sensible disallow rules (account, cart, checkout, query params) and optionally blocks AI training crawlers (GPTBot, ClaudeBot, CCBot, etc.) controlled by ROBOTS_DISALLOW_AI env var.
  • .env.example / README.md — documents all new configuration options.

Configuration

Variable Description Default
SITEMAP_LOCALE_MODE default / selected / all default
SITEMAP_COUNTRIES Comma-separated ISO codes (for selected mode) (empty)
ROBOTS_DISALLOW_AI Block AI training bots true
NEXT_PUBLIC_SITE_URL Fallback URL if store.url is not set (empty)

Test plan

  • npm run build passes — sitemap rendered as SSG, robots as static
  • npm run check / npm run lint — no errors in new files
  • /sitemap/0.xml generates correctly with products, categories, image sitemaps, and static pages
  • /robots.txt generates correctly with disallow rules and AI crawler blocking

Closes #10

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Configurable AI crawler blocking for listed AI user-agents
    • Multi-region, multi-locale sitemap generation with chunking for large sites
  • Documentation

    • Added optional environment variables and defaults for sitemap, robots, locale/country, and Sentry
    • Documented sitemap locale modes and updated deployment guidance
  • Chores

    • Extended example environment configuration with new options and examples

Cichorek and others added 2 commits February 13, 2026 16:47
…back

- Remove `export const revalidate` from sitemap.ts and robots.ts — not
  supported in Next.js 16 metadata route files, caused build failure
- Add NEXT_PUBLIC_SITE_URL fallback when store.url is empty
- Fix Biome formatting in sitemap.ts
- Use single Date instance for static pages lastModified
- Add early return optimization for single-chunk sitemaps
- Remove unused SITEMAP_REVALIDATE_SECONDS env variable

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds environment variables and README docs for sitemap, robots, and Sentry; implements a robots MetadataRoute and a chunked, multi-country/locale sitemap generator (with caching, pagination, and locale-resolution logic).

Changes

Cohort / File(s) Summary
Configuration
\.env.example
Adds SITEMAP_LOCALE_MODE, SITEMAP_COUNTRIES, ROBOTS_DISALLOW_AI, and Sentry vars (SENTRY_DSN, SENTRY_ORG, SENTRY_PROJECT, SENTRY_AUTH_TOKEN); preserves commented NEXT_PUBLIC_SITE_URL.
Documentation
README.md
Adds SITEMAP_LOCALE_MODE, SITEMAP_COUNTRIES, ROBOTS_DISALLOW_AI to Optional vars table; documents sitemap locale modes (default, selected, all) and updates development/deployment notes regarding sitemap config.
Robots metadata route
src/app/robots.ts
New MetadataRoute export robots() that reads ROBOTS_DISALLOW_AI, fetches store base URL, builds allow/disallow rules (sensitive paths), and conditionally disallows known AI crawlers; returns rules, sitemap URL, and host.
Sitemap generator & route
src/app/sitemap.ts
New generateSitemaps() and default sitemap() exports. Resolves country/locale pairs per SITEMAP_LOCALE_MODE, computes item counts for chunking (50k limit), paginates to fetch all products/taxons (with images) with in-memory caching, builds per-locale static/product/taxon entries (images, lastModified, priorities), slices by sitemap chunk id, and logs warnings/fallbacks for missing/invalid config.
Helpers & constants (internal)
src/app/sitemap.ts (internal)
Adds resolveCountryLocales, fetchTotalCount, fetchAllProducts, fetchAllTaxons, safeLastModified, URLS_PER_SITEMAP, STATIC_PAGES_PER_LOCALE, types and validation/warning logic used by sitemap generation.

Sequence Diagram(s)

sequenceDiagram
    participant NextApp as Next.js App
    participant StoreAPI as Store API
    participant Products as Products Service
    participant Taxons as Taxons Service
    participant Generator as Sitemap/Robots Logic

    NextApp->>StoreAPI: fetch store config (baseUrl, defaults, countries)
    StoreAPI-->>NextApp: store data

    alt generateSitemaps()
        NextApp->>Products: fetch product counts
        Products-->>NextApp: product count
        NextApp->>Taxons: fetch taxon counts
        Taxons-->>NextApp: taxon count
        NextApp->>Generator: calculate sitemap chunks
        Generator-->>NextApp: chunk descriptors
    end

    alt sitemap(id)
        NextApp->>Generator: resolveCountryLocales(mode, countries)
        Generator->>StoreAPI: (if needed) fetch countries/locales
        StoreAPI-->>Generator: country/locale pairs

        Generator->>Products: fetch all products (with images)
        Products-->>Generator: product list
        Generator->>Taxons: fetch all taxons
        Taxons-->>Generator: taxon list

        Generator->>Generator: build per-locale URLs, set lastModified/changefreq/priority
        Generator-->>NextApp: return sitemap chunk (id)
    end

    alt robots()
        NextApp->>Generator: read ROBOTS_DISALLOW_AI, fetch store baseUrl
        Generator-->>NextApp: return robots rules, sitemap URL, host
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
I hopped through envs and locale streams,
Wove sitemaps in tidy, chunked dreams.
Robots now guard the AI door,
Pages indexed, flags set — explore!
Thump-thump — deploy, let the crawlers soar.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add sitemap.xml and robots.txt generation' clearly and concisely summarizes the main changes, accurately reflecting the primary feature additions in the changeset.
Linked Issues check ✅ Passed All primary coding objectives from issue #10 are fully implemented: dynamic sitemap generation with multi-locale support, robots.txt generation with AI crawler blocking, environment variable configuration, and documentation updates.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing sitemap/robots functionality and documenting configuration; no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sitemap

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/app/sitemap.ts`:
- Around line 110-134: The sitemap generation uses new Date(product.updated_at)
and new Date(taxon.updated_at) which can produce "Invalid Date" if those fields
are null/undefined; update the product and taxon entry creation (the for-loops
that call entries.push) to compute lastModified defensively—e.g. use
product.updated_at ? new Date(product.updated_at) : (product.created_at ? new
Date(product.created_at) : undefined) and the same for taxon (taxon.updated_at ?
new Date(taxon.updated_at) : (taxon.created_at ? new Date(taxon.created_at) :
undefined)), and only include lastModified in the pushed object when the
computed value is a valid Date to avoid emitting "Invalid Date" in the sitemap.
- Around line 149-204: The function resolveCountryLocales currently casts
process.env.SITEMAP_LOCALE_MODE to SitemapLocaleMode without validation,
allowing typos to fall through to the "all" branch; update resolveCountryLocales
to validate the computed mode (derived from process.env.SITEMAP_LOCALE_MODE)
against the allowed SitemapLocaleMode values ("default","selected","all"), and
if it's not one of them log a warning and set mode to a safe default (e.g.,
"default") — use the existing mode variable and SitemapLocaleMode type for the
check and ensure subsequent logic (the "default"/"selected"/"all" branches) uses
this validated value.
🧹 Nitpick comments (2)
src/app/sitemap.ts (1)

59-75: Each sitemap chunk re-fetches all products and taxons.

sitemap() is called once per chunk ID. Each invocation fetches the complete product and taxon lists, then slices to its chunk. For stores large enough to need multiple chunks this multiplies API calls at build time (e.g., 3 chunks = 3× full fetches).

This is acceptable for SSG builds of moderate-size stores, but worth noting: if the store has tens of thousands of products, consider caching the fetched data across chunks or restructuring so each chunk fetches only its slice.

README.md (1)

237-240: Consider mentioning ROBOTS_DISALLOW_AI in the deploy section as well.

The deploy steps list sitemap-related env vars as optional but omit ROBOTS_DISALLOW_AI. It defaults to true so it's not strictly needed, but for completeness it could be mentioned alongside the other optional vars.

Cichorek and others added 3 commits February 13, 2026 17:14
robots.ts now imports generateSitemaps() and dynamically lists all
sitemap chunk URLs (/sitemap/0.xml, /sitemap/1.xml, etc.) instead of
the non-existent /sitemap.xml — Next.js does not auto-generate a
sitemap index when using generateSitemaps().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add safeLastModified() helper that guards against null/undefined/invalid
  date strings in product.updated_at and taxon.updated_at — omits
  lastModified instead of emitting "Invalid Date" in the XML
- Validate SITEMAP_LOCALE_MODE against allowed values; log a warning
  and fall back to "default" on typos instead of silently hitting the
  "all" branch

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 96-103: MD028 is triggered by the blank line between the two
blockquote sections ("Sitemap locale modes:" and "Privacy note:") in README.md;
fix it by removing the blank line or inserting a non-blockquote separator (for
example, an HTML comment line) between the two blockquotes so they remain
separate lines without a blank blockquote, and re-run markdownlint to confirm
the MD028 warning is resolved.

In `@src/app/sitemap.ts`:
- Around line 116-123: The mapping of product.images to images uses a filter
that only checks url !== null, which allows undefined to pass and violates the
asserted type string; update the filter used after product.images.map((img) =>
img.original_url) (the images mapping) to guard against both null and undefined
and ensure the value is a string (e.g., use a type guard that checks typeof url
=== 'string' and/or url != null) so only real string URLs reach the sitemap
images array.
🧹 Nitpick comments (3)
src/app/sitemap.ts (3)

59-75: Every sitemap chunk fetches all products and taxons, then discards most of the data.

sitemap() is called once per chunk (e.g., /sitemap/0.xml, /sitemap/1.xml). Each invocation runs fetchAllProducts() + fetchAllTaxons() and builds the complete entry array, only to slice one window. For stores that exceed 50k URLs (the only scenario with multiple chunks), this multiplies API calls and memory usage by the number of chunks.

This is acceptable for most stores today (single chunk), but worth a // TODO so it doesn't become a silent scaling problem. A future optimisation could compute the offset per chunk and fetch only the needed page range.

Also applies to: 137-142


236-236: VALID_LOCALE_MODES is declared after its first reference inside resolveCountryLocales (line 153).

This works at runtime because resolveCountryLocales is only called after the module finishes initialising, but it hurts readability — a reader scanning top-down sees an identifier before its definition. Consider moving it up near the other module-level constants (after line 29).

Suggested move
 const URLS_PER_SITEMAP = 50_000;
 const STATIC_PAGES_PER_LOCALE = 3;
+const VALID_LOCALE_MODES: SitemapLocaleMode[] = ["default", "selected", "all"];

…and remove line 236.


238-270: Pagination loops lack error handling — a transient API failure mid-pagination will throw an unhandled error and produce no sitemap at all.

If the Spree API returns an error on page 3 of 5, the entire sitemap() / generateSitemaps() call fails. Consider wrapping the fetch in a try/catch with a retry or, at minimum, logging which page failed so it's diagnosable.

Also, there's no safety bound on the loop — if response.meta.pages is unexpectedly huge or the API keeps returning new pages, this runs indefinitely. A reasonable upper limit (e.g., 1 000 pages → 100k items) would be prudent.

Cichorek and others added 2 commits February 16, 2026 16:48
The type guard `url is string` asserted string type but the runtime
check `!== null` allowed undefined to pass through. Changed to loose
equality `!= null` which excludes both null and undefined.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (2)
src/app/sitemap.ts (2)

199-205: Unmatched SITEMAP_COUNTRIES ISO code silently uses the fallback locale.

If a user lists a country ISO (e.g., "xx") that doesn't exist in the store's countries, found is undefined, found?.default_locale is undefined, and the entry is emitted as { country: "xx", locale: storeDefaultLocale }. This generates sitemap URLs for a country that may not be served by the store, with no diagnostic output to help debug the misconfiguration.

🛡️ Proposed fix
  return selectedIsos.map((iso) => {
    const found = allCountries.find((c) => c.iso.toLowerCase() === iso);
+    if (!found) {
+      console.warn(`SITEMAP_COUNTRIES: country "${iso}" not found in store countries. Skipping.`);
+      return null;
+    }
    return {
      country: iso,
      locale: found?.default_locale || storeDefaultLocale,
    };
-  });
+  }).filter((entry): entry is CountryLocale => entry !== null);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` around lines 199 - 205, The mapping over selectedIsos
currently emits entries for unknown ISO codes by using storeDefaultLocale when
found is undefined; update the logic in the selectedIsos.map block (the code
that computes found from allCountries and returns { country: iso, locale:
found?.default_locale || storeDefaultLocale }) to instead filter out or skip
ISOs that have no matching entry in allCountries and emit a warning (e.g., via
console.warn or the repo logger) listing the unknown ISO(s) so misconfigurations
are visible; if you still want to keep a fallback behavior make it explicit
(e.g., push a diagnostic log when using storeDefaultLocale for an unmatched ISO)
but prefer dropping unmatched entries to avoid generating sitemap URLs for
countries the store doesn’t serve.

40-57: generateSitemaps() is missing an explicit return type.

The coding guidelines require explicit return types on all functions. generateSitemaps returns a value inferred as { id: number }[] but lacks an annotation.

♻️ Proposed fix
-export async function generateSitemaps() {
+export async function generateSitemaps(): Promise<{ id: number }[]> {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` around lines 40 - 57, generateSitemaps currently lacks an
explicit return type; annotate the function signature to return Promise<{ id:
number }[]> (i.e. Promise of an array of objects with id:number) so the result
of using STATIC_PAGES_PER_LOCALE, URLS_PER_SITEMAP and the async calls
(getStore, resolveCountryLocales, fetchTotalCount) is properly typed; update the
declaration for function generateSitemaps to include this return type and ensure
any related imports/types are available.
🤖 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/app/sitemap.ts`:
- Around line 59-75: sitemap() currently calls fetchAllProducts() and
fetchAllTaxons() on every invocation which causes O(N×chunks) API calls when
generateSitemaps() produces multiple chunks; change the implementation so that
either (a) you memoize fetchAllProducts() and fetchAllTaxons() per process
invocation so repeated sitemap() calls reuse the already-fetched arrays (e.g., a
module-level cache accessed by sitemap), or (b) modify sitemap/generateSitemaps
to pass a chunk index/limit/offset into new paginated fetch functions (e.g.,
fetchProductsPage(offset, limit) / fetchTaxonsPage(offset, limit)) and fetch
only the slice needed for that chunk when sitemapCount > 1; update references to
sitemap, fetchAllProducts, fetchAllTaxons, and generateSitemaps accordingly and
ensure the chosen approach preserves countryLocales behavior and error handling.
- Around line 65-68: The sitemap's baseUrl calculation (using store.url ||
process.env.NEXT_PUBLIC_SITE_URL) can be empty, producing relative <loc> values;
update the top of the sitemap generation (where baseUrl is computed) to validate
that baseUrl is non-empty after trimming the trailing slash and if empty either
throw an Error or log an explicit error and return early so no sitemap is
emitted; reference the baseUrl variable and the store.url / NEXT_PUBLIC_SITE_URL
sources and ensure the guard prevents continuing to construct basePath or any
absolute URLs when baseUrl === "".

---

Nitpick comments:
In `@src/app/sitemap.ts`:
- Around line 199-205: The mapping over selectedIsos currently emits entries for
unknown ISO codes by using storeDefaultLocale when found is undefined; update
the logic in the selectedIsos.map block (the code that computes found from
allCountries and returns { country: iso, locale: found?.default_locale ||
storeDefaultLocale }) to instead filter out or skip ISOs that have no matching
entry in allCountries and emit a warning (e.g., via console.warn or the repo
logger) listing the unknown ISO(s) so misconfigurations are visible; if you
still want to keep a fallback behavior make it explicit (e.g., push a diagnostic
log when using storeDefaultLocale for an unmatched ISO) but prefer dropping
unmatched entries to avoid generating sitemap URLs for countries the store
doesn’t serve.
- Around line 40-57: generateSitemaps currently lacks an explicit return type;
annotate the function signature to return Promise<{ id: number }[]> (i.e.
Promise of an array of objects with id:number) so the result of using
STATIC_PAGES_PER_LOCALE, URLS_PER_SITEMAP and the async calls (getStore,
resolveCountryLocales, fetchTotalCount) is properly typed; update the
declaration for function generateSitemaps to include this return type and ensure
any related imports/types are available.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/app/sitemap.ts (1)

182-192: IIFE-in-ternary for the warning + fallback is unconventional — prefer a simple if/else.

The pattern works, but an immediately-invoked arrow function inside a ternary purely for a side-effect is harder to scan than a straightforward if/else.

♻️ Proposed refactor
-  const mode: SitemapLocaleMode = VALID_LOCALE_MODES.includes(
-    rawMode as SitemapLocaleMode,
-  )
-    ? (rawMode as SitemapLocaleMode)
-    : (() => {
-        console.warn(
-          `Invalid SITEMAP_LOCALE_MODE "${rawMode}". Expected one of: ${VALID_LOCALE_MODES.join(", ")}. Falling back to "default".`,
-        );
-        return "default" as const;
-      })();
+  if (!VALID_LOCALE_MODES.includes(rawMode as SitemapLocaleMode)) {
+    console.warn(
+      `Invalid SITEMAP_LOCALE_MODE "${rawMode}". Expected one of: ${VALID_LOCALE_MODES.join(", ")}. Falling back to "default".`,
+    );
+  }
+  const mode: SitemapLocaleMode = VALID_LOCALE_MODES.includes(rawMode as SitemapLocaleMode)
+    ? (rawMode as SitemapLocaleMode)
+    : "default";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` around lines 182 - 192, The current ternary that uses an
immediately-invoked arrow function to warn and return a fallback makes the logic
hard to read; refactor the assignment of mode by computing rawMode as before,
then use a clear if/else: if VALID_LOCALE_MODES.includes(rawMode as
SitemapLocaleMode) set mode = rawMode as SitemapLocaleMode, else call
console.warn with the same message and set mode = "default" as const; reference
rawMode, mode, SitemapLocaleMode, and VALID_LOCALE_MODES when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/app/sitemap.ts`:
- Around line 148-151: The image URL filter currently only excludes
null/undefined (product.images.map(...).filter((url): url is string => url !=
null)) but allows empty strings, producing invalid <image:loc> entries; update
the filter to also reject empty/whitespace-only strings (e.g., check url &&
url.trim() !== '' or use Boolean(url) after trimming) so only non-empty,
non-null image URLs are kept before building the sitemap.

---

Nitpick comments:
In `@src/app/sitemap.ts`:
- Around line 182-192: The current ternary that uses an immediately-invoked
arrow function to warn and return a fallback makes the logic hard to read;
refactor the assignment of mode by computing rawMode as before, then use a clear
if/else: if VALID_LOCALE_MODES.includes(rawMode as SitemapLocaleMode) set mode =
rawMode as SitemapLocaleMode, else call console.warn with the same message and
set mode = "default" as const; reference rawMode, mode, SitemapLocaleMode, and
VALID_LOCALE_MODES when making the change.

Cichorek and others added 3 commits February 20, 2026 17:34
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (4)
src/app/sitemap.ts (4)

179-192: IIFE for the warning fallback works but is a bit unusual.

The inline IIFE to log a warning and return the fallback value is functional but slightly harder to parse at a glance. A simple if/else or ternary with a separate warning line would be more readable. Not blocking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` around lines 179 - 192, The resolveCountryLocales
function uses an inline IIFE to warn and fall back when SITEMAP_LOCALE_MODE is
invalid; replace that IIFE with a clearer conditional: compute rawMode as
before, then if VALID_LOCALE_MODES.includes(rawMode as SitemapLocaleMode) set
mode = rawMode as SitemapLocaleMode else log the warning message (including
rawMode and VALID_LOCALE_MODES.join(", ")) and set mode = "default". Keep
references to VALID_LOCALE_MODES, SITEMAP_LOCALE_MODE and resolveCountryLocales
so the change is localized and behavior remains identical.

268-300: No upper-bound guard on pagination loops.

If the API returns an unexpectedly high or inconsistent meta.pages value, these loops could issue a very large number of requests. Consider adding a safety cap (e.g., page <= totalPages && page <= 1000) to prevent runaway loops during build.

Example guard
+const MAX_PAGES = 1000;
+
 async function fetchAllProducts(): Promise<StoreProduct[]> {
   const allProducts: StoreProduct[] = [];
   let page = 1;
   let totalPages = 1;
 
   do {
     const response = await getProducts({
       page,
       per_page: 100,
       includes: "images",
     });
     allProducts.push(...response.data);
     totalPages = response.meta.pages;
     page++;
-  } while (page <= totalPages);
+  } while (page <= totalPages && page <= MAX_PAGES);

   return allProducts;
 }

Apply the same to fetchAllTaxons.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` around lines 268 - 300, Both pagination loops in
fetchAllProducts and fetchAllTaxons lack an upper-bound guard and may run
runaway if meta.pages is huge; add a safety cap by introducing a maxPages
constant (e.g., const MAX_PAGES = 1000) and change the loop condition to stop
when page > Math.min(totalPages, MAX_PAGES) or use page <= totalPages && page <=
MAX_PAGES so the functions (fetchAllProducts, fetchAllTaxons) will abort after
the cap is reached while still iterating up to the real totalPages when it is
reasonable.

62-78: resolveCountryLocales is not cached — called separately in generateSitemaps and each sitemap chunk.

For "all" or "selected" modes, each invocation of resolveCountryLocales triggers a getCountries() API call. With N chunks, that's N+1 total calls for identical data. This is less impactful than the product/taxon calls (already cached), but applying the same caching pattern here would be consistent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` around lines 62 - 78, generateSitemaps calls
resolveCountryLocales which also gets called per sitemap chunk, causing repeated
getCountries() API calls; to fix, compute resolveCountryLocales once and reuse
it for all sitemap generation (either memoize resolveCountryLocales at module
scope or have generateSitemaps return/pass the resolved countryLocales into the
sitemap chunk generator), update references where sitemap chunks call
resolveCountryLocales so they use the cached countryLocales instead; target
symbols: resolveCountryLocales, generateSitemaps, and the sitemap chunk
generator function (e.g., sitemap) to ensure a single getCountries() call is
used.

266-266: VALID_LOCALE_MODES is declared after its usage site.

While this works at runtime (the function referencing it isn't called until after module initialization), placing the constant before resolveCountryLocales (e.g., near the other constants on lines 28-29) would improve readability and co-locate related definitions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/sitemap.ts` at line 266, Move the VALID_LOCALE_MODES constant so it
is declared before the resolveCountryLocales function and co-located with the
other top-level constants; update placement so VALID_LOCALE_MODES:
SitemapLocaleMode[] = ["default","selected","all"] appears alongside the other
constant definitions near the top of the module, ensuring resolveCountryLocales
references an already-declared symbol for improved readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Line 87: Table row for `GTM_ID` is missing the trailing pipe which causes
MD055; edit the table row containing "`GTM_ID | Google Tag Manager container ID
(e.g. `GTM-XXXXXXX`) | _(disabled)_`" and append a final `|` so it matches the
pipe-style of the other rows.

In `@src/app/sitemap.ts`:
- Around line 36-51: The code caches raw promises (cachedProducts/cachedTaxons)
which permanently poisons the cache if fetchAllProducts() or fetchAllTaxons()
rejects; update getCachedProducts and getCachedTaxons to attach a .catch(...)
handler that clears the corresponding cache variable (reset cachedProducts or
cachedTaxons to null) on rejection and re-throws the error so transient failures
can be retried on subsequent calls; keep using
fetchAllProducts()/fetchAllTaxons() but store the wrapped promise so successful
results still cache.
- Around line 146-153: Update the sitemap image mapping to use the correct Spree
5.x field: replace references to img.large_url with img.original_url in the
product image mapping (the block that builds images inside the sitemap
generation in src/app/sitemap.ts), ensuring the .map and subsequent .filter
operate on img.original_url so product images are included in the sitemap.

---

Duplicate comments:
In `@README.md`:
- Around line 98-104: The README contains a blank line inside the blockquoted
"Sitemap locale modes" section that triggers MD028; remove the empty line
between the two consecutive blockquote paragraphs so all lines beginning with
">" are contiguous (i.e., collapse the blank `>` line at the end of the list so
the sentence "Each country resolves its locale..." stays part of the same
blockquote), ensuring the blockquote formatting is consistent and MD028 is
resolved.

---

Nitpick comments:
In `@src/app/sitemap.ts`:
- Around line 179-192: The resolveCountryLocales function uses an inline IIFE to
warn and fall back when SITEMAP_LOCALE_MODE is invalid; replace that IIFE with a
clearer conditional: compute rawMode as before, then if
VALID_LOCALE_MODES.includes(rawMode as SitemapLocaleMode) set mode = rawMode as
SitemapLocaleMode else log the warning message (including rawMode and
VALID_LOCALE_MODES.join(", ")) and set mode = "default". Keep references to
VALID_LOCALE_MODES, SITEMAP_LOCALE_MODE and resolveCountryLocales so the change
is localized and behavior remains identical.
- Around line 268-300: Both pagination loops in fetchAllProducts and
fetchAllTaxons lack an upper-bound guard and may run runaway if meta.pages is
huge; add a safety cap by introducing a maxPages constant (e.g., const MAX_PAGES
= 1000) and change the loop condition to stop when page > Math.min(totalPages,
MAX_PAGES) or use page <= totalPages && page <= MAX_PAGES so the functions
(fetchAllProducts, fetchAllTaxons) will abort after the cap is reached while
still iterating up to the real totalPages when it is reasonable.
- Around line 62-78: generateSitemaps calls resolveCountryLocales which also
gets called per sitemap chunk, causing repeated getCountries() API calls; to
fix, compute resolveCountryLocales once and reuse it for all sitemap generation
(either memoize resolveCountryLocales at module scope or have generateSitemaps
return/pass the resolved countryLocales into the sitemap chunk generator),
update references where sitemap chunks call resolveCountryLocales so they use
the cached countryLocales instead; target symbols: resolveCountryLocales,
generateSitemaps, and the sitemap chunk generator function (e.g., sitemap) to
ensure a single getCountries() call is used.
- Line 266: Move the VALID_LOCALE_MODES constant so it is declared before the
resolveCountryLocales function and co-located with the other top-level
constants; update placement so VALID_LOCALE_MODES: SitemapLocaleMode[] =
["default","selected","all"] appears alongside the other constant definitions
near the top of the module, ensuring resolveCountryLocales references an
already-declared symbol for improved readability.

Cichorek and others added 3 commits February 20, 2026 18:43
- Fix promise cache poisoning: clear cached promise on rejection so
  transient failures can be retried on subsequent calls
- Use original_url instead of large_url for sitemap product images
  (correct Spree 5.x field, consistent with MediaGallery)
- Memoize resolveCountryLocales to avoid repeated getCountries() API calls
- Add MAX_PAGES (1000) safety cap to pagination loops
- Replace IIFE with clearer conditional for locale mode validation
- Move VALID_LOCALE_MODES constant to top of module with other constants
- Fix GTM_ID table row missing trailing pipe in README
- Fix MD028: merge adjacent blockquotes into single contiguous block

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

Sitemap

2 participants