refactor(routing): use router index file #463
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @boomwwww, 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!
I've undertaken a significant refactoring of the application's routing system. The primary goal was to centralize and streamline the routing logic, which was previously scattered across multiple files, including the main App.tsx component and a dedicated hook. This refactor introduces a new, dedicated router index file that encapsulates all routing definitions and navigation utilities. The changes also involve renaming key routing elements for better semantic clarity and relocating certain components to more appropriate directories based on their usage scope. This effort aims to enhance the overall maintainability, readability, and future extensibility of the application's navigation flow.
Highlights
- Centralized Routing Logic: I've consolidated all routing-related code into a new central file,
src/router/index.tsx. This includes the route definitions, the hash-based routing hook, and the navigation utility function. This change significantly improves the organization and maintainability of the routing logic, making it easier to understand and modify in the future. - Improved Naming Conventions: I've renamed several routing-related identifiers to improve their clarity and semantic meaning. Specifically,
useHashRouteis nowuseHashRouter,routersis nowroutes, andredirectis nowgoTo. These changes make the code more intuitive and align better with common naming conventions. - Component Relocation for Better Encapsulation: The
GameEditorProviderandTemplateEditorProvidercomponents, which were previously located insrc/components/Provider/, have been moved to their respective page directories (src/pages/editor/andsrc/pages/templateEditor/). This relocation reflects their specific usage within those pages rather than as globally reusable components, enhancing module encapsulation. - Modernized Navigation Button Implementation: The navigation buttons within
GameElementandTemplateElementcomponents have been updated. Instead of using<a>tags withhrefattributes for navigation, they now use standardButtoncomponents withonClickhandlers that call the newgoToutility function. This change provides more programmatic control over navigation and aligns with modern React practices for handling routing.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request is a nice refactoring that centralizes the routing logic into a dedicated src/router module, improving code organization and clarity. The renaming of functions and variables like useHashRoute to useHashRouter and redirect to goTo makes the code more semantic and easier to understand.
My review focuses on a pattern I noticed in the provider components (GameEditorProvider and TemplateEditorProvider). When these components perform a redirect, they don't stop their render execution, which can lead to unnecessary processing. I've suggested adding early returns to address this, which will make the components more efficient and robust.
| if (page !== 'game' || !gameDir) { | ||
| redirect('dashboard', 'game'); | ||
| goTo('dashboard', 'game'); | ||
| }; |
There was a problem hiding this comment.
When a component redirects, it's a good practice to stop its rendering process to avoid unnecessary computations and potential side effects from hooks that run after the redirection logic. Here, the component continues to render after goTo is called. You should add return null; to exit early. This also allows removing the redundant semicolon on line 15.
if (page !== 'game' || !gameDir) {
goTo('dashboard', 'game');
return null;
}| if (!fristLoading && !inGameList) { | ||
| redirect('dashboard', 'game'); | ||
| goTo('dashboard', 'game'); | ||
| } |
There was a problem hiding this comment.
Similar to the check above, you should return null after redirecting to prevent the component from rendering further in the current pass.
if (!fristLoading && !inGameList) {
goTo('dashboard', 'game');
return null;
}| if (page !== 'template' || !templateDir) { | ||
| redirect('dashboard', 'template'); | ||
| goTo('dashboard', 'template'); | ||
| } |
There was a problem hiding this comment.
When a component redirects, it's a good practice to stop its rendering process to avoid unnecessary computations. Here, the component continues to render after goTo is called. You should add return null; to exit early.
if (page !== 'template' || !templateDir) {
goTo('dashboard', 'template');
return null;
}| if (!fristLoading && !inTemplateList) { | ||
| redirect('dashboard', 'template'); | ||
| goTo('dashboard', 'template'); | ||
| } |
There was a problem hiding this comment.
Similar to the check above, you should return null after redirecting to prevent the component from rendering further in the current pass.
if (!fristLoading && !inTemplateList) {
goTo('dashboard', 'template');
return null;
}| const page = useEditorStore.use.page(); | ||
| const subPage = useEditorStore.use.subPage(); | ||
| if (page !== 'template' || !subPage) redirect('dashboard'); | ||
| if (page !== 'template' || !subPage) goTo('dashboard'); |
There was a problem hiding this comment.
To prevent the component from continuing its render cycle after redirection, you should return early. This avoids running subsequent hooks and logic unnecessarily.
if (page !== 'template' || !subPage) {
goTo('dashboard');
return null;
}
| <span className={styles.templateElement_dir}>{props.templateInfo.dir}</span> | ||
| <div className={styles.templateElement_action} onClick={(event) => event.stopPropagation()}> | ||
| <Button appearance='primary' as='a' href={`${routers.template.url}/${props.templateInfo.dir}`}> | ||
| <Button appearance='primary' onClick={() => goTo('template', props.templateInfo.dir)}> |
There was a problem hiding this comment.
先前的做法可以方便用户用鼠标中键在新标签页打开编辑界面
整理路由模块
src/router/index.tsx文件，这些代码原本在App组件内或在useHashRoute.ts文件内useHashRoute改为useHashRouter，将routers改为routes，将redirect改为goTo，如此更符合它们的语义GameEditorProvider和TemplateEditorProvider组件。原本它们在src/components/Provider/文件夹下，但这两个组件在全局只会使用一次，将它们放在src/components/文件夹下不太合理，因为这里通常存放会复用的组件，因此本人将它们分别移动到了src/pages/editor/文件夹下和src/pages/templateEditor/文件夹下