feat: stronger useRouter #38
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe project has introduced new TypeScript type utilities for enhanced property manipulation, deprecated the Changes
Poem
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.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (5)
- src/types.ts (1 hunks)
- src/useRoute/index.ts (1 hunks)
- src/useRouter/index.ts (1 hunks)
- src/useRouter/readme.md (1 hunks)
- src/utils.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- src/useRoute/index.ts
Additional comments: 20
src/types.ts (3)
- 11-11: The
Optionaltype is correctly defined and follows TypeScript's utility types pattern.- 13-13: The
RequiredPropertytype is correctly implemented, ensuring that the specified properties are required in the type.- 15-15: The
RequiredOnlytype is a proper combination ofPartialandRequiredProperty, which is a useful utility type for TypeScript.src/utils.ts (1)
- 17-17: The
pathResolvefunction uses a hardcoded base URL ('http://no-exists.com') to create a full URL object. Verify that this approach is compatible with the expected behavior in all use cases and does not introduce any side effects.src/useRouter/readme.md (1)
- 10-22: The documentation updates in the
useRouterreadme provide clear examples of how to use the new router functionality, including thetryTabBaroption andtabBarList.src/useRouter/index.ts (15)
- 28-41: The
initIfNotInitedfunction is correctly checking if the router has been initialized before setting up interceptors and refreshing the current pages.- 44-46: The
refreshCurrentPagesfunction correctly updates thepagesref with the current pages fromgetCurrentPages().- 48-60: The
warpPromiseOptionsfunction correctly wraps the options object with success and fail handlers that resolve or reject the promise.- 66-71: The
switchTabfunction is correctly returning a promise that resolves or rejects based on the outcome ofuni.switchTab.- 74-79: The
navigateTofunction is correctly returning a promise that resolves or rejects based on the outcome ofuni.navigateTo.- 82-87: The
redirectTofunction is correctly returning a promise that resolves or rejects based on the outcome ofuni.redirectTo.- 93-98: The
reLaunchfunction is correctly returning a promise that resolves or rejects based on the outcome ofuni.reLaunch.- 104-109: The
backfunction is correctly returning a promise that resolves or rejects based on the outcome ofuni.navigateBack.- 116-135: The
trySwitchTabfunction is correctly handling the logic to determine whether to useswitchTabor navigate functions based on thetryTabBarflag and the presence oftabBarList.- 139-142: The
isTabBarPathfunction is correctly determining if a given path is a tab bar path using thepathResolvefunction and thetabBarList.- 149-161: The
UseRouterOptionsinterface is correctly defined with optional properties fortryTabBarandtabBarList.- 170-188: The
useRouterfunction is correctly initializing the router if not already done and setting up a back press interceptor.- 193-195: The
navigatefunction is correctly usingtrySwitchTabto handle navigation with thetryTabBarlogic.- 200-201: The
redirectfunction is correctly usingtrySwitchTabto handle redirection with thetryTabBarlogic.- 202-233: The
useRouterfunction is correctly returning an object with properties and methods for managing routing, including deprecated properties with clear comments.
src/utils.ts
Outdated
Comment on lines
8
to
17
| export function pathResolve(target: string, current?: string) { | ||
| if (!current) { | ||
| const pages = getCurrentPages(); | ||
| current = pages[pages.length - 1].route; | ||
| } | ||
|
|
||
| if (!current) { | ||
| throw new Error('The current path is undefined and cannot be found.'); | ||
| } | ||
| return new URL(target, new URL(current, 'http://no-exists.com')).pathname; |
There was a problem hiding this comment.
The pathResolve function assumes that getCurrentPages() will always return an array with at least one page, which may not be the case. It should handle the scenario where getCurrentPages() is empty.
- current = pages[pages.length - 1].route;
+ current = pages.length > 0 ? pages[pages.length - 1].route : undefined;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 function pathResolve(target: string, current?: string) { | |
| if (!current) { | |
| const pages = getCurrentPages(); | |
| current = pages[pages.length - 1].route; | |
| } | |
| if (!current) { | |
| throw new Error('The current path is undefined and cannot be found.'); | |
| } | |
| return new URL(target, new URL(current, 'http://no-exists.com')).pathname; | |
| export function pathResolve(target: string, current?: string) { | |
| if (!current) { | |
| const pages = getCurrentPages(); | |
| current = pages.length > 0 ? pages[pages.length - 1].route : undefined; | |
| } | |
| if (!current) { | |
| throw new Error('The current path is undefined and cannot be found.'); | |
| } | |
| return new URL(target, new URL(current, 'http://no-exists.com')).pathname; | |
| } |
Comment on lines
+14
to
+17
| if (!current) { | ||
| throw new Error('The current path is undefined and cannot be found.'); | ||
| } | ||
| return new URL(target, new URL(current, 'http://no-exists.com')).pathname; |
There was a problem hiding this comment.
The error message in the pathResolve function is misleading because it suggests that the current path is always undefined, which may not be the case. The error should be thrown only if getCurrentPages() returns an empty array.
- throw new Error('The current path is undefined and cannot be found.');
+ throw new Error('No pages found in the current stack.');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
| if (!current) { | |
| throw new Error('The current path is undefined and cannot be found.'); | |
| } | |
| return new URL(target, new URL(current, 'http://no-exists.com')).pathname; | |
| if (!current) { | |
| throw new Error('No pages found in the current stack.'); | |
| } | |
| return new URL(target, new URL(current, 'http://no-exists.com')).pathname; |
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
package.jsonis excluded by:!**/*.json
Files selected for processing (3)
- build.config.ts (1 hunks)
- src/useRouter/index.ts (1 hunks)
- src/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/utils.ts
Additional comments: 16
build.config.ts (1)
- 7-9: The addition of
@dcloudio/uni-cli-sharedto theexternalsarray in the build configuration is noted and appears to be correctly implemented.src/useRouter/index.ts (15)
- 2-5: The new imports for
pathResolve,AppJson,RequiredOnly, andonBackPressare correctly added to the file.- 28-42: The
initIfNotInitedfunction is well implemented to ensure the router is only initialized once.- 44-46: The
refreshCurrentPagesfunction correctly updates thepagesref with the current pages.- 48-60: The
warpPromiseOptionsfunction is correctly implemented to wrap the options with success and fail handlers.- 66-71: The
switchTabfunction is correctly implemented as a Promise, encapsulating theuni.switchTabcall.- 74-79: The
navigateTofunction is correctly implemented as a Promise, encapsulating theuni.navigateTocall.- 82-87: The
redirectTofunction is correctly implemented as a Promise, encapsulating theuni.redirectTocall.- 93-98: The
reLaunchfunction is correctly implemented as a Promise, encapsulating theuni.reLaunchcall.- 104-109: The
backfunction is correctly implemented as a Promise, encapsulating theuni.navigateBackcall.- 116-135: The
trySwitchTabfunction is correctly implemented to handle conditional tab bar navigation.- 139-142: The
isTabBarPathfunction is correctly implemented to check if a path corresponds to a tab bar item.- 149-161: The
UseRouterOptionsinterface is correctly updated to include the newtryTabBarandtabBarListproperties.- 170-188: The modifications to the
useRouterfunction, including the initialization logic and theonBackPressevent handler, are correctly implemented.- 193-202: The
navigateandredirectfunctions are correctly implemented to utilize thetrySwitchTablogic.- 209-233: The return object of the
useRouterfunction correctly exposes the new navigation methods and deprecates the old properties.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description 描述
Reopen of #36
Linked Issues 关联的 Issues
Additional context 额外上下文
Summary by CodeRabbit
New Features
Documentation
useRoutedocumentation, recommendinguseRouter().currentUrlfor current routing needs.Refactor
useRouterto streamline routing functionality.Bug Fixes
pathResolvefunction to improve navigation reliability.