Open
Conversation
✅ Deploy Preview for btcmap ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
Review Summary by QodoLazy-load localized merchant names from API
WalkthroughsDescription• Lazy-load localized merchant names based on browser language preference • Fetch translated names from BTC Map API when available • Display localized names in page title, heading, and section headers • Gracefully fall back to default name if translation unavailable Diagramflowchart LR
A["Browser Language"] -->|Check if not EN| B["Fetch Localized Name"]
B -->|API Call| C["BTC Map API"]
C -->|Response| D{Name Different?}
D -->|Yes| E["Use Localized Name"]
D -->|No| F["Use Default Name"]
E --> G["Display in UI"]
F --> G
File Changes1. src/routes/merchant/[id]/+page.svelte
|
Contributor
Code Review by Qodo
1. localizedNameFetched unused variable
|
📝 WalkthroughWalkthroughThis pull request adds localization support for merchant names by fetching a localized version when the page loads (non-English), then replacing all usages of the original merchant name throughout the page with the localized variant, falling back to the original name if the fetch fails. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 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 |
Contributor
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+140
to
+147
| let localizedNameFetched = false; | ||
|
|
||
| $: { | ||
| const lang = navigator.language; | ||
| if (lang !== "en") { | ||
| fetchLocalizedName(data.id, data.name, lang); | ||
| } | ||
| localizedNameFetched = true; |
Contributor
There was a problem hiding this comment.
1. localizednamefetched unused variable 📘 Rule violation ✓ Correctness
The PR introduces localizedNameFetched but it is never read, which commonly triggers no-unused-vars lint errors. This can cause yarn run lint to fail.
Agent Prompt
## Issue description
`localizedNameFetched` is declared and assigned but never read, which can cause lint failures (unused variable).
## Issue Context
The variable appears intended to prevent repeated fetches from the reactive block, but it currently does not affect control flow.
## Fix Focus Areas
- src/routes/merchant/[id]/+page.svelte[140-147]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+142
to
+146
| $: { | ||
| const lang = navigator.language; | ||
| if (lang !== "en") { | ||
| fetchLocalizedName(data.id, data.name, lang); | ||
| } |
Contributor
There was a problem hiding this comment.
2. Ssr crash on navigator 🐞 Bug ⛯ Reliability
The new reactive block reads navigator.language unconditionally, which will throw during SSR and break rendering of the merchant page. This component already uses browser guards elsewhere, so this is a high-confidence regression.
Agent Prompt
## Issue description
Merchant page SSR can crash because a `$:` reactive block reads `navigator.language` unconditionally.
## Issue Context
SvelteKit renders this component during SSR. Browser-only APIs (`navigator`) must be guarded.
## Fix Focus Areas
- src/routes/merchant/[id]/+page.svelte[139-148]
### Suggested change
Wrap the reactive block with a `browser` check (already imported in this file), e.g.:
- `if (!browser) return;`
- then read `navigator.language` and trigger the fetch.
Alternatively: move the localized-name fetch into `onMount` so it never runs on the server.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+142
to
+163
| $: { | ||
| const lang = navigator.language; | ||
| if (lang !== "en") { | ||
| fetchLocalizedName(data.id, data.name, lang); | ||
| } | ||
| localizedNameFetched = true; | ||
| } | ||
|
|
||
| async function fetchLocalizedName( | ||
| placeId: string, | ||
| defaultName: string | undefined, | ||
| lang: string, | ||
| ) { | ||
| try { | ||
| const response = await fetch( | ||
| `https://api.btcmap.org/v4/places/${placeId}?fields=name&lang=${lang}`, | ||
| ); | ||
| if (!response.ok) return; | ||
| const data = await response.json(); | ||
| if (data.name && data.name !== defaultName) { | ||
| localizedName = data.name; | ||
| } |
Contributor
There was a problem hiding this comment.
3. Racy/stale localized name 🐞 Bug ✓ Correctness
Client-side navigation between merchant IDs can briefly show the previous merchant’s localized name, and a slower previous fetch can overwrite the current merchant name due to missing reset and missing request-generation guard. This affects title/H1 and can confuse users.
Agent Prompt
## Issue description
`localizedName` can become stale and async results can race when navigating between different merchant IDs in the same route component instance.
## Issue Context
SvelteKit can reuse `+page.svelte` instances across parameter changes. Without resetting state and guarding async updates, an older fetch can overwrite newer data.
## Fix Focus Areas
- src/routes/merchant/[id]/+page.svelte[139-167]
### Suggested approach
- Introduce a request generation counter (similar to `CountryCard.svelte`).
- In the reactive block triggered by `data.id` / `data.name`, set `localizedName = undefined` (or `localizedName = data.name`) before kicking off the fetch.
- When the fetch resolves, only apply the result if the generation id matches the latest one (or `placeId === data.id` at time of resolution).
- Optionally use `AbortController` to cancel in-flight requests when `data.id` changes.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/routes/merchant/`[id]/+page.svelte:
- Around line 142-167: The reactive block that uses navigator.language and
fetchLocalizedName is not SSR-safe and can produce stale localizedName on
navigation; wrap the reactive block in a browser guard (import { browser } from
"$app/environment") and use the $locale store as the primary locale source
before falling back to navigator.language, reset localizedName when data.id
changes, and implement per-request cancellation by tracking a local requestId
(increment on each fetch) inside fetchLocalizedName so responses with
non-matching requestId are ignored; update the $: reactive block to call
fetchLocalizedName(data.id, data.name, locale, currentRequestId) only in browser
and after resetting localizedName, and modify fetchLocalizedName to
accept/validate the requestId and early-return if it’s stale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa9a7c56-0d77-4798-b2fc-5108f9702b74
📒 Files selected for processing (1)
src/routes/merchant/[id]/+page.svelte
Comment on lines
+142
to
+167
| $: { | ||
| const lang = navigator.language; | ||
| if (lang !== "en") { | ||
| fetchLocalizedName(data.id, data.name, lang); | ||
| } | ||
| localizedNameFetched = true; | ||
| } | ||
|
|
||
| async function fetchLocalizedName( | ||
| placeId: string, | ||
| defaultName: string | undefined, | ||
| lang: string, | ||
| ) { | ||
| try { | ||
| const response = await fetch( | ||
| `https://api.btcmap.org/v4/places/${placeId}?fields=name&lang=${lang}`, | ||
| ); | ||
| if (!response.ok) return; | ||
| const data = await response.json(); | ||
| if (data.name && data.name !== defaultName) { | ||
| localizedName = data.name; | ||
| } | ||
| } catch { | ||
| // Silent failure - page continues with original name | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
SvelteKit Svelte reactive statements $: run during server-side rendering SSR browser globals navigator unavailable
💡 Result:
In SvelteKit, your component’s <script> (including legacy reactive statements like $:) is executed during server-side rendering (SSR) to produce the initial HTML. Since SSR runs in a server (Node/edge) environment, browser globals like navigator, window, and document don’t exist there—so $: navigator... will throw. SvelteKit’s docs explicitly note that SSR means the page is rendered on the server first, then hydrated in the browser later [1].
Fix patterns
- Guard browser-only code
<script>
import { browser } from '$app/environment';
$: if (browser) {
console.log(navigator.userAgent);
}
</script>browser is true only in the browser, false on the server [2].
- Move browser-only work to
onMount(Svelte 3/4/5)
<script>
import { onMount } from 'svelte';
onMount(() => {
console.log(navigator.userAgent);
});
</script>onMount does not run during server-side rendering [3].
-
(Svelte 5 runes) Use
$effectfor client-only effects
$effectalso does not run during SSR (so it’s safe fornavigator/DOM usage) [3]. -
Disable SSR for a page/layout (last resort)
// +page.js (or +layout.js)
export const ssr = false;This renders an empty shell on the server and does everything on the client [1].
Sources
[1] SvelteKit Docs — Page options (ssr) https://svelte.dev/docs/kit/page-options
[2] SvelteKit Docs — $app/environment (browser) https://svelte.dev/docs/kit/%24app-environment
[3] Svelte Docs — onMount / $effect SSR behavior https://svelte.dev/docs/svelte/svelte
[4] Stack Overflow — reactive blocks don’t wait for mount (run during init) https://stackoverflow.com/questions/64639780/how-do-you-write-a-reactive-statement-in-svelte-sapper-when-it-needs-to-access-a
🏁 Script executed:
# Check the actual code at Lines 142-167 in the merchant page
head -n 200 src/routes/merchant/[id]/+page.svelte | tail -n 70Repository: teambtcmap/btcmap.org
Length of output: 2061
🏁 Script executed:
# Check the imports at the top of the file to see if `browser` is imported
head -n 60 src/routes/merchant/[id]/+page.svelteRepository: teambtcmap/btcmap.org
Length of output: 1964
🏁 Script executed:
# Verify the i18n setup and locale store
cat -n src/lib/i18n/index.ts | head -n 80Repository: teambtcmap/btcmap.org
Length of output: 1642
Make the localized-name fetch SSR-safe and navigation-safe.
The reactive block at line 142 reads navigator.language without a browser guard, causing SSR to fail since browser globals don't exist on the server. Additionally, the block never clears localizedName or rejects stale responses, so client-side navigation between merchants can momentarily show the previous merchant's name or allow an older request to overwrite the new page data. The new <svelte:head> values will also remain untranslated in the SSR HTML because the fetch only runs in the browser.
Wrap the reactive block with browser, use the $locale store as the first locale source, reset localizedName when data.id changes, and track request IDs to ignore outdated responses.
🛠️ Fix sketch
-import { _ } from "$lib/i18n";
+import { _, locale } from "$lib/i18n";
...
let localizedName: string | undefined;
-let localizedNameFetched = false;
+let localizedNameRequest = 0;
-$: {
- const lang = navigator.language;
- if (lang !== "en") {
- fetchLocalizedName(data.id, data.name, lang);
- }
- localizedNameFetched = true;
-}
+$: if (browser) {
+ const lang = $locale ?? navigator.languages?.[0] ?? navigator.language ?? "en";
+ const requestId = ++localizedNameRequest;
+ localizedName = undefined;
+
+ if (!lang.startsWith("en")) {
+ void fetchLocalizedName(data.id, data.name, lang, requestId);
+ }
+}
async function fetchLocalizedName(
placeId: string,
defaultName: string | undefined,
lang: string,
+ requestId: number,
) {
try {
const response = await fetch(
`https://api.btcmap.org/v4/places/${placeId}?fields=name&lang=${lang}`,
);
if (!response.ok) return;
const data = await response.json();
- if (data.name && data.name !== defaultName) {
+ if (
+ requestId === localizedNameRequest &&
+ data.name &&
+ data.name !== defaultName
+ ) {
localizedName = data.name;
}
} catch {
// Silent failure - page continues with original name
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/routes/merchant/`[id]/+page.svelte around lines 142 - 167, The reactive
block that uses navigator.language and fetchLocalizedName is not SSR-safe and
can produce stale localizedName on navigation; wrap the reactive block in a
browser guard (import { browser } from "$app/environment") and use the $locale
store as the primary locale source before falling back to navigator.language,
reset localizedName when data.id changes, and implement per-request cancellation
by tracking a local requestId (increment on each fetch) inside
fetchLocalizedName so responses with non-matching requestId are ignored; update
the $: reactive block to call fetchLocalizedName(data.id, data.name, locale,
currentRequestId) only in browser and after resetting localizedName, and modify
fetchLocalizedName to accept/validate the requestId and early-return if it’s
stale.
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.
Note: locales are not limited to hardcoded ones, so it ingnores any website-scoped user preferences and relies on browser preferences instead.
If there is a way of getting user overrides, we can try user-set locales first, but I'm not sure how to do it in a clean way and if it's reliable, I suspect it can default to returning
enfor non-whitelisted languages, which is not optimal.Summary by CodeRabbit