Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces type-safe environment variable access for Vite-based configuration. It defines TypeScript types for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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 |
![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
🧹 Nitpick comments (1)
app/routes/demos/12-mini-game/interface/index.tsx (1)
66-66: Build the link withURLinstead of string concatenation.Line 66 can produce malformed URLs depending on env formatting. Use
new URL()for safer composition.Proposed refactor
export function Interface() { + const betaMiniGameUrl = new URL("/demos/mini-game", EnvConfig.next_version).toString(); return <div className={styles.interface}> @@ - <p>You can try <a href={`${EnvConfig.next_version}/demos/mini-game`} target="_blank" rel="noopener noreferrer">new beta version 2 here</a>. But beware, it's in progress, so it might not be perfect yet.</p> + <p>You can try <a href={betaMiniGameUrl} target="_blank" rel="noopener noreferrer">new beta version 2 here</a>. But beware, it's in progress, so it might not be perfect yet.</p>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/routes/demos/12-mini-game/interface/index.tsx` at line 66, Replace the string-concatenated href that uses `${EnvConfig.next_version}/demos/mini-game` with a safe URL composition using the URL API: construct new URL('/demos/mini-game', EnvConfig.next_version) and use its href (or toString()) for the anchor's href; update the JSX in the component that renders this <a> (the line referencing EnvConfig.next_version) so it builds the link via URL to avoid malformed URLs when EnvConfig.next_version includes or omits trailing slashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/utils/env.ts`:
- Around line 5-7: Remove the console.log from the static getter next_version
and validate import.meta.env.VITE_NEXT_VERSION before returning it; in the
static get next_version method check for a non-empty string and either return a
safe default (e.g., "") or throw a clear error when VITE_NEXT_VERSION is
missing/invalid so callers (like the 12-mini-game URL construction) don't get
"undefined" in paths—update references to import.meta.env.VITE_NEXT_VERSION
inside next_version and ensure no env object is logged.
---
Nitpick comments:
In `@app/routes/demos/12-mini-game/interface/index.tsx`:
- Line 66: Replace the string-concatenated href that uses
`${EnvConfig.next_version}/demos/mini-game` with a safe URL composition using
the URL API: construct new URL('/demos/mini-game', EnvConfig.next_version) and
use its href (or toString()) for the anchor's href; update the JSX in the
component that renders this <a> (the line referencing EnvConfig.next_version) so
it builds the link via URL to avoid malformed URLs when EnvConfig.next_version
includes or omits trailing slashes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/routes/demos/12-mini-game/interface/index.tsxapp/types/env.d.tsapp/utils/env.ts
| static get next_version(): string { | ||
| console.log(import.meta.env); | ||
| return import.meta.env.VITE_NEXT_VERSION; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "env.ts" -type f | head -20Repository: gianzamboni/three-js-react
Length of output: 88
🏁 Script executed:
cat -n app/utils/env.tsRepository: gianzamboni/three-js-react
Length of output: 337
🏁 Script executed:
rg "next_version" --type ts --type js -B 2 -A 2Repository: gianzamboni/three-js-react
Length of output: 859
🏁 Script executed:
rg "VITE_NEXT_VERSION" --type ts --type js -B 2 -A 2Repository: gianzamboni/three-js-react
Length of output: 443
Validate VITE_NEXT_VERSION and remove env logging from the getter.
Line 6 logs the full env object in a hot path, and Line 7 assumes the variable is always present. The env var is used to construct URLs (e.g., in 12-mini-game interface); if missing, this produces undefined/demos/mini-game.
Proposed fix
export class EnvConfig {
static get next_version(): string {
- console.log(import.meta.env);
- return import.meta.env.VITE_NEXT_VERSION;
+ const baseUrl = import.meta.env.VITE_NEXT_VERSION;
+ if (!baseUrl) {
+ throw new Error("Missing required env var: VITE_NEXT_VERSION");
+ }
+ return baseUrl.replace(/\/+$/, "");
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/utils/env.ts` around lines 5 - 7, Remove the console.log from the static
getter next_version and validate import.meta.env.VITE_NEXT_VERSION before
returning it; in the static get next_version method check for a non-empty string
and either return a safe default (e.g., "") or throw a clear error when
VITE_NEXT_VERSION is missing/invalid so callers (like the 12-mini-game URL
construction) don't get "undefined" in paths—update references to
import.meta.env.VITE_NEXT_VERSION inside next_version and ensure no env object
is logged.
1 participant
Summary by CodeRabbit