Skip to content

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

qianYuanJ
Copy link

@qianYuanJ qianYuanJ commented Aug 7, 2025

修复 app/backend-mock 模块中的所有类型报错

Summary by CodeRabbit

  • Bug Fixes

    • Improved authentication and token handling for more reliable user sessions.
    • More robust table sorting and query handling.
    • Endpoints now return consistent success/error responses.
  • Chores

    • Cleanup and explicit dependency declarations for improved maintainability.
    • Updated TypeScript configuration for better build and tooling support.
  • Refactor

    • Standardized API response formatting across mock endpoints.
  • Known Issues

    • Department creation endpoint is currently non-functional due to an unimplemented handler.

Copy link

changeset-bot bot commented Aug 7, 2025

⚠️ No Changeset found

Latest commit: e830df9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Aug 7, 2025

Walkthrough

Standardizes explicit imports from 'h3', adds/uses response helper wrappers, tightens JWT/user handling and type-safety, updates tsconfig, and introduces a breaking local unimplemented eventHandler in one dept endpoint.

Changes

Cohort / File(s) Change Summary
Auth API Handlers
apps/backend-mock/api/auth/codes.ts, apps/backend-mock/api/auth/login.post.ts, apps/backend-mock/api/auth/logout.post.ts, apps/backend-mock/api/auth/refresh.post.ts
Added explicit h3 imports and response helpers (useResponseSuccess, useResponseError), added/mock user lookups and access/refresh token generation/validation; responses now wrapped with response utilities; no signature changes.
Demo, Menu, Upload & User Handlers
apps/backend-mock/api/demo/bigint.ts, apps/backend-mock/api/menu/all.ts, apps/backend-mock/api/user/info.ts, apps/backend-mock/api/upload.ts
Added explicit h3 imports, response wrappers and MOCK_* data imports; verifies tokens and returns success/unauthorized via response helpers; core logic unchanged.
System Dept Handlers
apps/backend-mock/api/system/dept/.post.ts, apps/backend-mock/api/system/dept/[id].delete.ts, apps/backend-mock/api/system/dept/[id].put.ts, apps/backend-mock/api/system/dept/list.ts
Added explicit h3 imports for handlers; critical: .post.ts introduces a local function eventHandler(...) that throws "Function not implemented.", shadowing the real handler and breaking the endpoint.
System Menu & Role Handlers
apps/backend-mock/api/system/menu/list.ts, apps/backend-mock/api/system/menu/name-exists.ts, apps/backend-mock/api/system/menu/path-exists.ts, apps/backend-mock/api/system/role/list.ts
Added h3 imports (eventHandler, getQuery) and response helpers; uses useResponseSuccess for consistent responses; no signature changes.
Table Handler
apps/backend-mock/api/table/list.ts
Added h3 imports and response helpers; improved type-safety for sorting (explicit ItemType, guarded property checks), clarified numeric vs string sort logic, and kept paginated response via helper.
Status & Test, Middleware, Routes
apps/backend-mock/api/status.ts, apps/backend-mock/api/test.get.ts, apps/backend-mock/api/test.post.ts, apps/backend-mock/middleware/1.api.ts, apps/backend-mock/routes/[...].ts
Added explicit h3 imports (eventHandler, defineEventHandler, getQuery, setResponseStatus) and response helper usage; behavior unchanged beyond using helpers and explicit imports.
TS Config
apps/backend-mock/tsconfig.json
Replaced previous extend with @vben/tsconfig/node.json, added JSON schema, set compilerOptions (composite, buildInfo, baseUrl, paths alias "~/utils/*"), noEmit:false, and expanded include patterns.
Utils
apps/backend-mock/utils/cookie-utils.ts, apps/backend-mock/utils/jwt-utils.ts, apps/backend-mock/utils/response.ts
Added explicit h3 imports for cookie/header/status helpers; jwt-utils: safer header parsing, JWT verify casting, MOCK_USERS lookup and null-guards; response util now calls setResponseStatus for 401/403. No public API signature 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)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • mynetfan
  • anncwb
  • jinmao88

Poem

A rabbit hops in with imports made clear,
Tokens and mocks now spoken quite near.
Responses wrapped tidy, types trimmed with care,
One shadowed handler cries — "Not implemented" there.
🐇✨

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 local eventHandler function and add the missing import.

The local eventHandler function at lines 16-18 shadows the proper eventHandler 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 return undefined, 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 operations

Repeated string literals ('jwt') make future changes error-prone, and an explicit path: '/' 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 opportunity

The new imports are correct; while touching this area, you could tighten pathMap’s value type to string (currently any) 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 like useResponseSuccess. 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

📥 Commits

Reviewing files that changed from the base of the PR and between b93e22c and 5a9e2d9.

📒 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 good

Explicitly 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 good

Same 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 from h3, 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 of eventHandler (vs defineEventHandler) 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 the forbiddenResponse and unAuthorizedResponse 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 and getQuery 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 codes
  • MOCK_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 to useResponseSuccess(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 of useResponseSuccess 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 of useResponseSuccess 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 of useResponseSuccess 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, and useResponseSuccess 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 and useResponseSuccess 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, and MOCK_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 generation

These 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 and getQuery from 'h3' and adding sleep to the response utilities import enhance type safety and align with the standardization pattern across the backend-mock API routes.

@qianYuanJ qianYuanJ changed the title fix(@vben/backend-mock): 修复所有 ts 类型报错 fix(@vben/backend-mock): 修复该模块所有 ts 类型报错 Aug 7, 2025
@qianYuanJ qianYuanJ closed this Aug 7, 2025
@qianYuanJ qianYuanJ reopened this Aug 7, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5a9e2d9 and e830df9.

📒 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 — LGTM

Consistent use of h3's eventHandler/getQuery and centralized response utilities. Matches project conventions.

Also applies to: 4-8

Comment on lines +54 to +63
// 检查 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 断言为合法键

Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant