-
-
Notifications
You must be signed in to change notification settings - Fork 321
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
Added env variable NEXT_PUBLIC_API_URL #245
base: main
Are you sure you want to change the base?
Conversation
@OsoThevenin is attempting to deploy a commit to the Hayden Bleasel Team on Vercel. A member of the Team first needs to authorize it. |
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThe changes introduce a new environment variable Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/.env.example (1)
26-26
: Document the purpose of NEXT_PUBLIC_API_URL.Consider adding documentation about this environment variable's purpose and usage in the project's README or documentation.
Example documentation:
### Environment Variables - `NEXT_PUBLIC_API_URL`: URL for the API service, used by web and app interfaces for making API requests.apps/app/.env.example (1)
27-27
: Document the API URL configurationSince this is a new API endpoint configuration, ensure it's properly documented in the project's documentation.
Consider adding:
- Purpose of the API endpoint
- Expected configuration in different environments
- Any specific requirements for local development
Would you like me to help create documentation for this new configuration?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
packages/env/index.ts
is excluded by none and included by none
📒 Files selected for processing (3)
apps/api/.env.example
(1 hunks)apps/app/.env.example
(1 hunks)apps/web/.env.example
(1 hunks)
🔇 Additional comments (4)
apps/web/.env.example (1)
26-26
: LGTM! Verify port availability.
The new environment variable follows the established naming conventions and is appropriately placed in the Client section. The localhost port pattern is consistent with other services.
Let's verify there are no port conflicts:
✅ Verification successful
Port 3002 is correctly assigned to the API service
The port 3002 is consistently used across the codebase for the API service:
- Documented in
docs/structure/apps.mdx
as the designated port for the API service - Referenced in API's
package.json
for development server and Stripe webhook forwarding - Mentioned in setup documentation as the API endpoint
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other services using port 3002 in the codebase
# Search for other occurrences of port 3002
rg -g '!*.env*' '3002'
# Search for docker-compose files that might define services
fd 'docker-compose' --exec cat {}
Length of output: 496
apps/app/.env.example (1)
27-27
: Verify port 3002 availability across the application stack
The addition of NEXT_PUBLIC_API_URL
follows proper naming conventions and is correctly placed in the Client section. However, let's ensure port 3002 doesn't conflict with other services.
✅ Verification successful
Port 3002 is correctly assigned and documented for the API service
The verification confirms that port 3002 is:
- Documented in
docs/setup.mdx
anddocs/structure/apps.mdx
as the designated port for the API service - Configured in
apps/api/package.json
for both the Next.js dev server and Stripe webhook forwarding - No port conflicts found with other services in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other services using port 3002 in the codebase
# and verify documentation of port usage
# Search for other occurrences of port 3002
rg -g '!*.env*' '3002'
# Check if ports are documented in README or similar files
fd -g 'README*' -X cat
fd -g 'CONTRIBUTING*' -X cat
Length of output: 1232
apps/api/.env.example (2)
27-28
: LGTM! Changes follow environment variable best practices
The changes look good:
- Logical grouping with other NEXT_PUBLIC_ variables
- Consistent use of localhost URLs in example file
- Clear and descriptive variable naming
27-28
: Verify the purpose of duplicate localhost:3002 URLs
I notice that both NEXT_PUBLIC_API_URL
and NEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL
are set to "http://localhost:3002". Could you clarify if these serve different purposes? If they're meant for the same thing, we might want to consolidate them to avoid confusion.
Let's check how these variables are used across the codebase:
✅ Verification successful
These environment variables serve distinct purposes - no consolidation needed
Based on the codebase analysis, these variables have different use cases:
NEXT_PUBLIC_API_URL
is used for API schema validation and configuration in the env packageNEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL
is specifically used for:- Generating sitemap URLs in web app
- Setting canonical URLs for blog posts
- Configuring robots.txt
- As documented, it's automatically provided by Vercel for production deployments
The duplicate localhost URL in the example file is just a development placeholder and doesn't indicate an actual conflict in production usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usages of both environment variables to understand their distinct purposes
echo "Searching for NEXT_PUBLIC_API_URL usage:"
rg "NEXT_PUBLIC_API_URL" -A 2
echo -e "\nSearching for NEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL usage:"
rg "NEXT_PUBLIC_VERCEL_PROJECT_PRODUCTION_URL" -A 2
Length of output: 2698
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@OsoThevenin Is this env var required anywhere currently? Or is it just future-facing? |
@haydenbleasel this is future-facing. I thought it could be useful |
Description
Added env variable NEXT_PUBLIC_API_URL in .env.example files
Related Issues
Closes #<issue_number>
Checklist
Screenshots (if applicable)
Additional Notes
I find it useful for me to query the API from web and app
Summary by CodeRabbit
New Features
NEXT_PUBLIC_API_URL
for client-side applications, allowing configuration of the API endpoint.Chores
NEXT_PUBLIC_DOCS_URL
variable to a new line without changing its value.