-
Notifications
You must be signed in to change notification settings - Fork 8k
fix(@vben/backend-mock): 修复该模块所有 ts 类型报错 #6613
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
base: main
Are you sure you want to change the base?
Conversation
|
WalkthroughStandardizes explicit imports from 'h3', adds/uses response helper wrappers, tightens JWT/user handling and type-safety, updates tsconfig, and introduces a breaking local unimplemented Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API (handler)
participant JWTUtils
participant MockData
participant ResponseHelpers
Client->>API: HTTP request (may include Authorization / cookies)
API->>JWTUtils: verifyAccessToken / verifyRefreshToken
JWTUtils->>MockData: lookup user by username (from token)
MockData-->>JWTUtils: user or null
JWTUtils-->>API: verification result (payload or null)
API->>ResponseHelpers: format success/error response
ResponseHelpers-->>Client: HTTP response (wrapped success/error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
apps/backend-mock/api/system/dept/.post.ts (1)
8-18
: Critical: Remove the broken localeventHandler
function and add the missing import.The local
eventHandler
function at lines 16-18 shadows the propereventHandler
from 'h3' and will cause runtime errors. This breaks the entire endpoint.Apply this fix to align with other API endpoints:
+import { eventHandler } from 'h3'; import { verifyAccessToken } from '~/utils/jwt-utils'; import { sleep, unAuthorizedResponse, useResponseSuccess, } from '~/utils/response'; export default eventHandler(async (event) => { const userinfo = verifyAccessToken(event); if (!userinfo) { return unAuthorizedResponse(event); } await sleep(600); return useResponseSuccess(null); }); -function eventHandler(_: (event: any) => Promise<any>) { - throw new Error('Function not implemented.'); -}apps/backend-mock/api/auth/refresh.post.ts (1)
24-29
: Handle potential undefined user lookup.Similar to the issue in
jwt-utils.ts
, the user lookup could returnundefined
, but the code assumes it will always find a user. Consider adding explicit handling:const findUser = MOCK_USERS.find( (item) => item.username === userinfo.username, ); if (!findUser) { return forbiddenResponse(event); }The current code already has this check, so this is actually correct. My apologies for the confusion.
🧹 Nitpick comments (4)
apps/backend-mock/utils/cookie-utils.ts (1)
3-4
: Consider extracting the cookie name and adding an explicit path for cleaner, more reliable operationsRepeated string literals (
'jwt'
) make future changes error-prone, and an explicitpath: '/'
guarantees the delete operation targets the same cookie that was set.+const REFRESH_TOKEN_COOKIE = 'jwt'; export function clearRefreshTokenCookie(event: H3Event<EventHandlerRequest>) { - deleteCookie(event, 'jwt', { + deleteCookie(event, REFRESH_TOKEN_COOKIE, { httpOnly: true, sameSite: 'none', secure: true, + path: '/', // ensure consistent scope }); } export function setRefreshTokenCookie( event: H3Event<EventHandlerRequest>, refreshToken: string, ) { - setCookie(event, 'jwt', refreshToken, { + setCookie(event, REFRESH_TOKEN_COOKIE, refreshToken, { httpOnly: true, maxAge: 24 * 60 * 60, // unit: seconds sameSite: 'none', secure: true, + path: '/', // mirror deletion path }); } export function getRefreshTokenFromCookie(event: H3Event<EventHandlerRequest>) { - const refreshToken = getCookie(event, 'jwt'); + const refreshToken = getCookie(event, REFRESH_TOKEN_COOKIE); return refreshToken; }apps/backend-mock/api/system/menu/path-exists.ts (1)
1-5
: Minor typing tidy-up opportunityThe new imports are correct; while touching this area, you could tighten
pathMap
’s value type tostring
(currentlyany
) for stronger compile-time safety.-const pathMap: Record<string, any> = { '/': 0 }; +const pathMap: Record<string, string> = { '/': '0' };This removes an
any
, aligns with the string coercions below, and maintains behaviour.apps/backend-mock/utils/jwt-utils.ts (1)
39-42
: Consider safer JWT verification approach.The double type assertion
as unknown as UserPayload
bypasses TypeScript's type safety. While this is common for JWT handling, consider adding runtime validation for critical properties.- const decoded = jwt.verify( - token, - ACCESS_TOKEN_SECRET, - ) as unknown as UserPayload; + const decoded = jwt.verify(token, ACCESS_TOKEN_SECRET) as unknown; + if (typeof decoded !== 'object' || decoded === null || !('username' in decoded)) { + return null; + } + const payload = decoded as UserPayload;apps/backend-mock/api/auth/refresh.post.ts (1)
34-34
: Consider consistent response wrapping.This endpoint returns a bare
accessToken
while other endpoints use response wrappers likeuseResponseSuccess
. Consider maintaining consistency:- return accessToken; + return useResponseSuccess({ accessToken });However, verify this doesn't break existing API consumers before making this change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/backend-mock/api/auth/codes.ts
(1 hunks)apps/backend-mock/api/auth/login.post.ts
(1 hunks)apps/backend-mock/api/auth/logout.post.ts
(1 hunks)apps/backend-mock/api/auth/refresh.post.ts
(1 hunks)apps/backend-mock/api/demo/bigint.ts
(1 hunks)apps/backend-mock/api/menu/all.ts
(1 hunks)apps/backend-mock/api/status.ts
(1 hunks)apps/backend-mock/api/system/dept/.post.ts
(1 hunks)apps/backend-mock/api/system/dept/[id].delete.ts
(1 hunks)apps/backend-mock/api/system/dept/[id].put.ts
(1 hunks)apps/backend-mock/api/system/dept/list.ts
(1 hunks)apps/backend-mock/api/system/menu/list.ts
(1 hunks)apps/backend-mock/api/system/menu/name-exists.ts
(1 hunks)apps/backend-mock/api/system/menu/path-exists.ts
(1 hunks)apps/backend-mock/api/system/role/list.ts
(1 hunks)apps/backend-mock/api/table/list.ts
(2 hunks)apps/backend-mock/api/test.get.ts
(1 hunks)apps/backend-mock/api/test.post.ts
(1 hunks)apps/backend-mock/api/upload.ts
(1 hunks)apps/backend-mock/api/user/info.ts
(1 hunks)apps/backend-mock/middleware/1.api.ts
(1 hunks)apps/backend-mock/routes/[...].ts
(1 hunks)apps/backend-mock/tsconfig.json
(1 hunks)apps/backend-mock/utils/cookie-utils.ts
(1 hunks)apps/backend-mock/utils/jwt-utils.ts
(3 hunks)apps/backend-mock/utils/response.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: the `/api/user/info` endpoint in the vue vben admin project is protected by middleware that handles ...
Learnt from: wll8
PR: vbenjs/vue-vben-admin#4018
File: apps/backend-mock/api/user.cjs:10-24
Timestamp: 2024-08-03T03:11:46.130Z
Learning: The `/api/user/info` endpoint in the Vue Vben Admin project is protected by middleware that handles user authentication and returns an error if the user is not found or authentication fails.
Applied to files:
apps/backend-mock/api/user/info.ts
apps/backend-mock/api/demo/bigint.ts
📚 Learning: in `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown...
Learnt from: ArthurDarkstone
PR: vbenjs/vue-vben-admin#4807
File: docs/src/components/common-ui/vben-vxe-table.md:65-84
Timestamp: 2024-11-05T10:34:13.846Z
Learning: In `docs/src/components/common-ui/vben-vxe-table.md`, the code examples are provided within markdown scripts for documentation and do not require import statements or explicit type annotations.
Applied to files:
apps/backend-mock/api/table/list.ts
🧬 Code Graph Analysis (1)
apps/backend-mock/utils/jwt-utils.ts (1)
apps/backend-mock/utils/mock-data.ts (2)
MOCK_USERS
(10-34)UserInfo
(1-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (24)
apps/backend-mock/api/system/dept/[id].delete.ts (1)
1-1
: Import addition looks goodExplicitly importing
eventHandler
resolves the TS type error and clarifies the dependency. No further issues spotted.apps/backend-mock/api/system/dept/[id].put.ts (1)
1-1
: Import addition looks goodSame rationale as the DELETE handler—dependency is now explicit and type-safe.
apps/backend-mock/middleware/1.api.ts (1)
1-1
: Import addition looks good
defineEventHandler
is now explicitly sourced fromh3
, eliminating the previous implicit global and fixing the type error.apps/backend-mock/api/test.get.ts (1)
1-1
: LGTM! Explicit import resolves TypeScript type error.The explicit import of
defineEventHandler
from 'h3' ensures proper TypeScript type resolution and aligns with the PR objective to fix all TypeScript type errors.apps/backend-mock/api/test.post.ts (1)
1-1
: LGTM! Explicit import resolves TypeScript type error.The explicit import of
defineEventHandler
from 'h3' ensures proper TypeScript type resolution, matching the pattern used across other API handlers in this PR.apps/backend-mock/api/system/dept/list.ts (1)
2-2
: LGTM! Explicit import resolves TypeScript type error.The explicit import of
eventHandler
from 'h3' properly addresses the TypeScript type resolution issue. The usage ofeventHandler
(vsdefineEventHandler
) is appropriate for this async handler implementation.apps/backend-mock/utils/response.ts (1)
3-3
: LGTM! Explicit import enables proper HTTP status handling.The explicit import of
setResponseStatus
from 'h3' resolves TypeScript type errors and enables theforbiddenResponse
andunAuthorizedResponse
functions to properly set HTTP status codes 403 and 401 respectively.apps/backend-mock/api/system/menu/list.ts (1)
1-1
: LGTM! Explicit import resolves TypeScript type error.The explicit import of
eventHandler
from 'h3' ensures proper TypeScript type resolution and follows the consistent pattern established across other system API handlers in this PR.apps/backend-mock/routes/[...].ts (1)
1-1
: Good TypeScript improvement!Adding the explicit import for
defineEventHandler
resolves potential TypeScript module resolution issues and improves code clarity. This aligns perfectly with the PR objective to fix all TS type errors.apps/backend-mock/api/system/role/list.ts (1)
2-2
: Excellent TypeScript type safety improvement!Adding explicit imports for
eventHandler
andgetQuery
from 'h3' resolves TypeScript module resolution issues. These functions are actively used in the handler (lines 42 and 57), so the explicit imports are essential for proper type checking.apps/backend-mock/api/auth/login.post.ts (1)
1-12
: Comprehensive TypeScript and response handling improvements!The added imports are all actively used throughout the handler:
setResponseStatus
(line 17) for proper HTTP status codesMOCK_USERS
(line 24) for user authentication- Response utilities (lines 18, 30, 38) for consistent API responses
This standardizes response handling and resolves TypeScript type errors effectively.
apps/backend-mock/api/status.ts (1)
1-2
: TypeScript imports properly added!All imported functions are actively used in the handler. The explicit imports resolve TypeScript module resolution issues and maintain consistency with other API endpoints.
apps/backend-mock/api/user/info.ts (3)
1-1
: Good TypeScript improvement!Adding the explicit
eventHandler
import resolves TypeScript module resolution issues and follows the consistent pattern across all API endpoints.
3-3
: Response standardization improvement!Adding
useResponseSuccess
to the import and using it on line 10 standardizes the response format with other API endpoints while maintaining the same user info data structure.
10-10
: Consistent response wrapper applied!The change from returning raw
userinfo
touseResponseSuccess(userinfo)
standardizes the response format across the API while preserving the authentication logic and user data structure.apps/backend-mock/api/demo/bigint.ts (1)
1-3
: LGTM! Import standardization improves type safety.The explicit imports from 'h3' and the response utilities align with the PR's objective to fix TypeScript type errors and maintain consistency across the backend-mock module.
apps/backend-mock/api/upload.ts (1)
1-3
: LGTM! Consistent with API-wide standardization.The explicit import of
eventHandler
and the addition ofuseResponseSuccess
align with the standardization pattern across the backend-mock API endpoints.apps/backend-mock/api/auth/codes.ts (1)
1-4
: LGTM! Proper standardization of imports and response handling.The explicit import of
eventHandler
from 'h3' and the use ofuseResponseSuccess
maintain consistency with the API-wide standardization effort.apps/backend-mock/api/system/menu/name-exists.ts (1)
1-4
: LGTM! Consistent with the standardization pattern.The explicit imports from 'h3' (
eventHandler
,getQuery
) and the addition ofuseResponseSuccess
align with the TypeScript type safety improvements across the backend-mock module.apps/backend-mock/api/menu/all.ts (1)
1-4
: LGTM! Explicit imports improve type safety.The addition of explicit imports for
eventHandler
,MOCK_MENUS
, anduseResponseSuccess
enhances TypeScript type checking and makes dependencies clear. This aligns well with the PR's objective to fix TypeScript type errors.apps/backend-mock/api/auth/logout.post.ts (1)
1-1
: LGTM! Standardized imports and response handling.The explicit imports for
defineEventHandler
anduseResponseSuccess
improve type safety and maintain consistency with other API endpoints in the backend-mock module.Also applies to: 6-6
apps/backend-mock/api/auth/refresh.post.ts (1)
1-1
: LGTM! Explicit imports improve type safety.The addition of explicit imports for
defineEventHandler
, JWT utilities, andMOCK_USERS
enhances type checking and makes dependencies clear.Also applies to: 7-8
apps/backend-mock/tsconfig.json (1)
2-13
: LGTM! Comprehensive TypeScript configuration improvements.The configuration changes are well-structured and support the codebase modernization:
- JSON schema reference improves IDE support
- Shared base config promotes consistency across the project
- Composite builds enhance performance in monorepo setups
- Path alias
~/utils/*
properly supports the import patterns used throughout the codebase- Setting
noEmit: false
allows proper build output generationThese changes effectively support the TypeScript error fixes implemented across the backend-mock module.
apps/backend-mock/api/table/list.ts (1)
2-8
: LGTM! Import standardization improves type safety.The explicit imports of
eventHandler
andgetQuery
from 'h3' and addingsleep
to the response utilities import enhance type safety and align with the standardization pattern across the backend-mock API routes.
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/backend-mock/api/table/list.ts (1)
85-85
: It looks like pagination params come in as strings (or even arrays) and are passed directly into the helper, which accepts number|string but will not normalize bad values. We should parse and default them before calling usePageResponseSuccess.Suggested fix in apps/backend-mock/api/table/list.ts (lines 83–85):
- const { page, pageSize } = getQuery(event) - return usePageResponseSuccess(page as string, pageSize as string, listData) + const { page: rawPage, pageSize: rawPageSize } = getQuery(event) + const pageNum = parseInt(Array.isArray(rawPage) ? rawPage[0] : rawPage ?? '', 10) + const pageSizeNum = parseInt( + Array.isArray(rawPageSize) ? rawPageSize[0] : rawPageSize ?? '', + 10 + ) + return usePageResponseSuccess( + Number.isFinite(pageNum) && pageNum > 0 ? pageNum : 1, + Number.isFinite(pageSizeNum) && pageSizeNum > 0 ? pageSizeNum : 10, + listData + )
- We normalize arrays by taking the first element.
- We fallback page to 1 and pageSize to 10 when parsing fails.
- This ensures we never pass
"undefined"
or"1,2"
to the helper.
♻️ Duplicate comments (1)
apps/backend-mock/api/table/list.ts (1)
66-71
: Fix inconsistent sorting (price asc bug, numeric/boolean/date handling, desc branch)
- Ascending sorts price lexicographically (string), but descending parses numerically. This yields wrong results (e.g., "100" < "20").
- Descending branch compares non-price fields as strings, breaking for numeric fields (quantity, rating, weight).
- Dates and booleans degrade to string comparison.
Unify comparator: detect numbers/numeric-like, Date, boolean; fallback to string with numeric-aware localeCompare. Invert by isDesc.
Apply this diff to the sort comparator:
- listData.sort((a, b) => { - if (sortOrder === 'asc') { - const aValue = a[sortKey]; - const bValue = b[sortKey]; - - return typeof aValue === 'number' && typeof bValue === 'number' - ? aValue - bValue - : String(aValue).localeCompare(String(bValue)); - } else { - if (sortKey === 'price') { - return ( - Number.parseFloat(b[sortKey] as string) - - Number.parseFloat(a[sortKey] as string) - ); - } else { - return (a[sortKey] as string) < (b[sortKey] as string) ? 1 : -1; - } - } - }); + listData.sort((a, b) => { + const aValue = a[sortKey] as unknown; + const bValue = b[sortKey] as unknown; + + // Date + if (aValue instanceof Date && bValue instanceof Date) { + const diff = aValue.getTime() - bValue.getTime(); + return isDesc ? -diff : diff; + } + + // Boolean + if (typeof aValue === 'boolean' && typeof bValue === 'boolean') { + const diff = Number(aValue) - Number(bValue); + return isDesc ? -diff : diff; + } + + // Number or numeric-like strings (e.g., price) + const aNum = + typeof aValue === 'number' ? aValue : Number.parseFloat(String(aValue)); + const bNum = + typeof bValue === 'number' ? bValue : Number.parseFloat(String(bValue)); + const bothNumeric = !Number.isNaN(aNum) && !Number.isNaN(bNum); + + if (bothNumeric) { + const diff = aNum - bNum; + return isDesc ? -diff : diff; + } + + // Fallback string compare (numeric-aware) + const diff = String(aValue).localeCompare(String(bValue), undefined, { + numeric: true, + sensitivity: 'base', + }); + return isDesc ? -diff : diff; + });Also applies to: 73-80
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backend-mock/api/table/list.ts
(2 hunks)apps/backend-mock/utils/jwt-utils.ts
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/backend-mock/utils/jwt-utils.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (1)
apps/backend-mock/api/table/list.ts (1)
2-2
: Imports standardized — LGTMConsistent use of h3's eventHandler/getQuery and centralized response utilities. Matches project conventions.
Also applies to: 4-8
// 检查 sortBy 是否是 listData 元素的合法属性键 | ||
if ( | ||
sortBy && | ||
listData[0] && | ||
Object.prototype.hasOwnProperty.call(listData[0], sortBy as string) | ||
) { | ||
// 定义数组元素的类型 | ||
type ItemType = (typeof listData)[0]; | ||
const sortKey = sortBy as keyof ItemType; // 将 sortBy 断言为合法键 | ||
|
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.
🛠️ Refactor suggestion
Normalize query params; avoid unsafe casts of sortBy/sortOrder
sortBy/sortOrder from getQuery can be string | string[]. Casting with as string risks "array to string" and bypasses type safety. Normalize first and derive isDesc once.
Apply this diff within this block:
- // 检查 sortBy 是否是 listData 元素的合法属性键
- if (
- sortBy &&
- listData[0] &&
- Object.prototype.hasOwnProperty.call(listData[0], sortBy as string)
- ) {
- // 定义数组元素的类型
- type ItemType = (typeof listData)[0];
- const sortKey = sortBy as keyof ItemType; // 将 sortBy 断言为合法键
+ // 规范化 query 入参,兼容 string[]
+ const sortKeyRaw = Array.isArray(sortBy) ? sortBy[0] : sortBy;
+ const sortOrderRaw = Array.isArray(sortOrder) ? sortOrder[0] : sortOrder;
+
+ // 检查 sortBy 是否是 listData 元素的合法属性键
+ if (
+ typeof sortKeyRaw === 'string' &&
+ listData[0] &&
+ Object.prototype.hasOwnProperty.call(listData[0], sortKeyRaw)
+ ) {
+ // 定义数组元素的类型
+ type ItemType = (typeof listData)[0];
+ const sortKey = sortKeyRaw as keyof ItemType; // 将 sortBy 断言为合法键
+ const isDesc = sortOrderRaw === 'desc';
🤖 Prompt for AI Agents
In apps/backend-mock/api/table/list.ts around lines 54 to 63, the code unsafely
casts sortBy/sortOrder from getQuery (which can be string | string[]) to string,
risking array-to-string bugs; normalize both params first (e.g., if
Array.isArray(sortBy) use the first element, same for sortOrder), validate/trim
the resulting strings, then compute a boolean isDesc by comparing normalized
sortOrder (case-insensitive) to "desc"; finally use the normalized sortBy (typed
as keyof ItemType after confirming it exists on listData[0]) and the isDesc flag
in the sorting logic instead of direct casts.
修复 app/backend-mock 模块中的所有类型报错
Summary by CodeRabbit
Bug Fixes
Chores
Refactor
Known Issues