Skip to content

fix: Fix the incompatibility issues caused by version upgrade #7750

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.

Sign up for GitHub

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

Merged
wangdan-fit2cloud merged 1 commit into from
Nov 9, 2026
Merged

fix: Fix the incompatibility issues caused by version upgrade #7750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"dingtalk-jsapi": "^3.2.9",
"dompurify": "^3.4.14",
"echarts": "^6.1.0",
"element-plus": "^2.14.7",
"element-plus": "2.14.8",
"highlight.js": "^11.12.0",
"jsencrypt": "^3.5.4",
"katex": "^0.16.47",
Expand All @@ -51,7 +51,7 @@
"recorder-core": "^1.3.26070800",
"screenfull": "^6.0.2",
"uuid": "^14.0.2",
"vue": "^3.5.38",
"vue": "3.5.45",
"vue-clipboard3": "^2.0.0",
"vue-codemirror": "^6.1.1",
"vue-demi": "^0.14.10",
Expand Down
9 changes: 6 additions & 3 deletions ui/src/components/global/mk-dropdown/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { Fragment, isVNode, ref } from 'vue'
import { Fragment, isVNode, nextTick, ref } from 'vue'
import type { DropdownInstance } from 'element-plus'
import type { Options } from '@popperjs/core'
import { hasRenderableSlotContent } from '@/utils/vnode'
Expand All @@ -18,8 +18,11 @@ const dropdownRef = ref<DropdownInstance>()
// 保留模式也延迟到首次展开，避免列表初始化时挂载每一行的菜单及业务浮层。
const hasOpened = ref(false)

