Skip to content
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

reworking how template rendering works #92

Closed
wants to merge 1 commit into from

Conversation

thomasdavis
Copy link
Member

@thomasdavis thomasdavis commented May 9, 2024

mostly just to support fs again. (which took way too long to figure out)

but the abstractions will make more sense nontheless

not ready for review: pr'ing to show the line of thinking

  • move all lib like functionality out of the api folder
  • the api should serve all extension behavior .js, .html and the template rendering served as a component
  • remove locally cloned themes, now that fs works again
  • change docs to remove the idea of fs anyway (find relevant issue where @levino made a good point that it shouldn't be possible)

Copy link

changeset-bot bot commented May 9, 2024

⚠️ No Changeset found

Latest commit: c94d41e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented May 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
jsonresume-org-homepage2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 9, 2024 2:25pm
jsonresume-org-registry ❌ Failed (Inspect) May 9, 2024 2:25pm

Copy link
Contributor

coderabbitai bot commented May 9, 2024

Walkthrough

The recent updates focus on enhancing the functionality of a Next.js application for generating and handling resumes. Modifications include adjustments to webpack configurations, API route changes, and the introduction of new files for fetching, validating, and formatting resumes based on GitHub gists. Additionally, there's simplification in theme exports and an expansion in handling different file types and themes for resume formatting.

Changes

File Path Change Summary
apps/registry/next.config.js Updated webpack configuration to handle fs module and set webpack5: true. API route adjustment.
apps/registry/pages/[payload].js Added functions for static props, paths, and initial props in resume handling.
apps/registry/pages/api/... Introduced files for resume formatting and validation. Enhanced theme and formatter handling.
packages/.../index.js Simplified theme export in jsonresume-theme-spartacus.

Warning

Review ran into problems

Problems (1)
  • Git: Failed to clone repository. Please contact CodeRabbit support.

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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between c84329e and c94d41e.
Files ignored due to path filters (2)
  • apps/registry/package.json is excluded by !**/*.json
  • pnpm-lock.yaml is excluded by !pnpm-lock.yaml, !**/*.yaml
Files selected for processing (6)
  • apps/registry/next.config.js (2 hunks)
  • apps/registry/pages/[payload].js (1 hunks)
  • apps/registry/pages/api/formatters/template.js (1 hunks)
  • apps/registry/pages/api/template.js (1 hunks)
  • apps/registry/pages/lib/template.js (1 hunks)
  • packages/jsonresume-theme-spartacus/index.js (1 hunks)
Files skipped from review due to trivial changes (2)
  • apps/registry/pages/api/formatters/template.js
  • packages/jsonresume-theme-spartacus/index.js
Additional comments not posted (12)
apps/registry/next.config.js (3)

8-8: Ensure the new rewrite destination aligns with the intended routing logic.

#!/bin/bash
# Description: Verify that the new rewrite destination '/:payload' is correctly handled in the server logic.

# Test: Search for handling of '/:payload' in server-side code. Expect: Relevant handling logic.
rg --type js --type ts $'/:payload'

46-46: Enabling webpack5 ensures compatibility with the latest features. Good update.


55-55: Setting fs to false in webpack config might affect modules that require file system access. Verify this change doesn't break any functionality.

#!/bin/bash
# Description: Check for any modules that might require file system access and are affected by setting `fs` to false.

# Test: Search for modules that require 'fs'. Expect: No critical dependencies affected.
rg --type js --type ts $'require\\("fs"\\)'
apps/registry/pages/[payload].js (4)

3-16: Ensure that the getStaticProps function correctly fetches and passes the necessary props. Consider adding error handling for the async operations.

#!/bin/bash
# Description: Verify that `getStaticProps` function is correctly fetching and passing props.

# Test: Search for usage of `getStaticProps` in the project to ensure it's correctly implemented.
rg --type js --type ts $'getStaticProps'

29-34: getStaticPaths implementation looks correct for dynamic routing. Ensure the paths and fallback settings align with your routing requirements.


36-58: The Resume component handles rendering based on the resume's validation state. Ensure that all edge cases are handled, such as null or undefined output fields.

#!/bin/bash
# Description: Verify that all edge cases are handled in the `Resume` component.

# Test: Search for usage of `Resume` component to ensure comprehensive error handling.
rg --type js --type ts $'Resume'

60-60: Export of Resume component is correctly implemented. Ensure it is properly imported and used in other parts of the application.

#!/bin/bash
# Description: Verify that the `Resume` component is properly imported and used throughout the application.

# Test: Search for imports of the `Resume` component. Expect: Correct usage in other parts.
rg --type js --type ts $'import .*Resume'
apps/registry/pages/lib/template.js (3)

1-8: Imports and initializations are correctly implemented. Ensure that all imported modules are used appropriately and that the environment variables are securely managed.

#!/bin/bash
# Description: Verify that all imported modules in `template.js` are used appropriately and that environment variables are securely managed.

# Test: Search for usage of imported modules and environment variable references. Expect: Secure and appropriate usage.
rg --type js --type ts $'import|require|process.env'

17-19: Proper error handling for missing GITHUB_TOKEN. Ensure that this check is consistent with the application's security requirements.


28-39: ERROR_CODES are well-defined and cover a range of potential issues. Ensure that these codes are used consistently throughout the application to handle errors.

#!/bin/bash
# Description: Verify that `ERROR_CODES` are used consistently throughout the application.

# Test: Search for usage of `ERROR_CODES` in the project. Expect: Consistent usage in error handling.
rg --type js --type ts $'ERROR_CODES'
apps/registry/pages/api/template.js (2)

1-12: Imports and initializations are correctly implemented. Ensure that all imported modules are used appropriately and that the environment variables are securely managed.

#!/bin/bash
# Description: Verify that all imported modules in `api/template.js` are used appropriately and that environment variables are securely managed.

# Test: Search for usage of imported modules and environment variable references. Expect: Secure and appropriate usage.
rg --type js --type ts $'import|require|process.env'

14-25: Proper error handling for missing GITHUB_TOKEN and well-defined FILE_TYPES. Ensure that these settings align with the application's security and functionality requirements.

Comment on lines +18 to +27
export async function getInitialProps(context) {
console.log('get intial props', context);
const theme = context.query.theme;

return {
props: {
theme,
},
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getInitialProps is deprecated in favor of getStaticProps and getServerSideProps in newer Next.js versions. Consider refactoring to use the newer methods for better future compatibility.

- export async function getInitialProps(context) {
-   console.log('get intial props', context);
-   const theme = context.query.theme;
-   return {
-     props: {
-       theme,
-     },
-   };
- }
+ // Consider using getStaticProps or getServerSideProps instead

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.

Suggested change
export async function getInitialProps(context) {
console.log('get intial props', context);
const theme = context.query.theme;
return {
props: {
theme,
},
};
}
// Consider using getStaticProps or getServerSideProps instead

Comment on lines +21 to +26
const failMessage = (message) => {
return (
message +
', message @ajaxdavis on twitter if you need help (or tag me in your gist comments @thomasdavis'
);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failMessage function adds a personal touch to error messages. Ensure that this approach aligns with the overall user experience strategy of the application.

- ', message @ajaxdavis on twitter if you need help (or tag me in your gist comments @thomasdavis'
+ ', please contact support for further assistance.'

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.

Suggested change
const failMessage = (message) => {
return (
message +
', message @ajaxdavis on twitter if you need help (or tag me in your gist comments @thomasdavis'
);
};
const failMessage = (message) => {
return (
message +
', please contact support for further assistance.'
);
};

Comment on lines +41 to +154
{
headers: {
...(GITHUB_TOKEN ? { Authorization: 'Bearer ' + GITHUB_TOKEN } : {}), // If we have no token and are in development, we can still make some requests.
},
}
);
} catch (e) {
console.log(e);
return { error: ERROR_CODES.INVALID_USERNAME };
}

if (!gistData.data) {
// this is probably not worth it
return { error: ERROR_CODES.INVALID_USERNAME };
}

const resumeGist = find(gistData.data, (f) => {
// @todo - this won't work when when they have more than a 100 gist, and their resume.json
return f.files['resume.json']; // @todo - enum the resume.json file convention
});

if (!resumeGist) {
return { error: ERROR_CODES.NON_EXISTENT_GIST };
}

const gistId = resumeGist.id;

// @todo - figure out a better cache buster/strategy
const rawGistUrl =
`https://gist.githubusercontent.com/${username}/${gistId}/raw?cachebust=` +
new Date().getTime();

let resumeResponse = {};

try {
resumeResponse = await axios({
method: 'GET',
headers: { 'content-type': 'application/json' },
url: rawGistUrl,
});
} catch (e) {
// @todo - If gist url is invalid, flush the gistid in cache
console.log({ e });
return { error: ERROR_CODES.GIST_UNKNOWN_ERROR };
}

// prioritize manual theme, value from resume, then default to elegant
const selectedTheme = (
theme ||
(resumeResponse.data.meta && resumeResponse.data.meta.theme) ||
'elegant'
) // @todo - enum themes or something
.toLowerCase();

const resume = resumeResponse?.data ?? sampleResume;

const validation = schemaValidator.validate(resume, schema);

if (!validation.valid) {
// @todo - massively improve the UI of why the users resume is not valid - HIGH PRIORITY
// ${JSON.stringify(validation.errors, null, 2)}
return { error: ERROR_CODES.RESUME_SCHEMA_ERROR };
}

// @todo - Here we are inserting the resume into the registries database which is just used as a cache and to look at data. Need to create a terms of service and privacy policy for this. - HIGH PRIORITY

(async () => {
const { data, error } = await supabase
.from('resumes')
.upsert(
{
username,
resume: JSON.stringify(resume),
updated_at: new Date(),
},
{ onConflict: 'username' }
)
.select();
// @todo - this shouldn't break the execution of this function, but the log is useful if it does
})();

const renderOptions = { theme: selectedTheme, username };

let output = null;

try {
output = await template.format(resume, renderOptions);
} catch (e) {
// @todo - make error lib
console.error(e); // Log the error on the server so we see it in the logs
// @todo - do this A-LOT better
if (e.message === 'theme-missing') {
return { error: ERROR_CODES.TEMPLATE_MISSING };
}

return { error: ERROR_CODES.UNKNOWN_TEMPLATE_ERROR };
}

return { output, valid: validation.valid };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main handler function is comprehensive and includes detailed error handling and logging. Review and ensure that all asynchronous operations are handled correctly and that error messages are clear and user-friendly.

- // @todo - create a logging library and pipe to a service probably
+ // Consider implementing a dedicated logging service to improve error tracking and handling.

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.

Suggested change
export default async function handler(username, options) {
const { theme } = options;
const formatter = template;
// @todo - create a logging library and pipe to a service probably
console.log('Fetching gistId');
console.log(`https://api.github.com/users/${username}/gists`);
let gistData = {};
try {
// returns a list of gists
gistData = await axios.get(
`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.
},
}
);
} catch (e) {
console.log(e);
return { error: ERROR_CODES.INVALID_USERNAME };
}
if (!gistData.data) {
// this is probably not worth it
return { error: ERROR_CODES.INVALID_USERNAME };
}
const resumeGist = find(gistData.data, (f) => {
// @todo - this won't work when when they have more than a 100 gist, and their resume.json
return f.files['resume.json']; // @todo - enum the resume.json file convention
});
if (!resumeGist) {
return { error: ERROR_CODES.NON_EXISTENT_GIST };
}
const gistId = resumeGist.id;
// @todo - figure out a better cache buster/strategy
const rawGistUrl =
`https://gist.githubusercontent.com/${username}/${gistId}/raw?cachebust=` +
new Date().getTime();
let resumeResponse = {};
try {
resumeResponse = await axios({
method: 'GET',
headers: { 'content-type': 'application/json' },
url: rawGistUrl,
});
} catch (e) {
// @todo - If gist url is invalid, flush the gistid in cache
console.log({ e });
return { error: ERROR_CODES.GIST_UNKNOWN_ERROR };
}
// prioritize manual theme, value from resume, then default to elegant
const selectedTheme = (
theme ||
(resumeResponse.data.meta && resumeResponse.data.meta.theme) ||
'elegant'
) // @todo - enum themes or something
.toLowerCase();
const resume = resumeResponse?.data ?? sampleResume;
const validation = schemaValidator.validate(resume, schema);
if (!validation.valid) {
// @todo - massively improve the UI of why the users resume is not valid - HIGH PRIORITY
// ${JSON.stringify(validation.errors, null, 2)}
return { error: ERROR_CODES.RESUME_SCHEMA_ERROR };
}
// @todo - Here we are inserting the resume into the registries database which is just used as a cache and to look at data. Need to create a terms of service and privacy policy for this. - HIGH PRIORITY
(async () => {
const { data, error } = await supabase
.from('resumes')
.upsert(
{
username,
resume: JSON.stringify(resume),
updated_at: new Date(),
},
{ onConflict: 'username' }
)
.select();
// @todo - this shouldn't break the execution of this function, but the log is useful if it does
})();
const renderOptions = { theme: selectedTheme, username };
let output = null;
try {
output = await template.format(resume, renderOptions);
} catch (e) {
// @todo - make error lib
console.error(e); // Log the error on the server so we see it in the logs
// @todo - do this A-LOT better
if (e.message === 'theme-missing') {
return { error: ERROR_CODES.TEMPLATE_MISSING };
}
return { error: ERROR_CODES.UNKNOWN_TEMPLATE_ERROR };
}
return { output, valid: validation.valid };
}
export default async function handler(username, options) {
const { theme } = options;
const formatter = template;
// Consider implementing a dedicated logging service to improve error tracking and handling.
console.log('Fetching gistId');
console.log(`https://api.github.com/users/${username}/gists`);
let gistData = {};
try {
// returns a list of gists
gistData = await axios.get(
`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.
},
}
);
} catch (e) {
console.log(e);
return { error: ERROR_CODES.INVALID_USERNAME };
}
if (!gistData.data) {
// this is probably not worth it
return { error: ERROR_CODES.INVALID_USERNAME };
}
const resumeGist = find(gistData.data, (f) => {
// @todo - this won't work when when they have more than a 100 gist, and their resume.json
return f.files['resume.json']; // @todo - enum the resume.json file convention
});
if (!resumeGist) {
return { error: ERROR_CODES.NON_EXISTENT_GIST };
}
const gistId = resumeGist.id;
// @todo - figure out a better cache buster/strategy
const rawGistUrl =
`https://gist.githubusercontent.com/${username}/${gistId}/raw?cachebust=` +
new Date().getTime();
let resumeResponse = {};
try {
resumeResponse = await axios({
method: 'GET',
headers: { 'content-type': 'application/json' },
url: rawGistUrl,
});
} catch (e) {
// @todo - If gist url is invalid, flush the gistid in cache
console.log({ e });
return { error: ERROR_CODES.GIST_UNKNOWN_ERROR };
}
// prioritize manual theme, value from resume, then default to elegant
const selectedTheme = (
theme ||
(resumeResponse.data.meta && resumeResponse.data.meta.theme) ||
'elegant'
) // @todo - enum themes or something
.toLowerCase();
const resume = resumeResponse?.data ?? sampleResume;
const validation = schemaValidator.validate(resume, schema);
if (!validation.valid) {
// @todo - massively improve the UI of why the users resume is not valid - HIGH PRIORITY
// ${JSON.stringify(validation.errors, null, 2)}
return { error: ERROR_CODES.RESUME_SCHEMA_ERROR };
}
// @todo - Here we are inserting the resume into the registries database which is just used as a cache and to look at data. Need to create a terms of service and privacy policy for this. - HIGH PRIORITY
(async () => {
const { data, error } = await supabase
.from('resumes')
.upsert(
{
username,
resume: JSON.stringify(resume),
updated_at: new Date(),
},
{ onConflict: 'username' }
)
.select();
// @todo - this shouldn't break the execution of this function, but the log is useful if it does
})();
const renderOptions = { theme: selectedTheme, username };
let output = null;
try {
output = await template.format(resume, renderOptions);
} catch (e) {
// @todo - make error lib
console.error(e); // Log the error on the server so we see it in the logs
// @todo - do this A-LOT better
if (e.message === 'theme-missing') {
return { error: ERROR_CODES.TEMPLATE_MISSING };
}
return { error: ERROR_CODES.UNKNOWN_TEMPLATE_ERROR };
}
return { output, valid: validation.valid };
}

Comment on lines +26 to +31
const failMessage = (message) => {
return (
message +
', message @ajaxdavis on twitter if you need help (or tag me in your gist comments @thomasdavis'
);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failMessage function adds a personal touch to error messages. Ensure that this approach aligns with the overall user experience strategy of the application.

- ', message @ajaxdavis on twitter if you need help (or tag me in your gist comments @thomasdavis'
+ ', please contact support for further assistance.'

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.

Suggested change
const failMessage = (message) => {
return (
message +
', message @ajaxdavis on twitter if you need help (or tag me in your gist comments @thomasdavis'
);
};
const failMessage = (message) => {
return (
message +
', please contact support for further assistance.'
);
};

Comment on lines +33 to +210

let resumeRes = {};

try {
const fullResumeGistUrl =
`https://gist.githubusercontent.com/${username}/${gistId}/raw?cachebust=` +
new Date().getTime();

resumeRes = await axios({
method: 'GET',
headers: { 'content-type': 'application/json' },
url: fullResumeGistUrl,
});
} catch (e) {
// If gist url is invalid, flush the gistid in cache
return res.status(400).send(failMessage('Cannot fetch gist, no idea why'));
}

let realTheme =
theme || (resumeRes.data.meta && resumeRes.data.meta.theme) || 'elegant';

realTheme = realTheme.toLowerCase();

const selectedResume = resumeRes?.data ?? sampleResume;
const validation = v.validate(selectedResume, schema);

if (!validation.valid) {
return res.status(400).send(
failMessage('Validation failed') +
`

Your resume does not conform to the schema, visit https://jsonresume.org/schema/ to double check why. But the error message below should contain all the information you need.

=====

${JSON.stringify(validation.errors, null, 2)}

=====
`
);
}

// @todo - using as a resume cache for extra features
(async () => {
const { data, error } = await supabase
.from('resumes')
.upsert(
{
username,
resume: JSON.stringify(selectedResume),
updated_at: new Date(),
},
{ onConflict: 'username' }
)
.select();
console.log({ data, error });
})();

const options = { ...req.query, theme: realTheme, username };
let formatted = '';
try {
formatted = await formatter.format(selectedResume, options);
} catch (e) {
console.error(e); // Log the error on the server so we see it in the logs
// @todo - do this better
if (e.message === 'theme-missing') {
return res
.status(400)
.send(
failMessage(
'This theme is currently unsupported. Please visit this Github issue to request it https://github.com/jsonresume/jsonresume.org/issues/36 (unfortunately we have recently (11/2023) disabled a bunch of legacy themes due to critical flaws in them, please request if you would like them back.)'
)
);
}
return res
.status(400)
.send(
failMessage(
'Cannot format resume, no idea why #likely-a-validation-error'
)
);
}

formatted.headers.forEach((header) => {
res.setHeader(header.key, header.value);
});

res.setHeader(
'Access-Control-Allow-Headers',
'Origin, X-Requested-With, Content-Type, Accept'
);
res.setHeader('Access-Control-Allow-Origin', '*');

if (formatted.content instanceof Buffer) {
// handles images/binary
formatted.content.pipe(res);
} else {
return res.status(200).send(formatted.content);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main handler function is comprehensive and includes detailed error handling and logging. Review and ensure that all asynchronous operations are handled correctly and that error messages are clear and user-friendly.

- // @todo - do this better
+ // Consider implementing a dedicated error handling strategy to improve clarity and user experience.

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.

Suggested change
export default async function handler(req, res) {
const { theme, payload } = req.query;
const v = new Validator();
const payloadSplit = payload.split('.');
const username = payloadSplit[0];
let fileType = 'template';
if (payloadSplit.length === 2) {
fileType = payloadSplit[1];
}
if (!FILE_TYPES.has(fileType)) {
return res.status(200).send(failMessage('not supported file type'));
}
const FORMATTERS = {
qr,
json,
tex,
txt,
template,
yaml,
};
const formatter = FORMATTERS[fileType];
if (!formatter) {
return res.status(200).send(failMessage('not supported formatter'));
}
if (
[
'favicon.ico',
'competition',
'stats',
'apple-touch-icon.png',
'apple-touch-icon-precomposed.png',
'robots.txt',
].indexOf(username) !== -1
) {
return res.send(null);
}
let gistId;
console.log('Fetching gistId');
console.log(`https://api.github.com/users/${username}/gists`);
let gistData = {};
try {
gistData = await axios.get(
`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.
},
}
);
} catch (e) {
console.log(e);
return res.send(failMessage('This is not a valid Github username'));
}
if (!gistData.data) {
return res.send(failMessage('This is not a valid Github username'));
}
const resumeUrl = find(gistData.data, (f) => {
return f.files['resume.json'];
});
if (!resumeUrl) {
return res.send(
failMessage('You have no gists named resume.json or your gist is private')
);
}
gistId = resumeUrl.id;
let resumeRes = {};
try {
const fullResumeGistUrl =
`https://gist.githubusercontent.com/${username}/${gistId}/raw?cachebust=` +
new Date().getTime();
resumeRes = await axios({
method: 'GET',
headers: { 'content-type': 'application/json' },
url: fullResumeGistUrl,
});
} catch (e) {
// If gist url is invalid, flush the gistid in cache
return res.status(400).send(failMessage('Cannot fetch gist, no idea why'));
}
let realTheme =
theme || (resumeRes.data.meta && resumeRes.data.meta.theme) || 'elegant';
realTheme = realTheme.toLowerCase();
const selectedResume = resumeRes?.data ?? sampleResume;
const validation = v.validate(selectedResume, schema);
if (!validation.valid) {
return res.status(400).send(
failMessage('Validation failed') +
`
Your resume does not conform to the schema, visit https://jsonresume.org/schema/ to double check why. But the error message below should contain all the information you need.
=====
${JSON.stringify(validation.errors, null, 2)}
=====
`
);
}
// @todo - using as a resume cache for extra features
(async () => {
const { data, error } = await supabase
.from('resumes')
.upsert(
{
username,
resume: JSON.stringify(selectedResume),
updated_at: new Date(),
},
{ onConflict: 'username' }
)
.select();
console.log({ data, error });
})();
const options = { ...req.query, theme: realTheme, username };
let formatted = '';
try {
formatted = await formatter.format(selectedResume, options);
} catch (e) {
console.error(e); // Log the error on the server so we see it in the logs
// @todo - do this better
if (e.message === 'theme-missing') {
return res
.status(400)
.send(
failMessage(
'This theme is currently unsupported. Please visit this Github issue to request it https://github.com/jsonresume/jsonresume.org/issues/36 (unfortunately we have recently (11/2023) disabled a bunch of legacy themes due to critical flaws in them, please request if you would like them back.)'
)
);
}
return res
.status(400)
.send(
failMessage(
'Cannot format resume, no idea why #likely-a-validation-error'
)
);
}
formatted.headers.forEach((header) => {
res.setHeader(header.key, header.value);
});
res.setHeader(
'Access-Control-Allow-Headers',
'Origin, X-Requested-With, Content-Type, Accept'
);
res.setHeader('Access-Control-Allow-Origin', '*');
if (formatted.content instanceof Buffer) {
// handles images/binary
formatted.content.pipe(res);
} else {
return res.status(200).send(formatted.content);
}
}
export default async function handler(req, res) {
const { theme, payload } = req.query;
const v = new Validator();
const payloadSplit = payload.split('.');
const username = payloadSplit[0];
let fileType = 'template';
if (payloadSplit.length === 2) {
fileType = payloadSplit[1];
}
if (!FILE_TYPES.has(fileType)) {
return res.status(200).send(failMessage('not supported file type'));
}
const FORMATTERS = {
qr,
json,
tex,
txt,
template,
yaml,
};
const formatter = FORMATTERS[fileType];
if (!formatter) {
return res.status(200).send(failMessage('not supported formatter'));
}
if (
[
'favicon.ico',
'competition',
'stats',
'apple-touch-icon.png',
'apple-touch-icon-precomposed.png',
'robots.txt',
].indexOf(username) !== -1
) {
return res.send(null);
}
let gistId;
console.log('Fetching gistId');
console.log(`https://api.github.com/users/${username}/gists`);
let gistData = {};
try {
gistData = await axios.get(
`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.
},
}
);
} catch (e) {
console.log(e);
return res.send(failMessage('This is not a valid Github username'));
}
if (!gistData.data) {
return res.send(failMessage('This is not a valid Github username'));
}
const resumeUrl = find(gistData.data, (f) => {
return f.files['resume.json'];
});
if (!resumeUrl) {
return res.send(
failMessage('You have no gists named resume.json or your gist is private')
);
}
gistId = resumeUrl.id;
let resumeRes = {};
try {
const fullResumeGistUrl =
`https://gist.githubusercontent.com/${username}/${gistId}/raw?cachebust=` +
new Date().getTime();
resumeRes = await axios({
method: 'GET',
headers: { 'content-type': 'application/json' },
url: fullResumeGistUrl,
});
} catch (e) {
// If gist url is invalid, flush the gistid in cache
return res.status(400).send(failMessage('Cannot fetch gist, no idea why'));
}
let realTheme =
theme || (resumeRes.data.meta && resumeRes.data.meta.theme) || 'elegant';
realTheme = realTheme.toLowerCase();
const selectedResume = resumeRes?.data ?? sampleResume;
const validation = v.validate(selectedResume, schema);
if (!validation.valid) {
return res.status(400).send(
failMessage('Validation failed') +
`
Your resume does not conform to the schema, visit https://jsonresume.org/schema/ to double check why. But the error message below should contain all the information you need.
=====
${JSON.stringify(validation.errors, null, 2)}
=====
`
);
}
// @todo - using as a resume cache for extra features
(async () => {
const { data, error } = await supabase
.from('resumes')
.upsert(
{
username,
resume: JSON.stringify(selectedResume),
updated_at: new Date(),
},
{ onConflict: 'username' }
)
.select();
console.log({ data, error });
})();
const options = { ...req.query, theme: realTheme, username };
let formatted = '';
try {
formatted = await formatter.format(selectedResume, options);
} catch (e) {
console.error(e); // Log the error on the server so we see it in the logs
// Consider implementing a dedicated error handling strategy to improve clarity and user experience.
if (e.message === 'theme-missing') {
return res
.status(400)
.send(
failMessage(
'This theme is currently unsupported. Please visit this Github issue to request it https://github.com/jsonresume/jsonresume.org/issues/36 (unfortunately we have recently (11/2023) disabled a bunch of legacy themes due to critical flaws in them, please request if you would like them back.)'
)
);
}
return res
.status(400)
.send(
failMessage(
'Cannot format resume, no idea why #likely-a-validation-error'
)
);
}
formatted.headers.forEach((header) => {
res.setHeader(header.key, header.value);
});
res.setHeader(
'Access-Control-Allow-Headers',
'Origin, X-Requested-With, Content-Type, Accept'
);
res.setHeader('Access-Control-Allow-Origin', '*');
if (formatted.content instanceof Buffer) {
// handles images/binary
formatted.content.pipe(res);
} else {
return res.status(200).send(formatted.content);
}
}

@thomasdavis
Copy link
Member Author

thomasdavis commented May 9, 2024

got stuck on this vercel/next.js#17269

PR in progress, only file worth checking out is apps/registry/pages/[payload].js

-1 nextjs

@levino
Copy link
Collaborator

levino commented May 9, 2024

What kind of URLs would you like to prerender?

@levino
Copy link
Collaborator

levino commented May 9, 2024

I just saw: You are using the pages router which is outdated. I suggest to use exclusively the app router. Then there also is no getStaticProps.

Even if you are using getStaticProps you cannot possibly want to generate a page for each possible username and theme. I think you should just server side render these pages.

@levino
Copy link
Collaborator

levino commented May 9, 2024

With the app router you can access the searchParams prop in the render function.

@thomasdavis
Copy link
Member Author

thomasdavis commented May 9, 2024

Moving to the app router makes sense. I will migrate it all there tomorrow.

Been trying to furiously refactor code because I have some spare time at the moment.

The end goal of this PR is to get the registry template/theme rendering working inside of pages/ and then any extensions for resume.json, resume.yaml etc outputted in the api folder.

Which allows the templates to use nodejs libraries such as fs, and then to logically move pure functional lib like functions outside of the api folder.

@levino
Copy link
Collaborator

levino commented May 9, 2024

With next.js you are in serverless / lambda land. There is no file system. Using fs will never work properly because the code will be bundled and the bundler does not understand that the files which you will later load via fs are to be bundled as well. My recommendation: Just drop support for all the templates which use fs under the hood. I think this will never work properly.

@thomasdavis
Copy link
Member Author

feel free to commit any code you want to my pr's, it don't bother me, i'm sure we both have adequate git-fu

@thomasdavis
Copy link
Member Author

I know we've talked about the fs support before, and I lean towards what your thinking. Will share my reflections tomorrow.

@levino
Copy link
Collaborator

levino commented May 9, 2024

It is a question of requirements. I cannot decide that for you. I am just saying you would make your life easier if you lowered the bar. If I can be of help with some code, let me know and I will see what I can do, but I will not support fs. It is just too painful imo.

@levino
Copy link
Collaborator

levino commented May 10, 2024

#58 is related

@thomasdavis
Copy link
Member Author

thomasdavis commented May 17, 2024

Closing in favor of #58

Keep the branch alive for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants