-
Notifications
You must be signed in to change notification settings - Fork 964
rename SERVER_BASE_URL to NEXT_PUBLIC_SERVER_BASE_URL as previous one… #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from 'next/server'; | |||||
|
||||||
// The target backend server base URL, derived from environment variable or defaulted. | ||||||
// This should match the logic in your frontend's page.tsx for consistency. | ||||||
const TARGET_SERVER_BASE_URL = process.env.SERVER_BASE_URL || 'http://localhost:8001'; | ||||||
const TARGET_SERVER_BASE_URL = process.env.NEXT_PUBLIC_SERVER_BASE_URL || 'http://localhost:8001'; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This API route is server-side code and doesn't strictly need to use a For better separation of concerns and deployment flexibility, it's recommended to use a server-only environment variable for server-to-server communication. This prevents accidentally exposing server-internal configuration to the client and makes the architecture clearer.
Suggested change
|
||||||
|
||||||
// This is a fallback HTTP implementation that will be used if WebSockets are not available | ||||||
// or if there's an error with the WebSocket connection | ||||||
|
@@ -110,4 +110,4 @@ export async function OPTIONS() { | |||||
'Access-Control-Allow-Headers': 'Content-Type, Authorization', // Adjust as per client's request headers | ||||||
}, | ||||||
}); | ||||||
} | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's generally better to use a separate, non-public environment variable for server-side configurations like this one. The
NEXT_PUBLIC_
prefix is intended to expose variables to the browser, which isn't necessary for server-side code likenext.config.ts
and API routes.This separation allows for more flexible deployment scenarios. For example, you could use an internal service URL (e.g.,
http://api-service:8001
) on the server while the client uses a public-facing URL.To improve this, you could prioritize a server-specific variable and fall back to the public one. This would make the configuration more robust and align with best practices.