Centralize store config into src/lib/store.ts#108
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughCentralizes store configuration into a new Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Extract all store-level env var access (NEXT_PUBLIC_STORE_NAME, NEXT_PUBLIC_STORE_DESCRIPTION, NEXT_PUBLIC_SITE_URL, NEXT_PUBLIC_DEFAULT_COUNTRY, NEXT_PUBLIC_DEFAULT_LOCALE, EMAIL_FROM) into a single module with consistent fallback values. - Remove duplicated process.env access across 12+ files - Build ensureProtocol into getStoreUrl() so callers don't double-wrap - Remove re-exports from seo.ts — store config lives in store.ts only - Add console.warn when EMAIL_FROM fallback is used with Resend - Add console.warn when SITE_URL is empty (broken email links) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cichorek
force-pushed
the
refactor/centralize-store-config
branch
from
542934f to
8434f25
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/lib/metadata/home.ts (1)
14-16: Consider centralizing SEO-specific overrides.The code still directly accesses
process.env.STORE_SEO_TITLEandprocess.env.STORE_META_DESCRIPTIONwhile using the centralized helpers as fallbacks. While this pattern allows SEO-specific title/description overrides, it introduces inconsistency with the PR's centralization goal.Consider adding
getStoreSeoTitle()andgetStoreMetaDescription()helpers in@/lib/storethat encapsulate this override logic, keeping all environment variable access in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/metadata/home.ts` around lines 14 - 16, The code reads process.env.STORE_SEO_TITLE and process.env.STORE_META_DESCRIPTION directly when building storeName and description; centralize this override logic by adding new helpers getStoreSeoTitle() and getStoreMetaDescription() in the existing '@/lib/store' module that return the env override or fall back to getStoreName()/getStoreDescription(), then replace direct env access in src/lib/metadata/home.ts so storeName uses getStoreSeoTitle() and description uses getStoreMetaDescription().src/lib/webhooks/handlers.ts (1)
16-20: Scope this warning to production only.Current condition can spam logs in test/CI environments. Restricting it to production aligns better with the stated intent.
💡 Proposed fix
-if (!SITE_URL) { +if (!SITE_URL && process.env.NODE_ENV === "production") { console.warn( - "[webhooks] NEXT_PUBLIC_SITE_URL is not set — email links will be broken", + "[webhooks] NEXT_PUBLIC_SITE_URL / NEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL is not set — email links will be broken", ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/webhooks/handlers.ts` around lines 16 - 20, The console warning currently triggered by SITE_URL being unset should only run in production to avoid noisy CI/test logs; update the conditional that checks SITE_URL (the SITE_URL constant/variable in handlers.ts) to also require that process.env.NODE_ENV === 'production' (or your existing isProduction helper) before calling console.warn so the message only appears in production environments.
🤖 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/robots.ts`:
- Around line 6-7: Guard against empty baseUrl returned by getStoreUrl() before
building sitemap URLs and the host: if baseUrl is falsy, avoid producing
relative sitemap locations and leave host undefined or set to null. Update the
logic around baseUrl, the call to generateSitemaps(), and where sitemap
URLs/host are composed (references: baseUrl, getStoreUrl, generateSitemaps,
sitemap/host) so that when baseUrl === "" you skip prefixing sitemap paths with
an empty host and emit absolute URLs only when baseUrl is present; apply the
same guard where sitemap/host are used later (the other occurrence around
sitemap handling noted in the diff).
In `@src/lib/store.ts`:
- Around line 52-54: getDefaultCountry currently returns
process.env.NEXT_PUBLIC_DEFAULT_COUNTRY as-is which violates the JSDoc
"lowercase" promise; update the getDefaultCountry function to normalize the env
value to lowercase (e.g., call .toLowerCase() on the env value) while safely
handling undefined/null (fall back to "us") so the function always returns a
lowercase string; ensure you apply this change in the getDefaultCountry
implementation.
---
Nitpick comments:
In `@src/lib/metadata/home.ts`:
- Around line 14-16: The code reads process.env.STORE_SEO_TITLE and
process.env.STORE_META_DESCRIPTION directly when building storeName and
description; centralize this override logic by adding new helpers
getStoreSeoTitle() and getStoreMetaDescription() in the existing '@/lib/store'
module that return the env override or fall back to
getStoreName()/getStoreDescription(), then replace direct env access in
src/lib/metadata/home.ts so storeName uses getStoreSeoTitle() and description
uses getStoreMetaDescription().
In `@src/lib/webhooks/handlers.ts`:
- Around line 16-20: The console warning currently triggered by SITE_URL being
unset should only run in production to avoid noisy CI/test logs; update the
conditional that checks SITE_URL (the SITE_URL constant/variable in handlers.ts)
to also require that process.env.NODE_ENV === 'production' (or your existing
isProduction helper) before calling console.warn so the message only appears in
production environments.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ef6ab06-fbdd-4638-9763-95c38cf7d45d
📒 Files selected for processing (20)
.env.examplesrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/app/[country]/[locale]/(storefront)/c/[...permalink]/page.tsxsrc/app/[country]/[locale]/(storefront)/page.tsxsrc/app/[country]/[locale]/(storefront)/policies/[slug]/page.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/page.tsxsrc/app/[country]/[locale]/layout.tsxsrc/app/layout.tsxsrc/app/robots.tssrc/app/sitemap.tssrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/contexts/StoreContext.tsxsrc/lib/emails/send.tssrc/lib/metadata/home.tssrc/lib/metadata/store.tssrc/lib/seo.tssrc/lib/store.tssrc/lib/webhooks/handlers.tssrc/proxy.ts
- robots.ts: skip sitemap/host fields when baseUrl is empty - store.ts: normalize getDefaultCountry() to lowercase as JSDoc promises - store.ts: add getStoreSeoTitle() and getStoreMetaDescription() helpers - metadata/home.ts, metadata/store.ts: use new helpers instead of direct env access - handlers.ts: only warn about missing SITE_URL in production Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 12: Move the environment key NEXT_PUBLIC_SITE_URL so it appears above the
NEXT_PUBLIC_STORE_* keys in the .env.example to satisfy dotenv-linter's
UnorderedKey rule; locate the NEXT_PUBLIC_SITE_URL entry and cut/paste it into
the NEXT_PUBLIC_* block immediately before the NEXT_PUBLIC_STORE_* entries so
all NEXT_PUBLIC_* keys are grouped together and the linter warning disappears.
In `@src/lib/emails/send.ts`:
- Around line 63-69: The code in send.ts reads process.env.EMAIL_FROM directly;
replace that check with a store helper to centralize env access: add a function
isStoreEmailFromFallback() to src/lib/store.ts (returns boolean for missing
EMAIL_FROM) and update send.ts to call isStoreEmailFromFallback() instead of
reading process.env.EMAIL_FROM, keeping the existing logic that logs the warning
when getStoreEmailFrom() is being used as a fallback.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33ed9f18-191b-4c84-9a16-c5e2d4654ccb
📒 Files selected for processing (24)
.env.examplesrc/app/[country]/[locale]/(checkout)/layout.tsxsrc/app/[country]/[locale]/(storefront)/c/[...permalink]/page.tsxsrc/app/[country]/[locale]/(storefront)/page.tsxsrc/app/[country]/[locale]/(storefront)/policies/[slug]/page.tsxsrc/app/[country]/[locale]/(storefront)/products/[slug]/page.tsxsrc/app/[country]/[locale]/layout.tsxsrc/app/layout.tsxsrc/app/robots.tssrc/app/sitemap.tssrc/components/layout/Footer.tsxsrc/components/layout/Header.tsxsrc/contexts/StoreContext.tsxsrc/lib/emails/send.tssrc/lib/metadata/categories.tssrc/lib/metadata/category.tssrc/lib/metadata/home.tssrc/lib/metadata/product.tssrc/lib/metadata/products.tssrc/lib/metadata/store.tssrc/lib/seo.tssrc/lib/store.tssrc/lib/webhooks/handlers.tssrc/proxy.ts
✅ Files skipped from review due to trivial changes (13)
- src/app/[country]/[locale]/(storefront)/policies/[slug]/page.tsx
- src/lib/metadata/product.ts
- src/lib/metadata/category.ts
- src/lib/metadata/products.ts
- src/app/[country]/[locale]/(storefront)/products/[slug]/page.tsx
- src/app/[country]/[locale]/(storefront)/c/[...permalink]/page.tsx
- src/lib/metadata/categories.ts
- src/proxy.ts
- src/lib/metadata/home.ts
- src/lib/webhooks/handlers.ts
- src/lib/store.ts
- src/app/[country]/[locale]/(storefront)/page.tsx
- src/app/[country]/[locale]/(checkout)/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- src/app/robots.ts
- src/app/layout.tsx
- src/components/layout/Footer.tsx
- src/components/layout/Header.tsx
- src/contexts/StoreContext.tsx
- src/lib/metadata/store.ts
- src/app/[country]/[locale]/layout.tsx
Move NEXT_PUBLIC_SITE_URL before NEXT_PUBLIC_STORE_* in .env.example to satisfy dotenv-linter's UnorderedKey rule, and add isStoreEmailFromFallback() to src/lib/store.ts so send.ts no longer reads process.env.EMAIL_FROM directly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/webhooks/handlers.ts (1)
11-14: Consider adding explicit type annotations for module constants.Per coding guidelines on strict TypeScript, consider adding explicit types:
-const STORE_NAME = getStoreName(); -const SITE_URL = - getStoreUrl() || +const STORE_NAME: string = getStoreName(); +const SITE_URL: string = + getStoreUrl() || (process.env.NODE_ENV === "development" ? "http://localhost:3001" : "");This is a minor refinement since TypeScript infers the types correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/webhooks/handlers.ts` around lines 11 - 14, Add explicit TypeScript type annotations to the module constants: annotate STORE_NAME and SITE_URL with string types (e.g. declare STORE_NAME: string and SITE_URL: string) where they are initialized using getStoreName() and the getStoreUrl() || ... expression to satisfy strict typing; ensure the annotations match the runtime fallback behavior so SITE_URL is typed as string even when getStoreUrl() may return undefined.
🤖 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/lib/webhooks/handlers.ts`:
- Around line 11-14: Add explicit TypeScript type annotations to the module
constants: annotate STORE_NAME and SITE_URL with string types (e.g. declare
STORE_NAME: string and SITE_URL: string) where they are initialized using
getStoreName() and the getStoreUrl() || ... expression to satisfy strict typing;
ensure the annotations match the runtime fallback behavior so SITE_URL is typed
as string even when getStoreUrl() may return undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 732d50df-abd6-4420-b99f-3c3e9b8d666b
📒 Files selected for processing (7)
.env.examplesrc/app/robots.tssrc/lib/emails/send.tssrc/lib/metadata/home.tssrc/lib/metadata/store.tssrc/lib/store.tssrc/lib/webhooks/handlers.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/app/robots.ts
- .env.example
- src/lib/metadata/home.ts
Make storeName and storeUrl optional props with defaults from getStoreName()/getStoreUrl() in all four email components, removing the need for callers to pass them explicitly. Clean up handlers.ts and email preview files accordingly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/emails/order-confirmation.tsx`:
- Around line 60-61: The current assignment reintroduces a production-wide
hardcoded fallback for storeUrl; change the storeUrl assignment (where
getStoreUrl() is used) so the 'http://localhost:3001' fallback is only applied
in development. Replace the unconditional fallback with a conditional that uses
getStoreUrl() if present, otherwise returns the localhost only when
process.env.NODE_ENV === 'development' (and in production leave it undefined or
throw/raise an error so misconfiguration surfaces). Update any downstream uses
that assume storeUrl is always defined to handle the possibly undefined value
(or throw early) so production emails never contain the localhost link.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0f3caf5-b795-4612-9baf-ac47746263be
📒 Files selected for processing (9)
emails/order-canceled.tsxemails/order-confirmation.tsxemails/password-reset.tsxemails/shipment-shipped.tsxsrc/lib/emails/order-canceled.tsxsrc/lib/emails/order-confirmation.tsxsrc/lib/emails/password-reset.tsxsrc/lib/emails/shipment-shipped.tsxsrc/lib/webhooks/handlers.ts
💤 Files with no reviewable changes (3)
- emails/order-confirmation.tsx
- emails/order-canceled.tsx
- emails/shipment-shipped.tsx
getStoreUrl() now returns http://localhost:3001 only in development; in production it returns undefined so misconfiguration surfaces. Email templates and handlers.ts no longer carry their own fallback. Footer links and product links render conditionally when storeUrl is undefined. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/emails/shipment-shipped.tsx`:
- Around line 113-119: The product link construction in the Link rendering (in
src/lib/emails/shipment-shipped.tsx) can produce malformed URLs when storeUrl
ends with '/' or item.slug starts with '/'; update the Link href logic to
normalize both parts before concatenation: trim trailing slashes from storeUrl
and leading slashes from item.slug (use the normalized storeUrl alone when
item.slug is falsy) so the final URL always has exactly one slash between host
and path.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e7bb959-6038-46e8-87dc-32334e855f0c
📒 Files selected for processing (6)
src/lib/emails/order-canceled.tsxsrc/lib/emails/order-confirmation.tsxsrc/lib/emails/password-reset.tsxsrc/lib/emails/shipment-shipped.tsxsrc/lib/store.tssrc/lib/webhooks/handlers.ts
✅ Files skipped from review due to trivial changes (1)
- src/lib/store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/emails/password-reset.tsx
Comment on lines
+113
to
+119
| {storeUrl ? ( | ||
| <Link | ||
| href={ | ||
| item.slug | ||
| ? `${storeUrl}/products/${item.slug}` | ||
| : storeUrl | ||
| } |
There was a problem hiding this comment.
Normalize product URL construction before rendering email links.
Current concatenation can emit malformed URLs when storeUrl ends with / or item.slug starts with /.
Proposed fix
- href={
- item.slug
- ? `${storeUrl}/products/${item.slug}`
- : storeUrl
- }
+ href={
+ item.slug
+ ? new URL(
+ `/products/${item.slug.replace(/^\/+/, "")}`,
+ storeUrl,
+ ).toString()
+ : storeUrl
+ }📝 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
| {storeUrl ? ( | |
| <Link | |
| href={ | |
| item.slug | |
| ? `${storeUrl}/products/${item.slug}` | |
| : storeUrl | |
| } | |
| {storeUrl ? ( | |
| <Link | |
| href={ | |
| item.slug | |
| ? new URL( | |
| `/products/${item.slug.replace(/^\/+/, "")}`, | |
| storeUrl, | |
| ).toString() | |
| : storeUrl | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/emails/shipment-shipped.tsx` around lines 113 - 119, The product link
construction in the Link rendering (in src/lib/emails/shipment-shipped.tsx) can
produce malformed URLs when storeUrl ends with '/' or item.slug starts with '/';
update the Link href logic to normalize both parts before concatenation: trim
trailing slashes from storeUrl and leading slashes from item.slug (use the
normalized storeUrl alone when item.slug is falsy) so the final URL always has
exactly one slash between host and path.
Contributor
Author
There was a problem hiding this comment.
Product slugs, are creating on our backend - we trust format is good.
There was a problem hiding this comment.
@Cichorek, understood — since slugs are backend-generated and their format is trusted, no normalization is needed here. I'll keep this in mind for future reviews.
(╯°□°）╯
✏️ Learnings added
Learnt from: Cichorek
Repo: spree/storefront PR: 108
File: src/lib/emails/shipment-shipped.tsx:113-119
Timestamp: 2026-04-07T09:42:19.503Z
Learning: In spree/storefront, product slugs in email templates (e.g. src/lib/emails/shipment-shipped.tsx, order-confirmation.tsx) are generated by the backend and their format (no leading slashes, no double slashes) is trusted. Do not flag URL concatenation like `${storeUrl}/products/${item.slug}` as potentially malformed — no slug normalization is required.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Cichorek
Repo: spree/storefront PR: 43
File: src/components/navigation/megaMenu/MegaMenu.tsx:186-204
Timestamp: 2026-03-04T13:40:29.672Z
Learning: In the spree/storefront repository, do not flag useEffect-based data fetching in client components (e.g., MegaMenu.tsx, Header.tsx) as violations. The project currently uses client-side data fetching in useEffect; server components data fetching is a future consideration. Treat useEffect fetching in client components as acceptable for now and focus reviews on other issues (dependencies, side effects, rendering correctness).
Resolve conflicts in 5 files, keeping centralized @/lib/store imports while adopting main's i18n (next-intl) and SDK 0.21.0 changes. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cichorek
force-pushed
the
refactor/centralize-store-config
branch
from
c55c41c to
696ee65
Compare
damianlegawiec
approved these changes
Apr 9, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NEXT_PUBLIC_STORE_NAME,NEXT_PUBLIC_STORE_DESCRIPTION,NEXT_PUBLIC_SITE_URL,NEXT_PUBLIC_DEFAULT_COUNTRY,NEXT_PUBLIC_DEFAULT_LOCALE,EMAIL_FROM) into a singlesrc/lib/store.tsmodule with consistent fallback valuesprocess.envaccess across 12+ files — onlysrc/lib/store.tsreads these vars nowensureProtocolintogetStoreUrl()so callers no longer need to wrap it manuallyseo.ts— store config has a single import path (@/lib/store)console.warnwhenEMAIL_FROMfallback is used with Resend (would silently fail with 403/422)console.warnwhenSITE_URLis empty in production (email links would be broken)Test plan
npm run check(Biome) passesnpm run buildcompletes without errors/sitemap/0.xml).next/emails/)process.env.NEXT_PUBLIC_STORE_*access outsidesrc/lib/store.tsvia grep🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Chores