-
Notifications
You must be signed in to change notification settings - Fork 421
Old docs redirects #2331
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
Old docs redirects #2331
Conversation
Summary of ChangesHello @gbarros, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the management of documentation redirects within the Next.js application. It establishes a centralized "redirects.json" file to define permanent URL mappings, ensuring that users accessing old or deprecated paths are seamlessly guided to the correct, updated content. This enhancement improves navigation, maintains link integrity, and supports better SEO for the documentation site. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
i wonder...if there's a way we can export this to work with static site 🤔 |
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.
Code Review
This pull request introduces a redirect mechanism by reading from a redirects.json file, which is a great way to manage redirects centrally. The implementation in next.config.mjs is mostly correct, but I've suggested an improvement to use asynchronous file reading and refine the error handling. This will make the build process more robust and non-blocking. The rest of the changes, including the new redirects.json file and the removal of output: 'export', look good and are consistent with the goal of enabling server-side redirects in Next.js.
| async redirects() { | ||
| try { | ||
| const redirectsPath = path.join(process.cwd(), 'redirects.json') | ||
| const redirectsData = fs.readFileSync(redirectsPath, 'utf8') | ||
| return JSON.parse(redirectsData) | ||
| } catch (error) { | ||
| console.error('Error reading redirects.json:', error) | ||
| return [] | ||
| } | ||
| } |
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.
This implementation can be improved in two ways:
- Use asynchronous file reading: Since the
redirectsfunction isasync, it's best practice to use the asynchronousfs.promises.readFileinstead of the synchronousfs.readFileSyncto avoid blocking the event loop during the build process. - Improve error handling: The current
catchblock logs an error even ifredirects.jsonis simply not found. It's better to handle theENOENT(file not found) error code gracefully and only log other unexpected errors (like JSON parsing errors).
async redirects() {
try {
const redirectsPath = path.join(process.cwd(), 'redirects.json');
const redirectsData = await fs.promises.readFile(redirectsPath, 'utf8');
return JSON.parse(redirectsData);
} catch (error) {
if (error.code === 'ENOENT') {
// It's fine if the redirects file doesn't exist, just return an empty array.
return [];
}
// For other errors (e.g., invalid JSON), log it and return an empty array.
console.error('Error reading or parsing redirects.json:', error);
return [];
}
}
jcstein
left a comment
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.
LGTM
|
does this commit add ability for this to work on deployed gh pages? https://github.com/celestiaorg/docs/pull/2331/files/7c0e5a29084e8329bedca12a58f6aa6812ff4199..f3be8068cc719a6916dd14b8625a03ea3ffec4b4 |
This pull request introduces support for custom redirects in the Next.js configuration by reading from a new
redirects.jsonfile. It also makes minor adjustments to the Next.js export options. The most important changes are grouped below:Redirects feature:
redirects()function tonext.config.mjsthat loads redirect rules from the newredirects.jsonfile, enabling centralized management of redirects.redirects.jsonfile, which contains a list of permanent redirect mappings for various routes.Configuration updates:
output: 'export'option from the Next.js configuration, possibly to improve compatibility with the new redirects setup.fsandpathmodules innext.config.mjsto support reading the redirects file.