feat(shop): headless Shopify storefront at /shop#831
Merged
tannerlinsley merged 3 commits intomainfrom Apr 15, 2026
Merged
Conversation
Introduces a Shopify-backed commerce experience served directly by tanstack.com, replacing the outbound Cotton Bureau link on /merch. Routes - /shop — product grid (cached via Netlify edge, 5m TTL + swr) - /shop/products/$handle — PDP with variant selection - /shop/cart — cart with quantity controls, checkout redirect Architecture - Shopify Storefront API (2026-01) via @shopify/hydrogen-react types - Private token never leaves the server; cart ID in httpOnly cookie - All cart reads/mutations go through createServerFn (getCart, addToCart, updateCartLine, removeCartLine) - /shop parent loader prefetches cart + collections into RQ cache so every child page renders with real data on the first frame — no hydration gap, no skeletons - Optimistic updates via React Query onMutate/rollback - Shop layout anchors a collapsible sidebar to the viewport's left rail; Navbar Title flips the main site nav into flyout mode on any /shop/* route (same pattern as library landings) Also rolled in - createFileRoute literalization across library landings — the variable pattern was crashing router-generator's crawl and producing "Crawling result not available" at dev startup. Inlining the path string on both the createFileRoute and createLibraryLandingPage calls fixes it permanently. - check-name.ts: drop the "as any" cast on createFileRoute now that the generator can parse the literal directly. - .gitignore: match .env.* so future env variants (including .bak files from accidental backups) never leak into commits. - env merge: .env now folded into .env.local as a single source, with the dev script pointing at just .env.local.
✅ Deploy Preview for tanstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Shopify-backed shop (routes, product pages, cart UI), server Shopify fetch/utilities/GraphQL queries, TanStack server functions for shop/cart, React Query cart hooks and navbar cart UI, shop layout components, route-tree additions, env schema update for a Shopify token, docs and dependency changes, and many small route inlinings. Changes
Sequence DiagramsequenceDiagram
participant User
participant Client as Client (React)
participant Cache as ReactQuery Cache
participant Server as App Server (serverFns)
participant Shopify as Shopify API
Note over User,Shopify: Add-to-cart flow
User->>Client: Click "Add to cart"
Client->>Cache: Optimistic update (totalQuantity, line)
Client->>Server: POST addToCart(variantId, quantity)
Server->>Shopify: CartCreate / CartLinesAdd mutation
Shopify-->>Server: CartDetail
Server-->>Cache: Write cart response
Cache-->>Client: Re-render with updated cart
Client-->>User: NavbarCartButton shows new count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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 (1)
src/components/RedirectVersionBanner.tsx (1)
32-44:⚠️ Potential issue | 🟠 MajorReplace
as nevercasts with proper type handling to maintain router safety.The
as nevercasts ontoandparamsat lines 32-33 and 43-44 bypass TanStack Router's type validation entirely. While both usages are constrained to versioned routes (confirmed: only used insrc/routes/-library-landing.tsxandsrc/routes/$libraryId/$version.tsx), these casts prevent TypeScript from detecting future breaking changes. If the route structure or version param handling evolves, these assertions would silently allow invalid navigation at runtime.Consider using a more targeted type assertion or refactoring the navigation pattern to preserve compile-time route safety instead of opting out of it entirely with
as never.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RedirectVersionBanner.tsx` around lines 32 - 44, The code in RedirectVersionBanner is bypassing TanStack Router's type checks by casting both Link props to "as never"; replace these casts by passing properly typed route info or generating the target href with the router utilities so TypeScript can verify params. Specifically, in RedirectVersionBanner remove the "as never" casts on Link and Button props and instead either: 1) use the typed match object (activeMatch) to derive a correctly typed destination (e.g., use the router's createHref/generatePath helper or a typed Link generic) so params={{ version: 'latest' }} is checked, or 2) pass activeMatch.params (or a shallow-clone with version updated) as the params value so the version param is typed by the route; ensure changes touch the Link and Button usages that reference activeMatch.fullPath and params version so route safety is preserved.
🧹 Nitpick comments (7)
.gitignore (1)
10-10: Consider removing redundant.envpatterns.The new
.env.*wildcard pattern now covers the entries at lines 24 (.env.sentry-build-plugin) and 27 (.env.local). Removing those redundant entries would improve maintainability and clarity.🧹 Proposed cleanup to remove redundant patterns
Remove the redundant entries that are already matched by
.env.*:.env .env.* .vercel .output .vinxi .tanstack-start/build .nitro/* .netlify/* /build/ /api/ /server/build /public/build # Sentry Config File -.env.sentry-build-plugin dist .vscode/ -.env.local # Content Collections generated files .content-collections🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 10, Remove the redundant explicit .env entries now covered by the new .env.* pattern: delete the individual `.env.sentry-build-plugin` and `.env.local` lines from the .gitignore so only `.env.*` remains; ensure no other unique `.env` exceptions are needed and run a quick grep to confirm `.env.*` covers the removed patterns.src/components/application-builder/shared.ts (1)
156-166: Unify migration-intent checks to avoid behavior drift.
getStarterMigrationGuideUrlnow handles Next.js + Remix, but migration-specific prompt augmentation still gates on Next.js only. Consider introducing a sharedisMigrationInputhelper and reusing it in both paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/application-builder/shared.ts` around lines 156 - 166, Introduce a shared predicate isMigrationInput(input: string) that returns true for both isNextJsMigrationInput(input) and isRemixMigrationInput(input), then replace the ad-hoc Next.js-only checks elsewhere (the migration-specific prompt augmentation logic) to use isMigrationInput instead of isNextJsMigrationInput; also update getStarterMigrationGuideUrl to keep using isNextJsMigrationInput and isRemixMigrationInput for guide selection while any other migration-intent decision points (e.g., prompt augmentation) should call isMigrationInput to ensure consistent behavior.src/components/NavbarCartButton.tsx (1)
24-24: Pluralize the cart aria-label for singular counts.At Line 24, screen readers will get
Cart (1 items). Use singular/plural grammar for better accessibility polish.💡 Suggested tweak
- aria-label={totalQuantity > 0 ? `Cart (${totalQuantity} items)` : 'Cart'} + aria-label={ + totalQuantity > 0 + ? `Cart (${totalQuantity} ${totalQuantity === 1 ? 'item' : 'items'})` + : 'Cart' + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/NavbarCartButton.tsx` at line 24, The aria-label in the NavbarCartButton currently renders "Cart (1 items)" when totalQuantity is 1; update the aria-label expression in the NavbarCartButton component to choose the singular "item" when totalQuantity === 1 and "items" otherwise (e.g., use totalQuantity > 0 ? `Cart (${totalQuantity} ${totalQuantity === 1 ? 'item' : 'items'})` : 'Cart'), referencing the totalQuantity prop and the aria-label attribute to apply the conditional pluralization.docs/performance-plan-home-library-docs.md (1)
9-14: Use stable identifiers for the baseline.These hashed asset names will be obsolete after the next build, so the plan won’t stay useful as living documentation. Prefer route/chunk labels plus the measurement date, or link the generated size report that produced these numbers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/performance-plan-home-library-docs.md` around lines 9 - 14, The documented asset sizes use ephemeral hashed filenames (e.g., index-Bq0A5jmY.js, app-shell-BikUtTEO.js, SearchModal-Bl-tUxqr.js, DocsLayout-Bga1-HA9.js, MarkdownContent-ia2V1dk8.js, app-CBMELhsb.css) which will change after each build; replace those hashed identifiers with stable route/chunk labels (e.g., "Homepage route chunk", "Shared shell chunk", "Search modal chunk", "Docs shell chunk", "Markdown chrome chunk", "Global CSS") and add the measurement date or a permalink to the generated size report that produced the numbers so the baseline remains meaningful. Ensure the text in docs/performance-plan-home-library-docs.md references the stable labels and includes either a date or a link to the source size-report.docs/proposals/npm-watchlists-and-weekly-digests.md (1)
138-197: Keep the model definition in one proposal.These exact
NpmTrackedEntity/NpmRollup/NpmWatchlistshapes are also defined indocs/proposals/npm-watchlist-registry-draft.md. Duplicating the canonical model in two docs will drift quickly once the registry evolves. I’d keep the full type block in one place and summarize/link to it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/proposals/npm-watchlists-and-weekly-digests.md` around lines 138 - 197, The docs duplicate the canonical type definitions for NpmTrackedEntity, NpmRollup, and NpmWatchlist; remove the full type block from this proposal and replace it with a short summary and a link/reference to the single source-of-truth (the definitions in npm-watchlist-registry-draft.md), while keeping the type names NpmTrackedEntity, NpmRollup, and NpmWatchlist mentioned so readers can find the canonical shapes; ensure any examples here only reference the canonical types rather than re-defining them.src/utils/shopify-queries.ts (1)
73-80: Consider simplifying the redundant null union.
(Pick<StorefrontImage, ...> | null) | nullcan be simplified toPick<StorefrontImage, ...> | null.♻️ Suggested fix
export type ProductListItem = Pick<Product, 'id' | 'handle' | 'title'> & { - featuredImage: - | (Pick<StorefrontImage, 'url' | 'altText' | 'width' | 'height'> | null) - | null + featuredImage: Pick<StorefrontImage, 'url' | 'altText' | 'width' | 'height'> | null priceRange: { minVariantPrice: Pick<MoneyV2, 'amount' | 'currencyCode'> } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/shopify-queries.ts` around lines 73 - 80, The ProductListItem type's featuredImage currently has a redundant nested null union; update the featuredImage type on ProductListItem to use a single null union by changing its type from "(Pick<StorefrontImage, 'url' | 'altText' | 'width' | 'height'> | null) | null" to "Pick<StorefrontImage, 'url' | 'altText' | 'width' | 'height'> | null", keeping the same Pick fields (StorefrontImage) and preserving priceRange and other fields.src/utils/shop.functions.ts (1)
222-245: Document thatquantity: 0removes the line.The validator allows
v.minValue(0), which per Shopify's API will remove the line entirely. This behavior might surprise callers who expect a dedicatedremoveCartLinefor removal. Consider adding a brief comment or usingremoveCartLineinternally when quantity is 0.📝 Suggested documentation
export const updateCartLine = createServerFn({ method: 'POST' }) .inputValidator( v.object({ lineId: v.string(), + // Note: quantity=0 removes the line (Shopify behavior). Use removeCartLine for explicit removal. quantity: v.pipe(v.number(), v.integer(), v.minValue(0)), }), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/shop.functions.ts` around lines 222 - 245, The updateCartLine handler currently allows quantity 0 via the inputValidator (v.minValue(0)) which, per Shopify API, removes the line item — this should be made explicit or handled differently; update the updateCartLine function to either: a) add a clear inline comment above the inputValidator and handler stating "quantity: 0 will remove the line item per Shopify API" so callers are aware, or b) detect data.quantity === 0 inside the handler and call the existing removeCartLine flow instead of CART_LINES_UPDATE_MUTATION (use getCookie(CART_COOKIE_NAME) and throwIfUserErrors similarly), then return the resulting cart; ensure any added path uses the same error handling (throwIfUserErrors) and returns CartDetail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 47: Move the package "@shopify/hydrogen-react" from dependencies to
devDependencies in package.json because all uses are type-only; update
package.json so "@shopify/hydrogen-react": "^2026.4.0" appears under
devDependencies instead of dependencies, and run npm/yarn install to update
lockfile; confirm in src/utils/shopify-queries.ts that imports are "import type"
to justify the change.
In `@src/components/shop/ShopLayout.tsx`:
- Around line 247-252: The sidebar Link in ShopLayout.tsx is casting routes to
any to generate links to /shop/collections/$handle despite no route handler
existing; either add a proper route and page component for the dynamic
collection path (e.g., create a CollectionDetail component and register a route
like '/shop/collections/:handle' in your router/route config so Link's to/params
point to a real route), or stop rendering collection links in ShopLayout (filter
out collection entries before mapping to Link and remove the to={to as
any}/params={params as any} workaround) so navigation doesn't point to a
non-existent route.
In `@src/routes/shop.cart.tsx`:
- Around line 41-53: The <dl> in the cart UI contains a stray <p> ("Shipping and
taxes calculated at checkout.") which is invalid inside a description list; move
that note outside the <dl> so the <dl className="flex flex-col gap-2 text-sm">
only contains term/value groups (the two <div className="flex justify-between">
and the total row <div className="flex justify-between border-t ...">). Render
the paragraph immediately after the closing </dl> (preserving its className and
text) to ensure correct semantics and consistent screen-reader announcements.
In `@src/routes/shop.products`.$handle.tsx:
- Around line 109-114: The product.descriptionHtml is rendered with
dangerouslySetInnerHTML without sanitization; sanitize it first (e.g., import
sanitize-html or DOMPurify) and use the sanitized string when setting
dangerouslySetInnerHTML. Concretely, create a sanitized variable (e.g., const
safeDescription = sanitizeHtml(product.descriptionHtml, { allowedTags: [...],
allowedAttributes: {...} }) or DOMPurify.sanitize(product.descriptionHtml)),
then replace dangerouslySetInnerHTML={{ __html: product.descriptionHtml }} with
dangerouslySetInnerHTML={{ __html: safeDescription }} in the component rendering
the product (the JSX block that references product.descriptionHtml).
- Around line 34-36: The selected state is only initialized once and must be
reset when a new product is loaded; add an effect that watches the product
identity (e.g., product.id or product.handle) and calls setSelected with
Object.fromEntries(product.options.map(o => [o.name, o.values[0]!])) so the
selection reflects the current product before calling findMatchingVariant;
update the file to include this useEffect alongside the existing
selected/setSelected state initialization.
In `@src/server/shopify/fetch.ts`:
- Around line 55-80: The current fetch flow can hang or surface non-Shopify
exceptions; wrap the network/parse steps around a timeout and try/catch and
rethrow as ShopifyError with underlying error details: when performing the fetch
to `https://${domain}/api/${version}/graphql.json` (the fetch call) use an
AbortController with a sensible timeout and abort the request on timeout, and
surround both the fetch and the subsequent `await response.json()` (parsed as
`ShopifyResponse<TData>`) in a try/catch that converts any thrown error into a
`ShopifyError` (include the original error message/stack as context), leaving
the existing response.ok and json.errors checks intact so upstream failures stay
normalized.
In `@src/utils/shop.functions.ts`:
- Around line 108-120: Change getProduct to use method: 'GET' so responses can
be cached at the edge; update the handler to read the handle from the request
query instead of the POST body and validate it there. Specifically, modify
createServerFn({ method: 'POST' }) to createServerFn({ method: 'GET' }), adjust
the handler signature to accept and validate the request query (instead of
destructuring data) so you pass variables: { handle: query.handle } into
shopifyServerFetch, keep calling setBrowseCacheHeaders(), and preserve the same
return of result.product; ensure the input validation for the handle is applied
to the query parameter (e.g., validate query.handle with v.string()).
In `@src/utils/shopify-queries.ts`:
- Around line 190-197: Update the misleading header comment in the Cart section
of src/utils/shopify-queries.ts to reflect that cart operations are performed
server-side via shop.functions.ts (using the private Storefront token) and that
the cart ID is stored in an httpOnly cookie rather than localStorage; locate the
block around the "Cart — queries + mutations" comment and replace the
browser/localStorage wording with a concise statement that cart requests go
through the server functions (e.g., shop.functions.ts) and cookies are used for
cart ID storage.
---
Outside diff comments:
In `@src/components/RedirectVersionBanner.tsx`:
- Around line 32-44: The code in RedirectVersionBanner is bypassing TanStack
Router's type checks by casting both Link props to "as never"; replace these
casts by passing properly typed route info or generating the target href with
the router utilities so TypeScript can verify params. Specifically, in
RedirectVersionBanner remove the "as never" casts on Link and Button props and
instead either: 1) use the typed match object (activeMatch) to derive a
correctly typed destination (e.g., use the router's createHref/generatePath
helper or a typed Link generic) so params={{ version: 'latest' }} is checked, or
2) pass activeMatch.params (or a shallow-clone with version updated) as the
params value so the version param is typed by the route; ensure changes touch
the Link and Button usages that reference activeMatch.fullPath and params
version so route safety is preserved.
---
Nitpick comments:
In @.gitignore:
- Line 10: Remove the redundant explicit .env entries now covered by the new
.env.* pattern: delete the individual `.env.sentry-build-plugin` and
`.env.local` lines from the .gitignore so only `.env.*` remains; ensure no other
unique `.env` exceptions are needed and run a quick grep to confirm `.env.*`
covers the removed patterns.
In `@docs/performance-plan-home-library-docs.md`:
- Around line 9-14: The documented asset sizes use ephemeral hashed filenames
(e.g., index-Bq0A5jmY.js, app-shell-BikUtTEO.js, SearchModal-Bl-tUxqr.js,
DocsLayout-Bga1-HA9.js, MarkdownContent-ia2V1dk8.js, app-CBMELhsb.css) which
will change after each build; replace those hashed identifiers with stable
route/chunk labels (e.g., "Homepage route chunk", "Shared shell chunk", "Search
modal chunk", "Docs shell chunk", "Markdown chrome chunk", "Global CSS") and add
the measurement date or a permalink to the generated size report that produced
the numbers so the baseline remains meaningful. Ensure the text in
docs/performance-plan-home-library-docs.md references the stable labels and
includes either a date or a link to the source size-report.
In `@docs/proposals/npm-watchlists-and-weekly-digests.md`:
- Around line 138-197: The docs duplicate the canonical type definitions for
NpmTrackedEntity, NpmRollup, and NpmWatchlist; remove the full type block from
this proposal and replace it with a short summary and a link/reference to the
single source-of-truth (the definitions in npm-watchlist-registry-draft.md),
while keeping the type names NpmTrackedEntity, NpmRollup, and NpmWatchlist
mentioned so readers can find the canonical shapes; ensure any examples here
only reference the canonical types rather than re-defining them.
In `@src/components/application-builder/shared.ts`:
- Around line 156-166: Introduce a shared predicate isMigrationInput(input:
string) that returns true for both isNextJsMigrationInput(input) and
isRemixMigrationInput(input), then replace the ad-hoc Next.js-only checks
elsewhere (the migration-specific prompt augmentation logic) to use
isMigrationInput instead of isNextJsMigrationInput; also update
getStarterMigrationGuideUrl to keep using isNextJsMigrationInput and
isRemixMigrationInput for guide selection while any other migration-intent
decision points (e.g., prompt augmentation) should call isMigrationInput to
ensure consistent behavior.
In `@src/components/NavbarCartButton.tsx`:
- Line 24: The aria-label in the NavbarCartButton currently renders "Cart (1
items)" when totalQuantity is 1; update the aria-label expression in the
NavbarCartButton component to choose the singular "item" when totalQuantity ===
1 and "items" otherwise (e.g., use totalQuantity > 0 ? `Cart (${totalQuantity}
${totalQuantity === 1 ? 'item' : 'items'})` : 'Cart'), referencing the
totalQuantity prop and the aria-label attribute to apply the conditional
pluralization.
In `@src/utils/shop.functions.ts`:
- Around line 222-245: The updateCartLine handler currently allows quantity 0
via the inputValidator (v.minValue(0)) which, per Shopify API, removes the line
item — this should be made explicit or handled differently; update the
updateCartLine function to either: a) add a clear inline comment above the
inputValidator and handler stating "quantity: 0 will remove the line item per
Shopify API" so callers are aware, or b) detect data.quantity === 0 inside the
handler and call the existing removeCartLine flow instead of
CART_LINES_UPDATE_MUTATION (use getCookie(CART_COOKIE_NAME) and
throwIfUserErrors similarly), then return the resulting cart; ensure any added
path uses the same error handling (throwIfUserErrors) and returns CartDetail.
In `@src/utils/shopify-queries.ts`:
- Around line 73-80: The ProductListItem type's featuredImage currently has a
redundant nested null union; update the featuredImage type on ProductListItem to
use a single null union by changing its type from "(Pick<StorefrontImage, 'url'
| 'altText' | 'width' | 'height'> | null) | null" to "Pick<StorefrontImage,
'url' | 'altText' | 'width' | 'height'> | null", keeping the same Pick fields
(StorefrontImage) and preserving priceRange and other fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04addb31-7ba4-48e7-8d04-5f43da8868f0
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
.gitignoredocs/performance-plan-home-library-docs.mddocs/proposals/npm-watchlist-registry-draft.mddocs/proposals/npm-watchlists-and-weekly-digests.mdpackage.jsonsrc/components/Navbar.tsxsrc/components/NavbarCartButton.tsxsrc/components/RedirectVersionBanner.tsxsrc/components/application-builder/shared.tssrc/components/shop/ShopLayout.tsxsrc/hooks/useCart.tssrc/routeTree.gen.tssrc/routes/ai.$version.index.tsxsrc/routes/api/builder/deploy/check-name.tssrc/routes/cli.$version.index.tsxsrc/routes/config.$version.index.tsxsrc/routes/db.$version.index.tsxsrc/routes/devtools.$version.index.tsxsrc/routes/form.$version.index.tsxsrc/routes/hotkeys.$version.index.tsxsrc/routes/intent.$version.index.tsxsrc/routes/pacer.$version.index.tsxsrc/routes/query.$version.index.tsxsrc/routes/ranger.$version.index.tsxsrc/routes/router.$version.index.tsxsrc/routes/shop.cart.tsxsrc/routes/shop.index.tsxsrc/routes/shop.products.$handle.tsxsrc/routes/shop.tsxsrc/routes/start.$version.index.tsxsrc/routes/store.$version.index.tsxsrc/routes/table.$version.index.tsxsrc/routes/virtual.$version.index.tsxsrc/server/shopify/fetch.tssrc/utils/application-starter.server.tssrc/utils/env.tssrc/utils/shop.functions.tssrc/utils/shopify-format.tssrc/utils/shopify-queries.ts
| "@sentry/browser": "^10.47.0", | ||
| "@sentry/node": "^10.47.0", | ||
| "@sentry/tanstackstart-react": "^10.47.0", | ||
| "@shopify/hydrogen-react": "^2026.4.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find every usage of `@shopify/hydrogen-react` and show context
rg -n -C2 --type=ts --type=tsx --type=js --type=jsx "['\"]@shopify/hydrogen-react['\"]"
# Quick check for type-only imports
rg -n --type=ts --type=tsx "import type .*from ['\"]@shopify/hydrogen-react['\"]"Repository: TanStack/tanstack.com
Length of output: 92
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all imports of `@shopify/hydrogen-react` in TypeScript/JavaScript files
# Using rg without problematic type filters
rg -n -C2 "@shopify/hydrogen-react" --glob="*.ts" --glob="*.tsx" --glob="*.js" --glob="*.jsx"
echo "---"
echo "Checking for type-only imports:"
rg -n "import type.*@shopify/hydrogen-react" --glob="*.ts" --glob="*.tsx" --glob="*.js" --glob="*.jsx"Repository: TanStack/tanstack.com
Length of output: 782
🏁 Script executed:
#!/bin/bash
# Show the full import statement at the top of the file
head -20 src/utils/shopify-queries.ts
echo "---"
echo "Checking if ProductOption and ProductVariant are used at runtime:"
rg -n "ProductOption|ProductVariant" src/utils/shopify-queries.ts
echo "---"
echo "Checking for any other `@shopify` references in codebase:"
rg -n "@shopify" --glob="*.ts" --glob="*.tsx" --glob="*.js" --glob="*.jsx"Repository: TanStack/tanstack.com
Length of output: 1198
Move @shopify/hydrogen-react to devDependencies.
All imports from this package use import type syntax (lines 1-10 in src/utils/shopify-queries.ts), meaning they are type-only and not needed at runtime. This reduces production install surface with no functional impact.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 47, Move the package "@shopify/hydrogen-react" from
dependencies to devDependencies in package.json because all uses are type-only;
update package.json so "@shopify/hydrogen-react": "^2026.4.0" appears under
devDependencies instead of dependencies, and run npm/yarn install to update
lockfile; confirm in src/utils/shopify-queries.ts that imports are "import type"
to justify the change.
Comment on lines
+247
to
+252
| <Link | ||
| // Cast to `any` is intentional: sidebar links are built from typed | ||
| // route literals, but collections come from a dynamic server list, | ||
| // so the union of safe paths is wider than inference can handle here. | ||
| to={to as any} | ||
| params={params as any} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf "Route definition for /shop/collections/\$handle:\n"
rg -n --fixed-strings "createFileRoute('/shop/collections/\$handle')" src || true
printf "\nSidebar link usages:\n"
rg -n --fixed-strings 'to="/shop/collections/$handle"' src || trueRepository: TanStack/tanstack.com
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Context around lines 247-252 in ShopLayout.tsx ==="
sed -n '240,260p' src/components/shop/ShopLayout.tsx
echo ""
echo "=== All createFileRoute definitions in src ==="
rg -n "createFileRoute\(" src | head -20
echo ""
echo "=== Context around line 180 (the sidebar link usage) ==="
sed -n '175,185p' src/components/shop/ShopLayout.tsxRepository: TanStack/tanstack.com
Length of output: 3205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for all /shop routes ==="
rg -n "createFileRoute.*shop" src/routes
echo ""
echo "=== Check if there's a /shop/ subdirectory in routes ==="
find src/routes -type d -name "*shop*" 2>/dev/null || echo "No /shop directory found"
echo ""
echo "=== Look for any reference to collections route definition ==="
rg -n "collections" src/routes
echo ""
echo "=== Full SidebarLink component definition to understand the type safety ==="
sed -n '230,280p' src/components/shop/ShopLayout.tsxRepository: TanStack/tanstack.com
Length of output: 2503
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check for any references to /shop/collections in the codebase ==="
rg -n "/shop/collections" src
echo ""
echo "=== Look for any dynamic route handling or collection detail views ==="
rg -n "collection" src/components src/routes --ignore-case | grep -i "detail\|view\|page\|component" || true
echo ""
echo "=== Check where collections are used ==="
rg -n "collections" src/components/shop/ShopLayout.tsx
echo ""
echo "=== Full ShopLayout export to understand component purpose ==="
sed -n '1,50p' src/components/shop/ShopLayout.tsxRepository: TanStack/tanstack.com
Length of output: 3714
Define the /shop/collections/$handle route or remove it from navigation.
The as any casts suppress type safety for a route that doesn't exist. Clicking collection sidebar links navigates to /shop/collections/$handle, but no route handler is defined for that path. Either add the route and implement collection detail pages, or remove these links from the sidebar since they lead nowhere. The developer comment acknowledges this as a workaround for dynamic server data, but intentional doesn't mean safe—the router can't protect against missing routes when they're cast to any.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/shop/ShopLayout.tsx` around lines 247 - 252, The sidebar Link
in ShopLayout.tsx is casting routes to any to generate links to
/shop/collections/$handle despite no route handler existing; either add a proper
route and page component for the dynamic collection path (e.g., create a
CollectionDetail component and register a route like '/shop/collections/:handle'
in your router/route config so Link's to/params point to a real route), or stop
rendering collection links in ShopLayout (filter out collection entries before
mapping to Link and remove the to={to as any}/params={params as any} workaround)
so navigation doesn't point to a non-existent route.
Comment on lines
+41
to
+53
| <dl className="flex flex-col gap-2 text-sm"> | ||
| <div className="flex justify-between"> | ||
| <dt className="text-gray-600 dark:text-gray-400">Subtotal</dt> | ||
| <dd>{formatMoney(subtotal.amount, subtotal.currencyCode)}</dd> | ||
| </div> | ||
| <div className="flex justify-between border-t border-gray-200 dark:border-gray-800 pt-2 font-semibold"> | ||
| <dt>Total</dt> | ||
| <dd>{formatMoney(total.amount, total.currencyCode)}</dd> | ||
| </div> | ||
| <p className="text-xs text-gray-500 dark:text-gray-500 mt-1"> | ||
| Shipping and taxes calculated at checkout. | ||
| </p> | ||
| </dl> |
There was a problem hiding this comment.
Move the checkout note outside the description list.
The standalone <p> inside this <dl> is invalid description-list markup and can be announced inconsistently by assistive tech. Keep only term/value groups inside the dl and render the note after it.
Proposed fix
<dl className="flex flex-col gap-2 text-sm">
<div className="flex justify-between">
<dt className="text-gray-600 dark:text-gray-400">Subtotal</dt>
<dd>{formatMoney(subtotal.amount, subtotal.currencyCode)}</dd>
</div>
<div className="flex justify-between border-t border-gray-200 dark:border-gray-800 pt-2 font-semibold">
<dt>Total</dt>
<dd>{formatMoney(total.amount, total.currencyCode)}</dd>
</div>
- <p className="text-xs text-gray-500 dark:text-gray-500 mt-1">
- Shipping and taxes calculated at checkout.
- </p>
</dl>
+ <p className="text-xs text-gray-500 dark:text-gray-500 mt-1">
+ Shipping and taxes calculated at checkout.
+ </p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <dl className="flex flex-col gap-2 text-sm"> | |
| <div className="flex justify-between"> | |
| <dt className="text-gray-600 dark:text-gray-400">Subtotal</dt> | |
| <dd>{formatMoney(subtotal.amount, subtotal.currencyCode)}</dd> | |
| </div> | |
| <div className="flex justify-between border-t border-gray-200 dark:border-gray-800 pt-2 font-semibold"> | |
| <dt>Total</dt> | |
| <dd>{formatMoney(total.amount, total.currencyCode)}</dd> | |
| </div> | |
| <p className="text-xs text-gray-500 dark:text-gray-500 mt-1"> | |
| Shipping and taxes calculated at checkout. | |
| </p> | |
| </dl> | |
| <dl className="flex flex-col gap-2 text-sm"> | |
| <div className="flex justify-between"> | |
| <dt className="text-gray-600 dark:text-gray-400">Subtotal</dt> | |
| <dd>{formatMoney(subtotal.amount, subtotal.currencyCode)}</dd> | |
| </div> | |
| <div className="flex justify-between border-t border-gray-200 dark:border-gray-800 pt-2 font-semibold"> | |
| <dt>Total</dt> | |
| <dd>{formatMoney(total.amount, total.currencyCode)}</dd> | |
| </div> | |
| </dl> | |
| <p className="text-xs text-gray-500 dark:text-gray-500 mt-1"> | |
| Shipping and taxes calculated at checkout. | |
| </p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/shop.cart.tsx` around lines 41 - 53, The <dl> in the cart UI
contains a stray <p> ("Shipping and taxes calculated at checkout.") which is
invalid inside a description list; move that note outside the <dl> so the <dl
className="flex flex-col gap-2 text-sm"> only contains term/value groups (the
two <div className="flex justify-between"> and the total row <div
className="flex justify-between border-t ...">). Render the paragraph
immediately after the closing </dl> (preserving its className and text) to
ensure correct semantics and consistent screen-reader announcements.
Comment on lines
+34
to
+36
| const [selected, setSelected] = React.useState<Record<string, string>>(() => | ||
| Object.fromEntries(product.options.map((o) => [o.name, o.values[0]!])), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does React rerun a useState initializer when props or loader data change without unmounting the component?
💡 Result:
No, React does not rerun a useState initializer when props or loader data change without unmounting the component. The initializer function (or initial value) is only called during the initial render (mount). On subsequent re-renders caused by prop changes, state updates, or parent re-renders, React preserves the existing state value and ignores the initializer argument. This applies whether the initializer is a plain value, a function depending on props, or loader data. If the initializer is a non-function value derived from props (e.g. useState(props.data)), that expression is evaluated on every render, but React discards the result and uses the preserved state. To avoid wasteful computations, use a function initializer: useState( => computeInitialValue(props)). Loader data in frameworks like React Router behaves similarly—it's external input causing re-renders, not remounts, so useState initializers are not rerun without unmounting.
Citations:
- 1: https://beta.reactjs.org/reference/react/useState
- 2: https://18.react.dev/reference/react/useState
- 3: https://react.dev/reference/react/useState
🏁 Script executed:
# First, let's read the full file to see the complete context
wc -l src/routes/shop.products.\$handle.tsxRepository: TanStack/tanstack.com
Length of output: 105
🏁 Script executed:
# Read the full component file
cat -n src/routes/shop.products.\$handle.tsxRepository: TanStack/tanstack.com
Length of output: 7671
🏁 Script executed:
# Check if there's already a useEffect that handles product changes
rg -A 5 "useEffect" src/routes/shop.products.\$handle.tsxRepository: TanStack/tanstack.com
Length of output: 272
🏁 Script executed:
# Look at the loader to understand how product data flows
rg -B 5 -A 20 "export.*loader|export const loader" src/routes/shop.products.\$handle.tsxRepository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Check if there's a key prop on the component or route that might force remount
rg -B 3 -A 3 "key=|<Route" src/routes/shop.products.\$handle.tsxRepository: TanStack/tanstack.com
Length of output: 775
🏁 Script executed:
# Check if there's a key prop or other route configuration that forces remount
rg -B 5 -A 5 "key=" src/routes/shop.products.\$handle.tsxRepository: TanStack/tanstack.com
Length of output: 908
🏁 Script executed:
# Look at React Router/TanStack Router configuration to understand remount behavior
find . -name "*.tsx" -o -name "*.ts" | xargs rg -l "createFileRoute" | head -3 | xargs -I {} sh -c 'echo "File: {}"; head -50 {}'Repository: TanStack/tanstack.com
Length of output: 2249
🏁 Script executed:
# Check if there are integration tests that navigate between products
rg -i "shop.*product|product.*navigate|navigate.*product" . --type tsx --type ts | grep -i test | head -10Repository: TanStack/tanstack.com
Length of output: 92
🏁 Script executed:
# Check TanStack Router documentation or test files to understand component remount behavior
find . -type f \( -name "*.md" -o -name "*.test.ts" -o -name "*.test.tsx" \) | xargs rg -l "react-router|createFileRoute" 2>/dev/null | head -5Repository: TanStack/tanstack.com
Length of output: 264
🏁 Script executed:
# Look for any documentation about route instance reuse
rg -i "route.*instance|component.*reuse|unmount|remount" . --type md | head -20Repository: TanStack/tanstack.com
Length of output: 1257
🏁 Script executed:
# Search for package.json to check TanStack Router version and documentation
cat package.json | grep -A 2 -B 2 "react-router"Repository: TanStack/tanstack.com
Length of output: 378
🏁 Script executed:
# Verify TanStack Router behavior by checking if there are examples of routes with params
rg -B 5 -A 15 "createFileRoute.*\\\$" src/routes/ --type tsx | head -50Repository: TanStack/tanstack.com
Length of output: 92
🏁 Script executed:
# Check if there are other routes with similar param-based navigation that have state reset logic
rg -B 3 -A 10 "useEffect.*product\|useEffect.*params\|useEffect.*useLoaderData" src/routes/ --type tsx | head -40Repository: TanStack/tanstack.com
Length of output: 92
🏁 Script executed:
# Look for other parameterized routes
rg -B 5 -A 15 'createFileRoute.*\$' src/routes/ | head -80Repository: TanStack/tanstack.com
Length of output: 5703
🏁 Script executed:
# Check if any components with loader data have useEffect that depends on loader data
rg -B 2 -A 8 'useLoaderData|useEffect' src/routes/ | grep -A 8 -B 2 'useEffect.*\[.*\]' | head -50Repository: TanStack/tanstack.com
Length of output: 47
🏁 Script executed:
# Look up TanStack Router behavior documentation in the blog
cat src/blog/tanstack-router-signal-graph.md | head -100Repository: TanStack/tanstack.com
Length of output: 5726
Reset selected options when the loaded product changes.
useState only snapshots product.options on the first mount. If this route instance is reused for another product (e.g., user navigates from /shop/products/product-a to /shop/products/product-b), the previous option selections persist, and findMatchingVariant will attempt to match against the wrong product's options, leading to incorrect variant selection, wrong prices, or cart errors.
Proposed fix
function ProductPage() {
const { product } = Route.useLoaderData()
const [selected, setSelected] = React.useState<Record<string, string>>(() =>
Object.fromEntries(product.options.map((o) => [o.name, o.values[0]!])),
)
+
+ React.useEffect(() => {
+ setSelected(
+ Object.fromEntries(product.options.map((o) => [o.name, o.values[0]!])),
+ )
+ }, [product.id, product.options])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/shop.products`.$handle.tsx around lines 34 - 36, The selected
state is only initialized once and must be reset when a new product is loaded;
add an effect that watches the product identity (e.g., product.id or
product.handle) and calls setSelected with
Object.fromEntries(product.options.map(o => [o.name, o.values[0]!])) so the
selection reflects the current product before calling findMatchingVariant;
update the file to include this useEffect alongside the existing
selected/setSelected state initialization.
Comment on lines
+109
to
+114
| {product.descriptionHtml ? ( | ||
| <div | ||
| className="prose dark:prose-invert max-w-none mt-4" | ||
| // eslint-disable-next-line react/no-danger | ||
| dangerouslySetInnerHTML={{ __html: product.descriptionHtml }} | ||
| /> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Shopify Storefront API Product.descriptionHtmlguarantee sanitization of scripts and unsafe attributes, or should storefronts sanitize it before rendering with ReactdangerouslySetInnerHTML?
💡 Result:
No, the Shopify Storefront API Product.descriptionHtml does not guarantee sanitization of scripts and unsafe attributes. Storefronts should sanitize it before rendering with React dangerouslySetInnerHTML to prevent XSS risks.
Citations:
- 1: https://community.shopify.com/t/is-body-html-sanitized-in-products-api-for-example/108888
- 2: https://www.shopify.com/hr/bugbounty/criteria
- 3: https://www.shopify.com/bugbounty/criteria
- 4: https://help.shopify.com/en/manual/shopify-admin/productivity-tools/rich-text-editor
Add HTML sanitization before rendering product.descriptionHtml.
Shopify Storefront API does not guarantee sanitization of Product.descriptionHtml. Using dangerouslySetInnerHTML without sanitization creates an XSS vulnerability if unsafe markup reaches the payload. Sanitize the HTML before rendering (e.g., using a library like sanitize-html or DOMPurify).
🧰 Tools
🪛 ast-grep (0.42.1)
[warning] 112-112: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/shop.products`.$handle.tsx around lines 109 - 114, The
product.descriptionHtml is rendered with dangerouslySetInnerHTML without
sanitization; sanitize it first (e.g., import sanitize-html or DOMPurify) and
use the sanitized string when setting dangerouslySetInnerHTML. Concretely,
create a sanitized variable (e.g., const safeDescription =
sanitizeHtml(product.descriptionHtml, { allowedTags: [...], allowedAttributes:
{...} }) or DOMPurify.sanitize(product.descriptionHtml)), then replace
dangerouslySetInnerHTML={{ __html: product.descriptionHtml }} with
dangerouslySetInnerHTML={{ __html: safeDescription }} in the component rendering
the product (the JSX block that references product.descriptionHtml).
Comment on lines
+55
to
+80
| const response = await fetch( | ||
| `https://${domain}/api/${version}/graphql.json`, | ||
| { | ||
| method: 'POST', | ||
| headers, | ||
| body: JSON.stringify({ query: input.query, variables: input.variables }), | ||
| }, | ||
| ) | ||
|
|
||
| if (!response.ok) { | ||
| throw new ShopifyError( | ||
| `Shopify API error: ${response.status} ${response.statusText}`, | ||
| ) | ||
| } | ||
|
|
||
| const json = (await response.json()) as ShopifyResponse<TData> | ||
|
|
||
| if (json.errors?.length) { | ||
| throw new ShopifyError( | ||
| json.errors.map((e) => e.message).join('\n'), | ||
| json.errors, | ||
| ) | ||
| } | ||
|
|
||
| if (!json.data) { | ||
| throw new ShopifyError('Shopify API returned no data and no errors.') |
There was a problem hiding this comment.
Bound and normalize upstream failures here.
This helper sits on every shop request path. Right now a stalled Shopify connection can hang indefinitely, and transport / JSON parse failures escape as generic exceptions instead of ShopifyError.
Proposed fix
- const response = await fetch(
- `https://${domain}/api/${version}/graphql.json`,
- {
- method: 'POST',
- headers,
- body: JSON.stringify({ query: input.query, variables: input.variables }),
- },
- )
+ const controller = new AbortController()
+ const timeoutId = setTimeout(() => controller.abort(), 10_000)
+
+ let response: Response
+ try {
+ response = await fetch(`https://${domain}/api/${version}/graphql.json`, {
+ method: 'POST',
+ headers,
+ body: JSON.stringify({ query: input.query, variables: input.variables }),
+ signal: controller.signal,
+ })
+ } catch (error) {
+ throw new ShopifyError(
+ error instanceof Error
+ ? `Shopify request failed: ${error.message}`
+ : 'Shopify request failed.',
+ )
+ } finally {
+ clearTimeout(timeoutId)
+ }
if (!response.ok) {
throw new ShopifyError(
`Shopify API error: ${response.status} ${response.statusText}`,
)
}
- const json = (await response.json()) as ShopifyResponse<TData>
+ let json: ShopifyResponse<TData>
+ try {
+ json = (await response.json()) as ShopifyResponse<TData>
+ } catch {
+ throw new ShopifyError('Shopify API returned invalid JSON.')
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/server/shopify/fetch.ts` around lines 55 - 80, The current fetch flow can
hang or surface non-Shopify exceptions; wrap the network/parse steps around a
timeout and try/catch and rethrow as ShopifyError with underlying error details:
when performing the fetch to `https://${domain}/api/${version}/graphql.json`
(the fetch call) use an AbortController with a sensible timeout and abort the
request on timeout, and surround both the fetch and the subsequent `await
response.json()` (parsed as `ShopifyResponse<TData>`) in a try/catch that
converts any thrown error into a `ShopifyError` (include the original error
message/stack as context), leaving the existing response.ok and json.errors
checks intact so upstream failures stay normalized.
Comment on lines
+108
to
+120
| export const getProduct = createServerFn({ method: 'POST' }) | ||
| .inputValidator(v.object({ handle: v.string() })) | ||
| .handler(async ({ data }): Promise<ProductDetail | null> => { | ||
| setBrowseCacheHeaders() | ||
| const result = await shopifyServerFetch< | ||
| ProductQueryResult, | ||
| { handle: string } | ||
| >({ | ||
| query: PRODUCT_QUERY, | ||
| variables: { handle: data.handle }, | ||
| }) | ||
| return result.product | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other server functions with input validators handle HTTP method
rg -n "createServerFn.*method:" --type ts -A2 | head -60Repository: TanStack/tanstack.com
Length of output: 3689
🏁 Script executed:
#!/bin/bash
# Check if there are any GET methods used with createServerFn
rg "createServerFn.*method.*GET" --type tsRepository: TanStack/tanstack.com
Length of output: 3804
🏁 Script executed:
#!/bin/bash
# Find and examine the setBrowseCacheHeaders function
rg -n "setBrowseCacheHeaders" --type ts -B2 -A5Repository: TanStack/tanstack.com
Length of output: 2758
🏁 Script executed:
#!/bin/bash
# Check other product/data retrieval functions in shop.functions.ts
cat -n src/utils/shop.functions.ts | head -150Repository: TanStack/tanstack.com
Length of output: 5444
Change getProduct to use GET method to enable edge caching.
getProduct is a read-only browse operation like getShop, getProducts, and getCollections, all of which use GET. However, getProduct uses POST, which prevents CDN edge caching regardless of setBrowseCacheHeaders() on line 111. POST responses are not cached by default. The input—a simple string handle parameter—can be passed as a query parameter instead of a request body, making GET feasible without sacrificing input validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/shop.functions.ts` around lines 108 - 120, Change getProduct to use
method: 'GET' so responses can be cached at the edge; update the handler to read
the handle from the request query instead of the POST body and validate it
there. Specifically, modify createServerFn({ method: 'POST' }) to
createServerFn({ method: 'GET' }), adjust the handler signature to accept and
validate the request query (instead of destructuring data) so you pass
variables: { handle: query.handle } into shopifyServerFetch, keep calling
setBrowseCacheHeaders(), and preserve the same return of result.product; ensure
the input validation for the handle is applied to the query parameter (e.g.,
validate query.handle with v.string()).
Comment on lines
+190
to
+197
| /* ────────────────────────────────────────────────────────────────────────── | ||
| * Cart — queries + mutations | ||
| * | ||
| * The cart lives in Shopify. We hit the Storefront API directly from the | ||
| * browser with the public token (no server hop needed), and manage the cart | ||
| * ID client-side via localStorage. Checkout redirects to `cart.checkoutUrl` | ||
| * which is Shopify-hosted. | ||
| * ────────────────────────────────────────────────────────────────────────── */ |
There was a problem hiding this comment.
Misleading comment: cart operations run server-side, not client-side.
The comment states "We hit the Storefront API directly from the browser with the public token... and manage the cart ID client-side via localStorage." However, per the PR description and shop.functions.ts, cart operations run via server functions with the private token, and the cart ID is stored in an httpOnly cookie—not localStorage.
📝 Suggested fix
/* ──────────────────────────────────────────────────────────────────────────
* Cart — queries + mutations
*
- * The cart lives in Shopify. We hit the Storefront API directly from the
- * browser with the public token (no server hop needed), and manage the cart
- * ID client-side via localStorage. Checkout redirects to `cart.checkoutUrl`
- * which is Shopify-hosted.
+ * The cart lives in Shopify. All cart operations go through server functions
+ * using the private Storefront API token, with the cart ID stored in an
+ * httpOnly cookie. Checkout redirects to `cart.checkoutUrl` which is
+ * Shopify-hosted.
* ────────────────────────────────────────────────────────────────────────── */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/shopify-queries.ts` around lines 190 - 197, Update the misleading
header comment in the Cart section of src/utils/shopify-queries.ts to reflect
that cart operations are performed server-side via shop.functions.ts (using the
private Storefront token) and that the cart ID is stored in an httpOnly cookie
rather than localStorage; locate the block around the "Cart — queries +
mutations" comment and replace the browser/localStorage wording with a concise
statement that cart requests go through the server functions (e.g.,
shop.functions.ts) and cookies are used for cart ID storage.
The cart refactor moved all Shopify Storefront API calls server-side (createServerFn), so VITE_SHOPIFY_* env vars are no longer consumed by any code path. Vite still statically substitutes import.meta.env with every VITE_* var set at build time, so if the deploy environment defines VITE_SHOPIFY_STORE_DOMAIN with the same value as the server SHOPIFY_STORE_DOMAIN, Netlify's secrets scanner matches the value inside the client bundle and fails the build. Removing the VITE_* entries from the client env schema (nothing references them) plus unsetting VITE_SHOPIFY_* in Netlify's env config resolves the scan without an allowlist.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/env.ts (1)
23-26: Add fail-fast validation for Shopify env completeness and non-empty values.Line 23-Line 26 currently accept optional raw strings, so empty values (or no token at all) can pass parsing and fail later as runtime Shopify errors. Consider enforcing a startup guard when Shopify is configured.
Proposed hardening (fail fast at startup)
const parsedServerEnv = import.meta.env.SSR ? v.parse(serverEnvSchema, process.env) : {} +if (import.meta.env.SSR) { + const storeDomain = parsedServerEnv.SHOPIFY_STORE_DOMAIN?.trim() + const apiVersion = parsedServerEnv.SHOPIFY_API_VERSION?.trim() + const privateToken = parsedServerEnv.SHOPIFY_PRIVATE_STOREFRONT_TOKEN?.trim() + const publicToken = parsedServerEnv.SHOPIFY_PUBLIC_STOREFRONT_TOKEN?.trim() + + if (storeDomain && !privateToken && !publicToken) { + throw new Error( + 'Shopify is configured but no storefront token is set. Provide SHOPIFY_PRIVATE_STOREFRONT_TOKEN or SHOPIFY_PUBLIC_STOREFRONT_TOKEN.', + ) + } + + if (apiVersion && !/^\d{4}-\d{2}$/.test(apiVersion)) { + throw new Error( + `Invalid SHOPIFY_API_VERSION '${apiVersion}'. Expected format YYYY-MM (example: 2026-01).`, + ) + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/env.ts` around lines 23 - 26, The environment validators for SHOPIFY_STORE_DOMAIN, SHOPIFY_API_VERSION, SHOPIFY_PUBLIC_STOREFRONT_TOKEN, and SHOPIFY_PRIVATE_STOREFRONT_TOKEN currently allow empty strings; update the validation in src/utils/env.ts to enforce non-empty values and a fail-fast startup guard when any Shopify configuration is present: change v.optional(v.string()) to require non-empty strings (e.g., v.string().min(1) or equivalent) for the token and domain keys, make SHOPIFY_API_VERSION default still but validate format if present, and add a startup check (e.g., in the module init or a validateShopifyEnv function) that throws/logs and exits immediately if Shopify is configured but required values (SHOPIFY_STORE_DOMAIN and at least one non-empty token) are missing or blank.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/env.ts`:
- Around line 23-26: The environment validators for SHOPIFY_STORE_DOMAIN,
SHOPIFY_API_VERSION, SHOPIFY_PUBLIC_STOREFRONT_TOKEN, and
SHOPIFY_PRIVATE_STOREFRONT_TOKEN currently allow empty strings; update the
validation in src/utils/env.ts to enforce non-empty values and a fail-fast
startup guard when any Shopify configuration is present: change
v.optional(v.string()) to require non-empty strings (e.g., v.string().min(1) or
equivalent) for the token and domain keys, make SHOPIFY_API_VERSION default
still but validate format if present, and add a startup check (e.g., in the
module init or a validateShopifyEnv function) that throws/logs and exits
immediately if Shopify is configured but required values (SHOPIFY_STORE_DOMAIN
and at least one non-empty token) are missing or blank.
Netlify's secrets scanner treats every env var value as sensitive and
fails the build when any of those values appear in the output, with no
semantic awareness of which ones are public. SHOPIFY_STORE_DOMAIN
(tanstack-2.myshopify.com) and SHOPIFY_API_VERSION (2026-01) are
public by design — Shopify prints the domain on every hosted-checkout
URL, order email, and receipt, and the version is a platform-wide
identifier.
Moving both to source constants in src/server/shopify/fetch.ts:
- Keeps them out of the env-var watchlist entirely (no scan false
positive, no per-site SECRETS_SCAN_OMIT_KEYS config needed)
- Makes the store identity explicit in the codebase where the fetch
helper lives
- Leaves actual secrets (SHOPIFY_PRIVATE_STOREFRONT_TOKEN) in env
After this change, the only Shopify env var the deploy needs is
SHOPIFY_PRIVATE_STOREFRONT_TOKEN.
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
1 participant
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
/shop,/shop/products/$handle,/shop/cart— replaces the outbound Cotton Bureau link on/merchgetCart/addToCart/updateCartLine/removeCartLine) with the cart ID in an httpOnly cookie; the private Storefront API token never leaves the server/shoploader prefetches the cart into React Query so every child page SSRs with real data on the first frame — no hydration gapcreateFileRoute(variable)pattern across library landings (was crashingrouter-generator's crawl and producing "Crawling result not available" at dev startup)Details
Shop
/shop— product grid sorted byBEST_SELLING, edge-cached for 5m + swr/shop/products/$handle— PDP with variant-option selectors (Color, Size, etc.), auto-resolves to matching variant, hero image swaps to the variant image, SEO meta from Shopify product SEO fields/shop/cart— full cart with line items, quantity stepper, trash button, summary panel, Checkout button redirecting to Shopify-hostedcart.checkoutUrlTitletriggers the main site nav's flyout mode on/shop/*, so the shop sidebar takes over the primary left rail/shop/*, site-wide once the cart has items, hidden elsewhere when emptyCart architecture
tanstack_cart_idhttpOnly cookie (server-only, secure in prod, 1-year max-age)createServerFn— one consistent auth path, private token stays server-side, rate-limit headroom via bot-protection buyer-IP header if we want it later['shopify', 'cart']key; optimisticonMutatebumpstotalQuantityfor instant navbar badge feedback on add, with full line-level optimism on update/remove,onErrorrollback,onSuccessreconcile with server truthRolled in
createFileRoute('/library/$version/')literal across 16 library-landing routes +check-name.ts(drops theas anycast) — this is the permanent fix for the router-generator crawl error.env.*glob in.gitignoreto prevent env backups/variants from leaking.env.localis now the single env file (folded.envinto it;dotenv -e .env.localin the dev script)Dependencies added
@shopify/hydrogen-react(types + Storefront API schema; no providers used, so the client bundle only picks up the small utility imports we actually use)Test plan
SHOPIFY_*+VITE_SHOPIFY_*env in Netlify for deploy preview + prod/shoplists products fromtanstack-2.myshopify.com/shop/cartshows the added line with correct quantity, variant options, and subtotal/shop/cartafter adding items → SSR paints the populated cart on the first frame (no skeleton, no hydration pop-in)/docs), the cart badge is hidden when empty and appears when the cart has itemsSummary by CodeRabbit
New Features
Documentation
Chores