fix build #164
fix build #164
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe pull request introduces changes across multiple files to enhance the handling of the Supabase client and improve error management. The initialization of the Changes
Possibly related PRs
Warning Rate limit exceeded@thomasdavis has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/registry/app/jobs/page.jsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next" to extend from. Please check that the name of the config is correct. The config "next" was referenced from the config file in "/packages/eslint-config-custom/index.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. 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
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (7)
apps/registry/pages/api/jobs/all.js (2)
20-21: Consider moving Supabase URL to environment variablesWhile the key is properly handled via environment variables, the Supabase URL is hardcoded. This could make environment switching (staging/production) more difficult.
-const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co'; +const supabaseUrl = process.env.SUPABASE_URL;
26-29: Improve readability of time calculationThe current date calculation is difficult to read. Consider using named constants or MS_PER_DAY to make it more maintainable.
+const DAYS_TO_FETCH = 90; +const MS_PER_DAY = 24 * 60 * 60 * 1000; .gte( 'created_at', - new Date(Date.now() - 60 * 24 * 60 * 90 * 1000).toISOString() + new Date(Date.now() - DAYS_TO_FETCH * MS_PER_DAY).toISOString() )apps/registry/app/api/jobs/[uuid]/route.js (3)
6-7: LGTM! Consider enhancing the comment.The dynamic export is correctly implemented to prevent static optimization during build time.
Consider making the comment more descriptive:
-// This ensures the route is always dynamic +// Force dynamic rendering to ensure runtime evaluation of environment variables
19-19: Consider caching the Supabase client.While moving the client initialization inside the GET function helps with build-time issues, creating a new client for each request could impact performance under high load.
Consider caching the client instance:
+let supabaseClient = null; + export async function GET(request, { params }) { if (!process.env.SUPABASE_KEY) { return NextResponse.json( { message: 'API not available during build' }, { status: 503 } ); } try { - const supabase = createClient(supabaseUrl, process.env.SUPABASE_KEY); + if (!supabaseClient) { + supabaseClient = createClient(supabaseUrl, process.env.SUPABASE_KEY); + }
40-40: Enhance error logging for better debugging.The generic error message 'Error:' provides less context for debugging purposes.
Consider adding more context to the error log:
- console.error('Error:', error); + console.error(`Error fetching job ${params.uuid}:`, error);apps/registry/app/explore/page.js (2)
31-31: Consider memoizing the Supabase clientCreating a new Supabase client for each request could be inefficient. Consider memoizing the client instance.
+// Memoized client instance +let supabaseClient = null; + async function getResumes(page = 1, search = '') { try { if (!process.env.SUPABASE_KEY) { return { resumes: [], totalCount: 0, totalPages: 0 }; } - const supabase = createClient(supabaseUrl, process.env.SUPABASE_KEY); + if (!supabaseClient) { + supabaseClient = createClient(supabaseUrl, process.env.SUPABASE_KEY); + } + const supabase = supabaseClient;
Line range hint
5-5: Move sensitive database URL to environment variablesThe Supabase URL is currently hardcoded in the source code. This is a security concern as it exposes the database endpoint.
-const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co'; +const supabaseUrl = process.env.SUPABASE_URL;Please add
SUPABASE_URLto your environment variables and update the documentation accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
apps/registry/app/api/jobs/[uuid]/route.js(2 hunks)apps/registry/app/explore/page.js(1 hunks)apps/registry/pages/api/jobs/all.js(1 hunks)
🔇 Additional comments (6)
apps/registry/pages/api/jobs/all.js (4)
5-7: LGTM: Edge runtime configuration is appropriate
The edge runtime configuration is well-suited for this API endpoint, providing better performance and scalability.
14-17: LGTM: Proper build-time protection added
Good addition of the SUPABASE_KEY check with appropriate 503 status code. This prevents build failures and provides a clear message about API availability.
34-34: LGTM: Proper null handling in response
Good defensive programming practice using the || operator to ensure an empty array is returned instead of null.
37-37: LGTM: Secure error handling implementation
Good security practice to log the detailed error internally while returning a generic message to the client.
apps/registry/app/api/jobs/[uuid]/route.js (1)
10-16: Well-implemented build-time environment check!
The addition of this check with a 503 status code is a good practice for handling the absence of environment variables during build time.
apps/registry/app/explore/page.js (1)
26-29: LGTM! Good defensive programming practice
The early return with empty data when SUPABASE_KEY is not available is a good practice that:
- Prevents build-time errors
- Maintains consistent return structure
- Enables graceful degradation
| // This makes the page static at build time | ||
| export const dynamic = 'force-dynamic'; |
There was a problem hiding this comment.
Fix incorrect comment about page behavior
The comment "This makes the page static at build time" contradicts the actual behavior of force-dynamic, which ensures the page is dynamically rendered at runtime. This comment could mislead developers.
-// This makes the page static at build time
+// This ensures the page is dynamically rendered at runtime
export const dynamic = 'force-dynamic';📝 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.
| // This makes the page static at build time | |
| export const dynamic = 'force-dynamic'; | |
| // This ensures the page is dynamically rendered at runtime | |
| export const dynamic = 'force-dynamic'; |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/registry/app/api/resumes/route.js (1)
34-38: Consider optimizing the search query constructionWhile the switch to
textSearchwith websearch capabilities is an improvement, consider extracting the search configuration into a constant to improve maintainability.+ const SEARCH_CONFIG = { + config: 'english', + type: 'websearch' + }; if (search && search.trim() !== '') { - query.textSearch('resume', search.trim(), { - config: 'english', - type: 'websearch', - }); + query.textSearch('resume', search.trim(), SEARCH_CONFIG); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/registry/app/api/resumes/route.js(2 hunks)
🔇 Additional comments (3)
apps/registry/app/api/resumes/route.js (3)
7-8: LGTM: Dynamic export prevents build-time static optimization
The addition of dynamic = 'force-dynamic' is correct and necessary to ensure the route is always dynamically evaluated at runtime, which aligns with the PR's objective to fix build issues.
11-17: LGTM: Proper build-time environment variable check
The addition of the SUPABASE_KEY check with a 503 response is a good practice that:
- Prevents runtime errors during build
- Provides a clear message to API consumers
- Uses the appropriate HTTP status code
Line range hint 100-105: LGTM: Proper global error handling
The global error handling is well implemented with:
- Error logging for debugging
- Generic error message for security
- Appropriate status code
| try { | ||
| const resume = JSON.parse(row.resume); | ||
| return { | ||
| username: row.username, | ||
| label: resume?.basics?.label, | ||
| image: | ||
| resume?.basics?.image || | ||
| gravatar.url( | ||
| resume?.basics?.email || '', | ||
| { | ||
| s: '200', | ||
| r: 'x', | ||
| d: 'retro', | ||
| }, | ||
| true | ||
| ), | ||
| name: resume?.basics?.name, | ||
| location: resume?.basics?.location, | ||
| updated_at: row.updated_at, | ||
| created_at: row.created_at, | ||
| }; | ||
| } catch (e) { | ||
| console.error('Error parsing resume:', e); | ||
| return { | ||
| username: row.username, | ||
| label: 'Error parsing resume', | ||
| image: gravatar.url('', { s: '200', r: 'x', d: 'retro' }, true), | ||
| name: row.username, | ||
| location: null, | ||
| updated_at: row.updated_at, | ||
| created_at: row.created_at, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor duplicate gravatar URL construction
The gravatar URL construction logic is duplicated between the success and error cases. Consider extracting this into a helper function.
+ const getGravatarUrl = (email = '') => {
+ return gravatar.url(
+ email,
+ { s: '200', r: 'x', d: 'retro' },
+ true
+ );
+ };
try {
const resume = JSON.parse(row.resume);
return {
username: row.username,
label: resume?.basics?.label,
- image: resume?.basics?.image || gravatar.url(
- resume?.basics?.email || '',
- { s: '200', r: 'x', d: 'retro' },
- true
- ),
+ image: resume?.basics?.image || getGravatarUrl(resume?.basics?.email),
// ... rest of the properties
};
} catch (e) {
return {
username: row.username,
label: 'Error parsing resume',
- image: gravatar.url('', { s: '200', r: 'x', d: 'retro' }, true),
+ image: getGravatarUrl(),
// ... rest of the properties
};
}📝 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.
| try { | |
| const resume = JSON.parse(row.resume); | |
| return { | |
| username: row.username, | |
| label: resume?.basics?.label, | |
| image: | |
| resume?.basics?.image || | |
| gravatar.url( | |
| resume?.basics?.email || '', | |
| { | |
| s: '200', | |
| r: 'x', | |
| d: 'retro', | |
| }, | |
| true | |
| ), | |
| name: resume?.basics?.name, | |
| location: resume?.basics?.location, | |
| updated_at: row.updated_at, | |
| created_at: row.created_at, | |
| }; | |
| } catch (e) { | |
| console.error('Error parsing resume:', e); | |
| return { | |
| username: row.username, | |
| label: 'Error parsing resume', | |
| image: gravatar.url('', { s: '200', r: 'x', d: 'retro' }, true), | |
| name: row.username, | |
| location: null, | |
| updated_at: row.updated_at, | |
| created_at: row.created_at, | |
| }; | |
| } | |
| const getGravatarUrl = (email = '') => { | |
| return gravatar.url( | |
| email, | |
| { s: '200', r: 'x', d: 'retro' }, | |
| true | |
| ); | |
| }; | |
| try { | |
| const resume = JSON.parse(row.resume); | |
| return { | |
| username: row.username, | |
| label: resume?.basics?.label, | |
| image: resume?.basics?.image || getGravatarUrl(resume?.basics?.email), | |
| name: resume?.basics?.name, | |
| location: resume?.basics?.location, | |
| updated_at: row.updated_at, | |
| created_at: row.created_at, | |
| }; | |
| } catch (e) { | |
| console.error('Error parsing resume:', e); | |
| return { | |
| username: row.username, | |
| label: 'Error parsing resume', | |
| image: getGravatarUrl(), | |
| name: row.username, | |
| location: null, | |
| updated_at: row.updated_at, | |
| created_at: row.created_at, | |
| }; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
apps/registry/app/jobs/page.js (3)
9-10: Consider moving Supabase URL to environment variablesThe Supabase URL is currently hardcoded. Consider moving it to environment variables for better configuration management across different environments.
-const supabaseUrl = 'https://itxuhvvwryeuzuyihpkp.supabase.co'; +const supabaseUrl = process.env.SUPABASE_URL || 'https://itxuhvvwryeuzuyihpkp.supabase.co';
21-23: Consider extracting date calculation logicThe 90-day calculation logic could be extracted into a named function for better readability and reusability.
+const getNinetyDaysAgo = () => { + const date = new Date(); + date.setDate(date.getDate() - 90); + return date; +}; -const ninetyDaysAgo = new Date(); -ninetyDaysAgo.setDate(ninetyDaysAgo.getDate() - 90); +const ninetyDaysAgo = getNinetyDaysAgo();
24-28: Consider implementing pagination for better performanceThe current query fetches all jobs from the last 90 days without pagination, which could lead to performance issues as the dataset grows.
Consider adding pagination:
- const { data: jobs, error } = await supabase + const { data: jobs, error } = await supabase .from('jobs') .select('*') .gte('created_at', ninetyDaysAgo.toISOString()) - .order('created_at', { ascending: false }); + .order('created_at', { ascending: false }) + .range(0, 49); // Fetch first 50 results
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/registry/app/jobs/page.js(2 hunks)
🔇 Additional comments (2)
apps/registry/app/jobs/page.js (2)
14-16: LGTM! Good handling of missing SUPABASE_KEY
The early return with empty jobs array when SUPABASE_KEY is missing is a good practice for build-time handling.
56-56: Verify ClientJobBoard component interface
Ensure that the ClientJobBoard component properly handles the case when initialJobs is an empty array.
✅ Verification successful
Empty array handling is properly implemented
The verification shows that:
ClientJobBoardcomponent properly initializes state withinitialJobsand handles it in the filtering logicgetJobsfunction explicitly handles the empty array case by returning{ jobs: [] }when SUPABASE_KEY is not available- The component uses proper state management with
useStatehooks for both the original jobs and filtered jobs lists
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check ClientJobBoard implementation for empty array handling
ast-grep --pattern 'export default function ClientJobBoard({ initialJobs }) {
$$$
}'
Length of output: 88
Script:
#!/bin/bash
# Search for ClientJobBoard component implementation with ripgrep
rg "ClientJobBoard" -A 10 -B 2
Length of output: 2322
Script:
#!/bin/bash
# Let's check the getJobs function implementation to understand what it returns
rg "async function getJobs" -A 15
Length of output: 1067
| const supabaseKey = process.env.SUPABASE_KEY; | ||
| const supabase = createClient(supabaseUrl, supabaseKey); | ||
|
|
||
| // This ensures the page is dynamic at runtime |
There was a problem hiding this comment.
Add export configuration for Next.js dynamic behavior
The comment indicates the page should be dynamic, but the configuration is missing.
Add the following configuration:
+export const dynamic = 'force-dynamic';📝 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.
| // This ensures the page is dynamic at runtime | |
| // This ensures the page is dynamic at runtime | |
| export const dynamic = 'force-dynamic'; |
Summary by CodeRabbit
New Features
Bug Fixes
Documentation