refactoring to move to app router #96
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe recent updates enhance the registry application with new error handling utilities, data formatters, and resume generation functions. The schema for resumes is defined, enabling fetching and formatting resume data from GitHub. API endpoint handlers have been refactored for improved performance and maintainability, with updated dependencies and import paths. Changes
Tip New Features and ImprovementsReview SettingsIntroduced new personality profiles for code reviews. Users can now select between "Chill" and "Assertive" review tones to tailor feedback styles according to their preferences. The "Assertive" profile posts more comments and nitpicks the code more aggressively, while the "Chill" profile is more relaxed and posts fewer comments. AST-based InstructionsCodeRabbit offers customizing reviews based on the Abstract Syntax Tree (AST) pattern matching. Read more about AST-based instructions in the documentation. Community-driven AST-based RulesWe are kicking off a community-driven initiative to create and share AST-based rules. Users can now contribute their AST-based rules to detect security vulnerabilities, code smells, and anti-patterns. Please see the ast-grep-essentials repository for more information. New Static Analysis ToolsWe are continually expanding our support for static analysis tools. We have added support for Tone SettingsUsers can now customize CodeRabbit to review code in the style of their favorite characters or personalities. Here are some of our favorite examples:
Revamped Settings PageWe have redesigned the settings page for a more intuitive layout, enabling users to find and adjust settings quickly. This change was long overdue; it not only improves the user experience but also allows our development team to add more settings in the future with ease. Going forward, the changes to Miscellaneous
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 8
Outside diff range and nitpick comments (2)
apps/registry/pages/api/themes.js (1)
Line range hint
5-9: Refactor to usefor...offor better readability and performance.- Object.keys(THEMES).forEach((value) => { - themes[value] = `https://registry.jsonresume.org/thomasdavis?theme=${value}`; - }); + for (const value of Object.keys(THEMES)) { + themes[value] = `https://registry.jsonresume.org/thomasdavis?theme=${value}`; + }apps/registry/lib/formatters/template.js (1)
Line range hint
67-90: Convert theformatfunction to an arrow function for consistency and modern JavaScript practices.- const format = async function (resume, options) { + const format = async (resume, options) => {This change aligns with modern JavaScript syntax and improves consistency across the codebase.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
apps/registry/package.jsonis excluded by!**/*.json
Files selected for processing (10)
- apps/registry/lib/error/buildError.js (1 hunks)
- apps/registry/lib/formatters/formatters.js (1 hunks)
- apps/registry/lib/formatters/template.js (1 hunks)
- apps/registry/lib/generateResume.js (1 hunks)
- apps/registry/lib/getResumeGist.js (1 hunks)
- apps/registry/lib/schema.js (1 hunks)
- apps/registry/pages/api/[payload].js (1 hunks)
- apps/registry/pages/api/letter.js (1 hunks)
- apps/registry/pages/api/theme/[theme].js (1 hunks)
- apps/registry/pages/api/themes.js (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/registry/lib/formatters/formatters.js
Additional Context Used
Biome (8)
apps/registry/lib/formatters/template.js (1)
67-90: This function expression can be turned into an arrow function.
apps/registry/lib/generateResume.js (1)
44-44: Change to an optional chain.
apps/registry/lib/getResumeGist.js (2)
14-14: Template literals are preferred over string concatenation.
41-42: Template literals are preferred over string concatenation.
apps/registry/pages/api/[payload].js (2)
33-35: Prefer for...of instead of forEach.
43-45: This else clause can be omitted because previous branches break early.
apps/registry/pages/api/letter.js (1)
42-42: This let declares a variable that is only assigned once.
apps/registry/pages/api/themes.js (1)
5-9: Prefer for...of instead of forEach.
Additional comments not posted (3)
apps/registry/pages/api/theme/[theme].js (1)
2-2: The updated import path enhances clarity and maintainability.apps/registry/lib/error/buildError.js (1)
1-43: Structured error handling enhances maintainability and debuggability.apps/registry/lib/schema.js (1)
1-521: Ensure the schema definitions are comprehensive and correctly formatted.The JSON schema is well-structured and includes detailed descriptions and appropriate data types for various resume sections. This will facilitate accurate data validation and enhance data integrity.
apps/registry/lib/generateResume.js
Outdated
| const generateResume = async (username, extension = 'template', query = {}) => { | ||
| const { theme } = query; | ||
| const formatter = formatters[extension]; | ||
|
|
||
| if (!EXTENSIONS.has(extension)) { | ||
| return buildError(ERROR_CODES.INVALID_EXTENSION); | ||
| } | ||
|
|
||
| if (!formatter) { | ||
| return buildError(ERROR_CODES.UNKNOWN_FORMATTER); | ||
| } | ||
|
|
||
| // retrieve the users github gist | ||
| const { error: gistError, resume } = await getResumeGist(username); | ||
|
|
||
| if (gistError) { | ||
| return buildError(gistError); | ||
| } | ||
|
|
||
| const v = new Validator(); | ||
| const validation = v.validate(resume, schema); | ||
|
|
||
| if (!validation.valid) { | ||
| return buildError(ERROR_CODES.RESUME_SCHEMA_ERROR, { | ||
| validation: validation.errors, | ||
| }); | ||
| } | ||
|
|
||
| let selectedTheme = theme || (resume.meta && resume.meta.theme) || 'elegant'; | ||
|
|
||
| selectedTheme = selectedTheme.toLowerCase(); | ||
|
|
||
| // @todo - using as a resume cache for extra features | ||
| (async () => { | ||
| await supabase | ||
| .from('resumes') | ||
| .upsert( | ||
| { | ||
| username, | ||
| resume: JSON.stringify(resume), | ||
| updated_at: new Date(), | ||
| }, | ||
| { onConflict: 'username' } | ||
| ) | ||
| .select(); | ||
| })(); | ||
|
|
||
| const options = { ...query, theme: selectedTheme, username }; | ||
|
|
||
| let formatted = {}; | ||
|
|
||
| try { | ||
| formatted = await formatter.format(resume, options); | ||
| } catch (e) { | ||
| console.error(e); | ||
| // @todo - do this better | ||
| if (e.message === 'theme-missing') { | ||
| return buildError(ERROR_CODES.TEMPLATE_MISSING); | ||
| } | ||
|
|
||
| return buildError(ERROR_CODES.UNKNOWN_TEMPLATE_ERROR); | ||
| } | ||
|
|
||
| return { content: formatted.content, headers: formatted.headers || [] }; | ||
| }; |
There was a problem hiding this comment.
Refactor the generateResume function to improve readability and maintainability.
- const generateResume = async (username, extension = 'template', query = {}) => {
+ async function generateResume(username, extension = 'template', query = {}) {This change makes the function declaration more traditional and may improve readability for some developers.
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.
| const generateResume = async (username, extension = 'template', query = {}) => { | |
| const { theme } = query; | |
| const formatter = formatters[extension]; | |
| if (!EXTENSIONS.has(extension)) { | |
| return buildError(ERROR_CODES.INVALID_EXTENSION); | |
| } | |
| if (!formatter) { | |
| return buildError(ERROR_CODES.UNKNOWN_FORMATTER); | |
| } | |
| // retrieve the users github gist | |
| const { error: gistError, resume } = await getResumeGist(username); | |
| if (gistError) { | |
| return buildError(gistError); | |
| } | |
| const v = new Validator(); | |
| const validation = v.validate(resume, schema); | |
| if (!validation.valid) { | |
| return buildError(ERROR_CODES.RESUME_SCHEMA_ERROR, { | |
| validation: validation.errors, | |
| }); | |
| } | |
| let selectedTheme = theme || (resume.meta && resume.meta.theme) || 'elegant'; | |
| selectedTheme = selectedTheme.toLowerCase(); | |
| // @todo - using as a resume cache for extra features | |
| (async () => { | |
| await supabase | |
| .from('resumes') | |
| .upsert( | |
| { | |
| username, | |
| resume: JSON.stringify(resume), | |
| updated_at: new Date(), | |
| }, | |
| { onConflict: 'username' } | |
| ) | |
| .select(); | |
| })(); | |
| const options = { ...query, theme: selectedTheme, username }; | |
| let formatted = {}; | |
| try { | |
| formatted = await formatter.format(resume, options); | |
| } catch (e) { | |
| console.error(e); | |
| // @todo - do this better | |
| if (e.message === 'theme-missing') { | |
| return buildError(ERROR_CODES.TEMPLATE_MISSING); | |
| } | |
| return buildError(ERROR_CODES.UNKNOWN_TEMPLATE_ERROR); | |
| } | |
| return { content: formatted.content, headers: formatted.headers || [] }; | |
| }; | |
| async function generateResume(username, extension = 'template', query = {}) { | |
| const { theme } = query; | |
| const formatter = formatters[extension]; | |
| if (!EXTENSIONS.has(extension)) { | |
| return buildError(ERROR_CODES.INVALID_EXTENSION); | |
| } | |
| if (!formatter) { | |
| return buildError(ERROR_CODES.UNKNOWN_FORMATTER); | |
| } | |
| // retrieve the users github gist | |
| const { error: gistError, resume } = await getResumeGist(username); | |
| if (gistError) { | |
| return buildError(gistError); | |
| } | |
| const v = new Validator(); | |
| const validation = v.validate(resume, schema); | |
| if (!validation.valid) { | |
| return buildError(ERROR_CODES.RESUME_SCHEMA_ERROR, { | |
| validation: validation.errors, | |
| }); | |
| } | |
| let selectedTheme = theme || (resume.meta && resume.meta.theme) || 'elegant'; | |
| selectedTheme = selectedTheme.toLowerCase(); | |
| // @todo - using as a resume cache for extra features | |
| (async () => { | |
| await supabase | |
| .from('resumes') | |
| .upsert( | |
| { | |
| username, | |
| resume: JSON.stringify(resume), | |
| updated_at: new Date(), | |
| }, | |
| { onConflict: 'username' } | |
| ) | |
| .select(); | |
| })(); | |
| const options = { ...query, theme: selectedTheme, username }; | |
| let formatted = {}; | |
| try { | |
| formatted = await formatter.format(resume, options); | |
| } catch (e) { | |
| console.error(e); | |
| // @todo - do this better | |
| if (e.message === 'theme-missing') { | |
| return buildError(ERROR_CODES.TEMPLATE_MISSING); | |
| } | |
| return buildError(ERROR_CODES.UNKNOWN_TEMPLATE_ERROR); | |
| } | |
| return { content: formatted.content, headers: formatted.headers || [] }; | |
| } |
| `https://api.github.com/users/${username}/gists?per_page=100`, | ||
| { | ||
| headers: { | ||
| ...(GITHUB_TOKEN ? { Authorization: 'Bearer ' + GITHUB_TOKEN } : {}), // If we have no token and are in development, we can still make some requests. |
There was a problem hiding this comment.
Use template literals for string concatenation.
- ...(GITHUB_TOKEN ? { Authorization: 'Bearer ' + GITHUB_TOKEN } : {}),
+ ...(GITHUB_TOKEN ? { Authorization: `Bearer ${GITHUB_TOKEN}` } : {}),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.
| ...(GITHUB_TOKEN ? { Authorization: 'Bearer ' + GITHUB_TOKEN } : {}), // If we have no token and are in development, we can still make some requests. | |
| ...(GITHUB_TOKEN ? { Authorization: `Bearer ${GITHUB_TOKEN}` } : {}), // If we have no token and are in development, we can still make some requests. |
| @@ -33,7 +33,7 @@ export default async function handler(req, res) { | |||
|
|
|||
| const api = new ChatGPTAPI({ | |||
| apiKey: process.env.OPENAI_API_KEY, | |||
| model: 'gpt-3.5-32k-turbo', | |||
| model: 'gpt-4o', | |||
There was a problem hiding this comment.
Use const for variables that are not reassigned.
- let prompt = [
+ const prompt = [Committable suggestion was skipped due low confidence.
|
@levino tried to move to I wouldn't want to make theme development any worse than it is so the main idea is probably to use this flag -> https://nextjs.org/docs/app/api-reference/next-config-js/serverComponentsExternalPackages On a previous note, I was considering using pages or app/page.js because when a theme errors, it makes it easy to pretty print a component to explain the error, as opposed to just a json error. This PR mostly cleans up the code, and makes errors more consistent. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!pnpm-lock.yaml,!**/*.yaml
Files selected for processing (10)
- .github/workflows/ci.yml (2 hunks)
- README.md (5 hunks)
- apps/registry/lib/generateResume.js (1 hunks)
- apps/registry/lib/getResumeGist.js (1 hunks)
- apps/registry/pages/api/[payload].js (1 hunks)
- apps/registry/pages/api/suggestions.js (1 hunks)
- apps/registry/playwright.config.js (1 hunks)
- apps/registry/src/ui/Layout.js (1 hunks)
- apps/registry/tests/render.spec.js (1 hunks)
- themes/papirus/.eslintrc (1 hunks)
Files skipped from review due to trivial changes (4)
- .github/workflows/ci.yml
- apps/registry/pages/api/suggestions.js
- apps/registry/tests/render.spec.js
- themes/papirus/.eslintrc
Files skipped from review as they are similar to previous changes (1)
- apps/registry/lib/generateResume.js
Additional Context Used
LanguageTool (8)
README.md (8)
Near line 7: Possible missing comma found.
Context: ...base templates, utils etc ## Apps All projects hosted on this domain, will be found in...
Rule ID: AI_HYDRA_LEO_MISSING_COMMA
Near line 96: Make sure that the noun ‘setup’ is correct. Did you mean the past participle “set up”?
Context: ...d be most applicable to you. It is not setup to be automated at the moment, and the ...
Rule ID: BE_VB_OR_NN
Near line 96: For conciseness, consider replacing this expression with an adverb.
Context: ...o you. It is not setup to be automated at the moment, and the formatting is garbage. Each po...
Rule ID: AT_THE_MOMENT
Near line 102: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: .../thomasdavis/jobs) ### Letter This is a very simple service that prompts GPT with your resu...
Rule ID: EN_WEAK_ADJECTIVE
Near line 110: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...davis/letter) ### Suggestions This is a very simple service that prompts GPT with your resu...
Rule ID: EN_WEAK_ADJECTIVE
Near line 116: This word is normally spelled as one.
Context: ...terview This is an implementation of a chat bot, your resume is injected, and the conve...
Rule ID: EN_COMPOUNDS_CHAT_BOT
Near line 116: Did you mean “yourself”? Remove the space in between.
Context: ...ded in the prompt. So you can interview your self or talk as if you were being interviewe...
Rule ID: YOUR_SELF_TO_YOURSELF
Near line 120: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ocalhost:3000/thomasdavis/interview) # todo - AI tools - add an option to use yo...
Rule ID: TO_DO_HYPHEN
Markdownlint (8)
README.md (8)
120: null
Multiple top-level headings in the same document
18: null
Fenced code blocks should have a language specified
28: null
Fenced code blocks should have a language specified
34: null
Fenced code blocks should have a language specified
40: null
Fenced code blocks should have a language specified
46: null
Fenced code blocks should have a language specified
54: null
Fenced code blocks should have a language specified
80: null
Fenced code blocks should have a language specified
Biome (7)
apps/registry/lib/getResumeGist.js (1)
14-14: Template literals are preferred over string concatenation.
apps/registry/pages/api/[payload].js (1)
33-35: Prefer for...of instead of forEach.
apps/registry/playwright.config.js (4)
17-17: The computed expression can be simplified without the use of a string literal.
23-23: The computed expression can be simplified without the use of a string literal.
25-25: The computed expression can be simplified without the use of a string literal.
27-27: The computed expression can be simplified without the use of a string literal.
apps/registry/src/ui/Layout.js (1)
159-163: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing.
Additional comments not posted (11)
apps/registry/pages/api/[payload].js (5)
1-1: LGTM! ImportinggenerateResumealigns with the PR's focus on refactoring.
4-4: Refactored to use destructuring forreq.queryand handling ofpayloadto determineusernameandextension. This simplifies the code and makes it more readable.Also applies to: 9-9, 11-12
15-19: The use ofgenerateResumehere is a good example of modular code. It encapsulates the resume generation logic, which is a key part of this PR's objectives.
21-22: Proper error handling using theerrorobject returned fromgenerateResume. This ensures that errors are handled consistently, which is one of the PR's goals.
41-43: Simplified conditional rendering based onforceJSON. This makes the code cleaner and easier to understand.apps/registry/lib/getResumeGist.js (2)
1-1: Import statements are correctly updated to include necessary modules.
7-52: ThegetResumeGistfunction is well-structured and handles errors effectively usingbuildError. It also uses template literals consistently, which improves readability.apps/registry/playwright.config.js (1)
16-16: The configuration for the web server is correctly set to use environment variables for flexibility in different environments (like CI).README.md (3)
9-11: Updated links provide direct access to the relevant directories within the GitHub repository, improving the documentation's usability.
Line range hint
44-68: The environment variables section is clearly documented, which is crucial for setting up the project correctly.
81-81: The command for starting the homepage app is correctly documented, facilitating easier project setup.
| headers.forEach((header) => { | ||
| res.setHeader(header.key, header.value); | ||
| }); |
There was a problem hiding this comment.
Refactor to use for...of for iterating over headers.
- headers.forEach((header) => {
- res.setHeader(header.key, header.value);
- });
+ for (const header of headers) {
+ res.setHeader(header.key, header.value);
+ }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.
| headers.forEach((header) => { | |
| res.setHeader(header.key, header.value); | |
| }); | |
| for (const header of headers) { | |
| res.setHeader(header.key, header.value); | |
| } |
| @@ -158,7 +158,7 @@ export default function Layout({ children }) { | |||
|
|
|||
| <script | |||
| async | |||
| data-id="101412887" | |||
| data-id="101412017" | |||
There was a problem hiding this comment.
The script tag should be self-closing to adhere to JSX best practices.
- <script async data-id="101412017" src="https://static.getclicky.com/js"></script>
+ <script async data-id="101412017" src="https://static.getclicky.com/js" />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.
| data-id="101412017" | |
| <script async data-id="101412017" src="https://static.getclicky.com/js" /> |
No description provided.