feat: useRouter add query support #45
Conversation
WalkthroughThe changes enhance the routing functionality of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant Utils
User->>Router: navigateTo({ query: { bar: 'bar' } })
Router->>Utils: setQuery(baseUrl, query)
Utils->>Utils: Construct query string
Utils-->>Router: Return updated URL
Router-->>User: Navigate to updated URL
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 Configuration 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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/useRouter/index.ts (4 hunks)
- src/useRouter/readme.md (2 hunks)
- src/utils.test.ts (1 hunks)
- src/utils.ts (1 hunks)
Additional context used
Biome
src/useRouter/index.ts
[error] 110-110: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (20)
src/utils.test.ts (5)
5-6: LGTM!The test case correctly checks if the
setParamsfunction constructs the URL with valid parameters.The code changes are approved.
7-7: LGTM!The test case correctly checks if the
setParamsfunction handles undefined parameters.The code changes are approved.
8-8: LGTM!The test case correctly checks if the
setParamsfunction handles undefined and null parameters.The code changes are approved.
9-9: LGTM!The test case correctly checks if the
setParamsfunction handles undefined and zero parameters.The code changes are approved.
10-10: LGTM!The test case correctly checks if the
setParamsfunction handles undefined and boolean parameters.The code changes are approved.
src/useRouter/readme.md (6)
11-11: LGTM!The import statements are correctly added to support the new functionality.
The code changes are approved.
35-36: LGTM!The code correctly demonstrates how to navigate to a specific page while carrying parameters.
The code changes are approved.
41-42: LGTM!The code correctly demonstrates how to navigate to a specific page while carrying parameters.
The code changes are approved.
50-51: LGTM!The computed property correctly extracts the
idfromcurrentParams.The code changes are approved.
54-54: LGTM!The reactive reference
datais correctly initialized.The code changes are approved.
56-61: LGTM!The
watchEffectfunction correctly fetches data from an external API using the computedid.The code changes are approved.
src/useRouter/index.ts (9)
5-6: LGTM!The imports for
tryOnLoad,pathResolve, andsetParamsare appropriate and necessary for the new functionality.The code changes are approved.
89-91: LGTM!The
NavigateToOptionsinterface correctly extendsUniNamespace.NavigateToOptionsand includes an optionalparamsproperty.The code changes are approved.
93-95: LGTM!The
RedirectToOptionsinterface correctly extendsUniNamespace.RedirectToOptionsand includes an optionalparamsproperty.The code changes are approved.
108-110: LGTM!The
navigateTofunction correctly uses thehandleParamsfunction to process theparamsproperty.The code changes are approved.
Tools
Biome
[error] 110-110: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
114-116: LGTM!The
redirectTofunction correctly uses thehandleParamsfunction to process theparamsproperty.The code changes are approved.
196-197: LGTM!The
currentParamsreactive reference is correctly added to store the current parameters.The code changes are approved.
198-200: LGTM!The
currentParamsvalue is correctly set during the loading phase.The code changes are approved.
203-208: LGTM!The
navigateandredirectfunctions correctly accept the new options types and use the updated functions.The code changes are approved.
221-222: LGTM!The
currentParamsis correctly added to the returned object.The code changes are approved.
src/utils.ts
Outdated
| export function setParams(baseUrl: string, params: Record<string, any>): string { | ||
| if (!Object.keys(params).length) { | ||
| return baseUrl; | ||
| } | ||
|
|
||
| let parameters = ''; | ||
|
|
||
| for (const key in params) { | ||
| if (params[key] === undefined || params[key] === null || params[key] === '') { | ||
| continue; | ||
| } // 检查每个参数值是否有效 | ||
| parameters += `${key}=${encodeURIComponent(params[key])}&`; | ||
| } | ||
|
|
||
| // 移除末尾多余的 '&' 并构建最终的 URL | ||
| parameters = parameters.replace(/&$/, ''); | ||
| if (!parameters.length) { | ||
| return baseUrl; | ||
| } | ||
| return (/\?$/.test(baseUrl)) | ||
| ? baseUrl + parameters | ||
| : baseUrl.replace(/\/?$/, '?') + parameters; | ||
| } |
There was a problem hiding this comment.
Improve readability and efficiency.
The function can be improved by using URLSearchParams for constructing the query string.
Apply this diff to improve the function:
export function setParams(baseUrl: string, params: Record<string, any>): string {
const url = new URL(baseUrl, 'http://dummy-base');
const searchParams = new URLSearchParams();
for (const key in params) {
if (params[key] !== undefined && params[key] !== null && params[key] !== '') {
searchParams.append(key, params[key]);
}
}
url.search = searchParams.toString();
return url.toString().replace('http://dummy-base', '');
}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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function setParams(baseUrl: string, params: Record<string, any>): string { | |
| if (!Object.keys(params).length) { | |
| return baseUrl; | |
| } | |
| let parameters = ''; | |
| for (const key in params) { | |
| if (params[key] === undefined || params[key] === null || params[key] === '') { | |
| continue; | |
| } // 检查每个参数值是否有效 | |
| parameters += `${key}=${encodeURIComponent(params[key])}&`; | |
| } | |
| // 移除末尾多余的 '&' 并构建最终的 URL | |
| parameters = parameters.replace(/&$/, ''); | |
| if (!parameters.length) { | |
| return baseUrl; | |
| } | |
| return (/\?$/.test(baseUrl)) | |
| ? baseUrl + parameters | |
| : baseUrl.replace(/\/?$/, '?') + parameters; | |
| } | |
| export function setParams(baseUrl: string, params: Record<string, any>): string { | |
| const url = new URL(baseUrl, 'http://dummy-base'); | |
| const searchParams = new URLSearchParams(); | |
| for (const key in params) { | |
| if (params[key] !== undefined && params[key] !== null && params[key] !== '') { | |
| searchParams.append(key, params[key]); | |
| } | |
| } | |
| url.search = searchParams.toString(); | |
| return url.toString().replace('http://dummy-base', ''); | |
| } |
There was a problem hiding this comment.
@zhuddan, that makes perfect sense. Ensuring compatibility with mini-program environments is crucial, and the current implementation is indeed more suitable given the limitations. Thanks for the clarification!
src/useRouter/index.ts
Outdated
| /** | ||
| * 处理 options.params | ||
| * @param options | ||
| */ | ||
| function handleParams<T extends NavigateToOptions | RedirectToOptions>(options: T) { | ||
| if (options.params && typeof options.url === 'string') { | ||
| options.url = setParams(options.url, options.params); | ||
| delete options.params; | ||
| } | ||
| return options; | ||
| } |
There was a problem hiding this comment.
Avoid using the delete operator for performance reasons.
The handleParams function correctly processes the params property. However, the delete operator can impact performance. Consider setting the property to undefined instead.
Apply this diff to avoid using the delete operator:
function handleParams<T extends NavigateToOptions | RedirectToOptions>(options: T) {
if (options.params && typeof options.url === 'string') {
options.url = setParams(options.url, options.params);
- delete options.params;
+ options.params = undefined;
}
return options;
}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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 处理 options.params | |
| * @param options | |
| */ | |
| function handleParams<T extends NavigateToOptions | RedirectToOptions>(options: T) { | |
| if (options.params && typeof options.url === 'string') { | |
| options.url = setParams(options.url, options.params); | |
| delete options.params; | |
| } | |
| return options; | |
| } | |
| /** | |
| * 处理 options.params | |
| * @param options | |
| */ | |
| function handleParams<T extends NavigateToOptions | RedirectToOptions>(options: T) { | |
| if (options.params && typeof options.url === 'string') { | |
| options.url = setParams(options.url, options.params); | |
| options.params = undefined; | |
| } | |
| return options; | |
| } |
There was a problem hiding this comment.
@zhuddan 感谢你根据建议更新了代码！一切看起来都很棒。😊
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/useRouter/index.ts (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/useRouter/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/useRouter/index.ts (4 hunks)
- src/useRouter/readme.md (2 hunks)
- src/utils.test.ts (1 hunks)
- src/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/useRouter/readme.md
- src/utils.test.ts
Additional comments not posted (4)
src/utils.ts (1)
66-66: No changes detected.The function
isThenableremains unchanged and is correctly implemented.src/useRouter/index.ts (3)
89-95: Interface extensions are well-implemented.The extensions to
NavigateToOptionsandRedirectToOptionsto include aqueryproperty are correctly implemented and align with the new routing functionality.
101-107: FunctionhandleQueryis correctly implemented.The function processes the
queryproperty effectively, appending query parameters to the URL and then cleaning up by settingqueryto undefined.
196-201: Reactive referencecurrentQueryis a useful addition.The introduction of
currentQueryto manage and store the current query parameters enhances the state management within the router.
| export function setQuery(baseUrl: string, query: Record<string, any>): string { | ||
| if (!Object.keys(query).length) { | ||
| return baseUrl; | ||
| } | ||
|
|
||
| let parameters = ''; | ||
|
|
||
| for (const key in query) { | ||
| if (query[key] === undefined || query[key] === null || query[key] === '') { | ||
| continue; | ||
| } // 检查每个参数值是否有效 | ||
| parameters += `${key}=${encodeURIComponent(query[key])}&`; | ||
| } | ||
|
|
||
| // 移除末尾多余的 '&' 并构建最终的 URL | ||
| parameters = parameters.replace(/&$/, ''); | ||
| if (!parameters.length) { | ||
| return baseUrl; | ||
| } | ||
| return (/\?$/.test(baseUrl)) | ||
| ? baseUrl + parameters | ||
| : baseUrl.replace(/\/?$/, '?') + parameters; | ||
| } |
There was a problem hiding this comment.
Consider handling array parameters.
The function setQuery is well-implemented for basic key-value pairs. However, consider supporting array parameters, which are common in query strings. This can be done by checking if the value is an array and then iterating over it to append each value with the same key.
Here's a suggested implementation for handling array parameters:
for (const key in query) {
if (query[key] === undefined || query[key] === null || query[key] === '') {
continue;
}
+ if (Array.isArray(query[key])) {
+ query[key].forEach(value => {
+ parameters += `${key}[]=${encodeURIComponent(value)}&`;
+ });
+ } else {
parameters += `${key}=${encodeURIComponent(query[key])}&`;
+ }
}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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function setQuery(baseUrl: string, query: Record<string, any>): string { | |
| if (!Object.keys(query).length) { | |
| return baseUrl; | |
| } | |
| let parameters = ''; | |
| for (const key in query) { | |
| if (query[key] === undefined || query[key] === null || query[key] === '') { | |
| continue; | |
| } // 检查每个参数值是否有效 | |
| parameters += `${key}=${encodeURIComponent(query[key])}&`; | |
| } | |
| // 移除末尾多余的 '&' 并构建最终的 URL | |
| parameters = parameters.replace(/&$/, ''); | |
| if (!parameters.length) { | |
| return baseUrl; | |
| } | |
| return (/\?$/.test(baseUrl)) | |
| ? baseUrl + parameters | |
| : baseUrl.replace(/\/?$/, '?') + parameters; | |
| } | |
| export function setQuery(baseUrl: string, query: Record<string, any>): string { | |
| if (!Object.keys(query).length) { | |
| return baseUrl; | |
| } | |
| let parameters = ''; | |
| for (const key in query) { | |
| if (query[key] === undefined || query[key] === null || query[key] === '') { | |
| continue; | |
| } // 检查每个参数值是否有效 | |
| if (Array.isArray(query[key])) { | |
| query[key].forEach(value => { | |
| parameters += `${key}[]=${encodeURIComponent(value)}&`; | |
| }); | |
| } else { | |
| parameters += `${key}=${encodeURIComponent(query[key])}&`; | |
| } | |
| } | |
| // 移除末尾多余的 '&' 并构建最终的 URL | |
| parameters = parameters.replace(/&$/, ''); | |
| if (!parameters.length) { | |
| return baseUrl; | |
| } | |
| return (/\?$/.test(baseUrl)) | |
| ? baseUrl + parameters | |
| : baseUrl.replace(/\/?$/, '?') + parameters; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/useRouter/index.ts (4 hunks)
Additional comments not posted (6)
src/useRouter/index.ts (6)
89-91: Interface Extension Approved:NavigateToOptionsThe addition of the optional
queryproperty enhances the flexibility of navigation options by supporting query parameters.The changes to the interface are approved.
93-95: Interface Extension Approved:RedirectToOptionsThe addition of the optional
queryproperty toRedirectToOptionsis consistent with the changes made toNavigateToOptions, enhancing the redirection capabilities with query parameters.The changes to the interface are approved.
101-108: Effective Query Handling inhandleQueryFunctionThe function
handleQueryeffectively integrates query parameters into the URL and cleans up the options object by setting thequeryproperty to undefined after use. This approach ensures that the URL is correctly formatted with query parameters and that the options object remains clean.The implementation of
handleQueryis approved.
110-112: Integration of Query Handling innavigateToFunctionThe function
navigateToeffectively integrates thehandleQueryfunction to process query parameters before navigating. This ensures that the navigation is performed with the correct URL and query parameters.The changes to
navigateToare approved.
116-118: Consistent Query Handling inredirectToFunctionThe function
redirectTonow consistently handles query parameters, similar tonavigateTo, ensuring that redirections are performed with the correct URL and query parameters.The changes to
redirectToare approved.
198-202: Addition of Reactive ReferencecurrentQueryApprovedThe new reactive reference
currentQueryenhances state management within the router by allowing current query parameters to be stored and accessed reactively. This is a valuable addition, especially for pages that need to react to changes in query parameters dynamically.The addition of
currentQueryis approved.
zhuddan
changed the title
|
暂无此应用场景，不过既然 query 最终还是得拼接到 url 上，那么就没有必要额外多加一个 query 选项，而是让用户自己去拼接URL。 仅需要监听 onload 的query参数即可，如你的下面这段。 const currentQuery = ref<Record<string, any>>({});
tryOnLoad((options) => {
currentQuery.value = options ?? {};
});移除 query 参数后，关于你的 setQuery 函数，由于此库是 此函数建议可以改进成这样： /**
* 封装带有查询参数的 URL
* @param baseUrl - 基础 URL
* @param query - 要附加到 URL 的查询参数对象
* @returns 返回附加了查询参数的完整 URL
*/
export function buildUrl(baseUrl: string, query: Record<string, any>): string {
if (!Object.keys(query).length) {
return baseUrl;
}
const pieces: string[] = [];
for (const key in query) {
if (query[key] === undefined || query[key] === null || query[key] === '') {
// 用户传入了 key，理应附加此 key，而非移除
pieces.push(`${key}=`);
} else {
pieces.push(`${key}=${encodeURIComponent(query[key])}`);
}
}
// 使用数组 join 方法，而不是使用字符串拼接，以提高性能
const parameters = pieces.join('&');
if (!parameters.length) {
return baseUrl;
}
return baseUrl.includes('?') // 可不用正则解决的需求尽量不要用正则。
? `${baseUrl}&${parameters}`
: `${baseUrl}?${parameters}`;
} |
感谢您仔细审阅并提供反馈。我理解现有应用场景下，直接让用户拼接 URL 确实更为简洁高效，而不需要额外增加 query 选项。我会遵循您的建议，仅在 onload 时监听 query 参数。既然如此，我会关闭这个PR。非常感谢您的指导，今后我会更加注意确保方案与仓库的目标一致。 |
描述
这个 PR 新增了 vue router 的 query 参数模式。
额外上下文
我明白这个模式不符合
uni-app的默认跳转方式，但我习惯了 Vue 原生路由的参数传递方式，因此非常需要这个功能。它支持在路由跳转时携带参数，并在页面中获取这些参数。以下是一个使用示例：尽管这个模式与
uni-app的跳转机制有所冲突，但它极大地提高了我的开发效率。关于测试，由于单元测试不支持getCurrentPages，目前尚未找到合适的解决方案，因此我只测试了utils中新增的函数。文档部分已根据新功能进行了相应修改。请仔细审查这个 PR 的变更内容，如果可以，希望能够合并，因为这个功能对我的项目非常重要。
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation