Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe changes remove cloud-specific feature gating from the 2D/3D mode toggle in the globe component, making it universally available, and add MAPBOX_TOKEN environment variable configuration to the Docker Compose backend service. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/src/app/[site]/globe/components/ModeSelector.tsx(2 hunks)docker-compose.yml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{client,server}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{client,server}/**/*.{ts,tsx}: Use TypeScript with strict typing throughout both client and server
Use try/catch blocks with specific error types for error handling
Use camelCase for variables and functions, PascalCase for components and types
Group imports by external, then internal, and sort alphabetically within groups
Files:
client/src/app/[site]/globe/components/ModeSelector.tsx
client/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Frontend: Use Next.js, Tailwind CSS, Shadcn UI, Tanstack Query, Zustand, Luxon, Nivo, and react-hook-form
Files:
client/src/app/[site]/globe/components/ModeSelector.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build Client Images (ubuntu-24.04-arm, linux/arm64)
- GitHub Check: Build Client Images (ubuntu-latest, linux/amd64)
| import { cn } from "@/lib/utils"; | ||
| import { Clock, Globe2, HouseIcon, Radio } from "lucide-react"; | ||
| import { useGlobeStore } from "../globeStore"; |
There was a problem hiding this comment.
Fix import ordering to comply with coding guidelines.
The import grouping does not follow the coding guidelines, which specify: "Group imports by external, then internal, and sort alphabetically within groups." External libraries (lucide-react) should come before internal imports (@/lib/utils).
As per coding guidelines. Apply this diff to correct the import order:
-import { cn } from "@/lib/utils";
-import { Clock, Globe2, HouseIcon, Radio } from "lucide-react";
-import { useGlobeStore } from "../globeStore";
+import { Clock, Globe2, HouseIcon, Radio } from "lucide-react";
+
+import { cn } from "@/lib/utils";
+import { useGlobeStore } from "../globeStore";📝 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.
| import { cn } from "@/lib/utils"; | |
| import { Clock, Globe2, HouseIcon, Radio } from "lucide-react"; | |
| import { useGlobeStore } from "../globeStore"; | |
| import { Clock, Globe2, HouseIcon, Radio } from "lucide-react"; | |
| import { useGlobeStore } from "../globeStore"; | |
| import { cn } from "@/lib/utils"; |
🤖 Prompt for AI Agents
In client/src/app/[site]/globe/components/ModeSelector.tsx around lines 1 to 3,
the import grouping/order is incorrect: external libs should come before
internal imports and each group should be alphabetically sorted; reorder the
imports so the lucide-react import appears first (alphabetized members if
needed), followed by the internal "@/lib/utils" and then the local
"../globeStore" import, ensuring each group is separated and sorted to comply
with the coding guidelines.
| <button | ||
| className="text-xs font-medium rounded-lg bg-neutral-900/60 text-neutral-200 backdrop-blur-sm p-1.5 border border-white/10 hover:bg-neutral-700/60 hover:text-white transition-all" | ||
| onClick={() => setMapMode(mapMode === "2D" ? "3D" : "2D")} | ||
| > | ||
| {mapMode === "2D" ? "2D" : "3D"} | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
Good change - 2D/3D toggle now universally available.
Removing the IS_CLOUD gating makes the map mode toggle available in all environments, which aligns with the PR objectives to support self-hosted Mapbox configurations.
Verify that no other IS_CLOUD references remain in globe-related components that might need similar updates:
🏁 Script executed:
#!/bin/bash
# Search for remaining IS_CLOUD usage in globe components
rg -nP --type=ts --type=tsx 'IS_CLOUD' client/src/app/\[site\]/globe/Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for remaining IS_CLOUD usage in globe components (corrected)
rg -nP --type=ts 'IS_CLOUD' client/src/app/\[site\]/globe/Length of output: 170
🏁 Script executed:
#!/bin/bash
# View the globeStore.ts file to understand the context of IS_CLOUD usage
cat client/src/app/\[site\]/globe/globeStore.tsLength of output: 499
Remove IS_CLOUD gating from initial mapMode state in globeStore.ts.
The ModeSelector.tsx change removes IS_CLOUD gating from the toggle button, making the 2D/3D mode universally available. However, globeStore.ts (line 15) still gates the initial mapMode value:
mapMode: IS_CLOUD ? "3D" : "2D",This creates an inconsistency: non-cloud environments will always reset to "2D" on page refresh, even if the user toggled to "3D". To fully support self-hosted Mapbox configurations, initialize mapMode to a consistent default (e.g., "2D") without the IS_CLOUD check, allowing the toggle to persist user preference.
Additionally, the IS_CLOUD import on line 3 can be removed once this change is made.
🤖 Prompt for AI Agents
In client/src/app/[site]/globe/globeStore.ts around lines 3 and 15, remove the
IS_CLOUD gating for the initial mapMode and set mapMode to a consistent default
(e.g., mapMode: "2D") so non-cloud installs can toggle to 3D and keep that
preference; also remove the now-unused IS_CLOUD import on line 3 and ensure any
related code/path still compiles after the import removal.
| - BASE_URL=${BASE_URL} | ||
| - DISABLE_SIGNUP=${DISABLE_SIGNUP} | ||
| - DISABLE_TELEMETRY=${DISABLE_TELEMETRY} | ||
| - MAPBOX_TOKEN=${MAPBOX_TOKEN} |
There was a problem hiding this comment.
🧩 Analysis chain
Verify backend behavior when MAPBOX_TOKEN is not provided.
The environment variable is added without a default value. Ensure the backend gracefully handles cases where MAPBOX_TOKEN is undefined or empty, especially for self-hosted deployments that may not use Mapbox.
Run the following script to check how the backend handles the MAPBOX_TOKEN:
Additionally, verify if documentation (e.g., .env.example, README.md, setup guides) needs to be updated to inform users about this new optional environment variable:
🏁 Script executed:
#!/bin/bash
# Search for MAPBOX_TOKEN usage in backend code to verify error handling
rg -nP --type=ts -C5 'MAPBOX_TOKEN|mapbox.*token' server/Length of output: 1318
🏁 Script executed:
#!/bin/bash
# Check if MAPBOX_TOKEN is documented
rg -n 'MAPBOX_TOKEN' README.md .env.example docs/ 2>/dev/null || echo "MAPBOX_TOKEN not found in common documentation files"Length of output: 52
🏁 Script executed:
#!/bin/bash
# Search for mapboxToken usage in frontend/client code
rg -nP --type=ts --type=tsx -C5 'mapboxToken' client/ src/ 2>/dev/null | head -80Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check for any Mapbox initialization or error handling
rg -nP --type=ts --type=tsx -C3 'mapbox|MAPBOX' client/ src/ 2>/dev/null | grep -A3 -B3 'error\|throw\|catch\|warn\|log' | head -60Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find all directories to understand project structure
fd -t d -max-depth 2 | head -30Length of output: 230
🏁 Script executed:
#!/bin/bash
# Search for getConfig endpoint calls and mapboxToken usage broadly
rg -nP 'getConfig|mapboxToken' --type=ts --type=tsx --type=js --type=jsx 2>/dev/null | head -50Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's any error handling or optional chaining around token usage
rg -nP 'mapbox' -i --type=ts --type=tsx --type=js --type=jsx 2>/dev/null | head -40Length of output: 0
🏁 Script executed:
#!/bin/bash
# List top-level directories and structure
ls -laLength of output: 1844
🏁 Script executed:
#!/bin/bash
# Find all files that import from getConfig
rg -l 'from.*getConfig|import.*getConfig' 2>/dev/nullLength of output: 19
🏁 Script executed:
#!/bin/bash
# Check for .env files and documentation
ls -la .env* README* 2>/dev/null || echo "Checking alternative locations..."
find . -maxdepth 2 -name '.env*' -o -name 'README*' -o -name '.env.example' 2>/dev/nullLength of output: 282
🏁 Script executed:
#!/bin/bash
# Check docker-compose file to see full context around MAPBOX_TOKEN addition
cat docker-compose.yml | grep -A10 -B10 'MAPBOX_TOKEN'Length of output: 757
🏁 Script executed:
#!/bin/bash
# Check root .env.example for MAPBOX_TOKEN documentation
cat .env.exampleLength of output: 709
🏁 Script executed:
#!/bin/bash
# Check server/.env.example
cat server/.env.exampleLength of output: 756
🏁 Script executed:
#!/bin/bash
# Check server/src/index.ts to see how getConfig is used
cat server/src/index.ts | grep -A5 -B5 'getConfig'Length of output: 1717
🏁 Script executed:
#!/bin/bash
# Search for any config endpoint or getConfig calls in client
rg -n 'config|/api' client/ --type=ts --type=tsx --type=js --type=jsx 2>/dev/null | grep -i 'getConfig\|/api/config\|config.*fetch' | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Broader search for /api/config usage or config endpoint calls
rg -n '/api/config\|getConfig|fetch.*config' --type=ts --type=tsx --type=js --type=jsx 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there are type definitions or interfaces for config response
rg -n 'mapboxToken|MAPBOX_TOKEN' -A2 -B2 --type=ts --type=tsx 2>/dev/nullLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at the actual getConfig.ts file to see full implementation
cat server/src/api/getConfig.tsLength of output: 290
Add MAPBOX_TOKEN to documentation and ensure consistent docker-compose default pattern.
The environment variable is missing from .env.example files and lacks a default value in docker-compose.yml, creating an undocumented requirement for operators. While the backend code at server/src/api/getConfig.ts passes the token through without validation or error handling, the critical issue is the documentation and configuration gap.
Required fixes:
- Add
MAPBOX_TOKEN=to both.env.exampleandserver/.env.exampleto document this optional variable - Update
docker-compose.ymlline 87 to use consistent pattern:- MAPBOX_TOKEN=${MAPBOX_TOKEN:-}(matching other optional env vars likeCLICKHOUSE_PASSWORD) - Consider adding optional chaining or null-check in
server/src/api/getConfig.tsif the frontend depends on this token, or document that it's safe to be undefined
🤖 Prompt for AI Agents
In docker-compose.yml around line 87, .env.example and server/.env.example, and
server/src/api/getConfig.ts, the MAPBOX_TOKEN env var is undocumented and
docker-compose lacks the optional-default pattern; add a MAPBOX_TOKEN= entry to
both .env.example files to document the optional variable, change the
docker-compose line to use the same optional-default pattern as other vars (i.e.
set the container env to use MAPBOX_TOKEN with an empty-default), and in
server/src/api/getConfig.ts either add a null-check/optional chaining before
passing the token to the frontend or update documentation/comments to state the
token may be undefined.
Summary by CodeRabbit
New Features
Chores