function handleVisibleChange(visible: boolean) {
if (visible) hasOpened.value = true
async function handleVisibleChange(visible: boolean) {
if (!visible || hasOpened.value) return
// 首次入场钩子执行后再切换 persistent，避免菜单停在透明的初始状态。
await nextTick()
hasOpened.value = true
}

const slots = defineSlots<{

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a concise review of the code changes with potential issues and optimizations:

1. Potential Issue: Race Condition & Performance

  • await nextTick() inside an event handler: While nextTick ensures the DOM updates after Vue re-render, using async/await here adds unnecessary microtask overhead. If hasOpened.value = true triggers a massive re-render (e.g., mounting complex dropdown menus), delaying this assignment might cause a visible flash or layout shift if the UI depends on this flag immediately.
  • Suggestion: Unless you specifically need to wait for a child component's onMounted hook before changing this value, standard synchronous assignment is faster and safer: 
    function handleVisibleChange(visible: boolean) {
  if (visible && !hasOpened.value) {
    hasOpened.value = true;
  }
}
    Only keep nextTick if you are waiting for specific child lifecycles.

2. Code Clarity & Naming

  • Comment Ambiguity: The comment says "首次入场钩子执行后再切换" (Switch after first entry hooks execute). This implies dependency on child lifecycle hooks. If that’s not strictly necessary, remove the complexity. If it is necessary, name the variable/function more explicitly to reflect why we are delaying.
  • Early Return Style: The new early return style (if (!visible || hasOpened.value) return) is cleaner than the original. 👍

3. Optimization Suggestions

  • Remove Unused Import: Check if Fragment and isVNode are still used elsewhere in the file. If they were only imported for logic removed elsewhere, clean them up.
  • Type Safety: Ensure DropdownInstance correctly types handleVisibleChange. Element Plus dropdowns usually emit @visible-change, so confirm the parameter type matches.

4. Edge Case Risk

  • Multiple Rapid Toggles: If the user rapidly opens/closes the dropdown before nextTick resolves, the second true call will skip due to hasOpened.value, but the await nextTick from the first call might still resolve later, redundantly setting hasOpened.value = true again (minor, but inefficient).
    • Fix: Add a guard check inside the async block if kept:
    async function handleVisibleChange(visible: boolean) {
  if (!visible || hasOpened.value) return
  const currentId = ++transitionId // optional: track state versions
  await nextTick()
  if (hasOpened.value === false && transitionId === currentId) {
    hasOpened.value = true
  }
}

Recommendation

Revert to synchronous assignment unless there is a proven visual bug requiring nextTick. Async operations in visibility handlers should be avoided unless strictly necessary for timing-sensitive animations.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/router/ROUTE_README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ Workspace 共享覆盖 resourceScope，保留 Workspace scope 及知识库 activ

| 菜单 | 路径与条件 |
| ---------- | --------------------------------------------------------------------------------------------------------------------- |
| 资料库 | 空 path 分组；document、image、tag；仅 BASE 显示图片 |
| 资料库 | 空 path 分组；document、image、tag；BASE 显示图片，Workspace 自有知识库的 WORKFLOW 类型也显示图片 |
| 工作流 | workflow-entry；WORKFLOW 类型 beforeEnter 重定向对应全屏画布，其他类型显示转换页 |
| 检索优化 | retrieval/hit-test、retrieval/question、retrieval/dictionary |
| 授权与集成 | integration/chat-user、integration/external-retrieval |
Expand Down
2 changes: 1 addition & 1 deletion ui/src/router/admin/workspace/modules/knowledge.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const knowledgeRoutes: RouteRecordRaw[] = [
path: 'image',
name: 'workspace-knowledge-image-list',
component: () => import('@/views/knowledge-detail/library/image/index.vue'),
meta: { permission: ({ params }) => perm.knowledge.workspace.image.read(String(params.knowledgeId)), next: 'workspace-knowledge-tag', title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE },
meta: { permission: ({ params }) => perm.knowledge.workspace.image.read(String(params.knowledgeId)), next: 'workspace-knowledge-tag', title: '图片', order: 20, detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW },
},
{
path: 'tag',

@shaohuzhang1 shaohuzhang1 Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the analysis of the provided code diff:

1. Irregularities / Issues

  • None detected (Syntax): The syntax is valid TypeScript/JavaScript and correct Vue Router configuration.
  • Logic Validity: Assuming KNOWLEDGE_TYPE_KEY.WORKFLOW is a defined constant, the logic is sound. It extends the visibility of the "Image" menu item for knowledge bases of type BASE or WORKFLOW.

2. Potential Concerns

  • Readability & Maintainability: As more types are added (e.g., DOC, URL, etc.), this condition (params) => params.type === A || params.type === B will become hard to maintain.
  • Type Safety: Ensure KNOWLEDGE_TYPE_KEY is defined elsewhere in the project. If not imported in this file or the surrounding module, it will cause a build/runtime error.

3. Optimization Suggestions

✅ Recommended Improvement: Use an Allowlist Array

Replace the chained || operators with an array .includes() check. This is cleaner, easier to extend, and less prone to logical errors when adding new types.

Before:

detailMenuVisible: (params) => params.type === KNOWLEDGE_TYPE_KEY.BASE || params.type === KNOWLEDGE_TYPE_KEY.WORKFLOW

After:

const ALLOWED_TYPES_FOR_IMAGE = [
  KNOWLEDGE_TYPE_KEY.BASE,
  KNOWLEDGE_TYPE_KEY.WORKFLOW,
  // Add other types here easily in the future
];

// Inside meta:
detailMenuVisible: (params) => ALLOWED_TYPES_FOR_IMAGE.includes(params.type),

💡 Additional Best Practice

If detailMenuVisible is used similarly across multiple routes (e.g., only showing tag, file, etc., for certain types), consider extracting these permission/visibility rules into separate composable functions or constants files to avoid duplication and ensure consistency.

For example:

// utils/menu-utils.ts
export const isVisibleForWorkspaceTypes = (allowedTypes: string[]) => 
  (params: any) => allowedTypes.includes(params.type);

// In Route config:
meta: {
  ...
  detailMenuVisible: isVisibleForWorkspaceTypes([KNOWLEDGE_TYPE_KEY.BASE, KNOWLEDGE_TYPE_KEY.WORKFLOW])
}

Expand Down
Loading