mirror of
https://github.com/langgenius/dify.git
synced 2026-02-09 23:20:12 -05:00
refactor(skill)!: add file node view-state flow and mode-based file data hook
- introduce resolving/ready/missing node view-state to avoid unsupported flicker - switch useSkillFileData to explicit mode: none/content/download - add hook tests for view-state transitions and mode query gating BREAKING CHANGE: useSkillFileData signature changed from (appId, nodeId, isEditable) to (appId, nodeId, mode).
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
'use client'
|
||||
|
||||
import type { OnMount } from '@monaco-editor/react'
|
||||
import type { SkillFileDataMode } from './hooks/use-skill-file-data'
|
||||
import type { AppAssetTreeView } from '@/types/app-asset'
|
||||
import { loader } from '@monaco-editor/react'
|
||||
import isDeepEqual from 'fast-deep-equal'
|
||||
@@ -20,6 +21,7 @@ import { START_TAB_ID } from './constants'
|
||||
import CodeFileEditor from './editor/code-file-editor'
|
||||
import MarkdownFileEditor from './editor/markdown-file-editor'
|
||||
import { useSkillSaveManager } from './hooks/skill-save-context'
|
||||
import { useFileNodeViewState } from './hooks/use-file-node-view-state'
|
||||
import { useFileTypeInfo } from './hooks/use-file-type-info'
|
||||
import { useSkillAssetNodeMap } from './hooks/use-skill-asset-tree'
|
||||
import { useSkillFileData } from './hooks/use-skill-file-data'
|
||||
@@ -69,7 +71,12 @@ const FileContentPanel = () => {
|
||||
const activeTabId = useStore(s => s.activeTabId)
|
||||
const editorAutoFocusFileId = useStore(s => s.editorAutoFocusFileId)
|
||||
const storeApi = useWorkflowStore()
|
||||
const { data: nodeMap } = useSkillAssetNodeMap()
|
||||
const {
|
||||
data: nodeMap,
|
||||
isLoading: isNodeMapLoading,
|
||||
isFetching: isNodeMapFetching,
|
||||
isFetched: isNodeMapFetched,
|
||||
} = useSkillAssetNodeMap()
|
||||
|
||||
const isStartTab = activeTabId === START_TAB_ID
|
||||
const fileTabId = isStartTab ? null : activeTabId
|
||||
@@ -80,10 +87,23 @@ const FileContentPanel = () => {
|
||||
|
||||
const currentFileNode = fileTabId ? nodeMap?.get(fileTabId) : undefined
|
||||
const shouldAutoFocusEditor = Boolean(fileTabId && editorAutoFocusFileId === fileTabId)
|
||||
const fileNodeViewState = useFileNodeViewState({
|
||||
fileTabId,
|
||||
hasCurrentFileNode: Boolean(currentFileNode),
|
||||
isNodeMapLoading,
|
||||
isNodeMapFetching,
|
||||
isNodeMapFetched,
|
||||
})
|
||||
const isNodeReady = fileNodeViewState === 'ready'
|
||||
|
||||
const { isMarkdown, isCodeOrText, isImage, isVideo, isPdf, isSQLite, isEditable, isPreviewable } = useFileTypeInfo(currentFileNode)
|
||||
const { isMarkdown, isCodeOrText, isImage, isVideo, isPdf, isSQLite, isEditable, isPreviewable } = useFileTypeInfo(isNodeReady ? currentFileNode : undefined)
|
||||
const fileDataMode: SkillFileDataMode = !fileTabId || !isNodeReady
|
||||
? 'none'
|
||||
: isEditable
|
||||
? 'content'
|
||||
: 'download'
|
||||
|
||||
const { fileContent, downloadUrlData, isLoading, error } = useSkillFileData(appId, fileTabId, isEditable)
|
||||
const { fileContent, downloadUrlData, isLoading, error } = useSkillFileData(appId, fileTabId, fileDataMode)
|
||||
|
||||
const originalContent = fileContent?.content ?? ''
|
||||
const currentContent = draftContent !== undefined ? draftContent : originalContent
|
||||
@@ -246,6 +266,24 @@ const FileContentPanel = () => {
|
||||
)
|
||||
}
|
||||
|
||||
if (fileNodeViewState === 'resolving') {
|
||||
return (
|
||||
<div className="flex h-full w-full items-center justify-center bg-components-panel-bg">
|
||||
<Loading type="area" />
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
if (fileNodeViewState === 'missing') {
|
||||
return (
|
||||
<div className="flex h-full w-full items-center justify-center bg-components-panel-bg text-text-tertiary">
|
||||
<span className="system-sm-regular">
|
||||
{t('skillSidebar.loadError')}
|
||||
</span>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
if (isLoading) {
|
||||
return (
|
||||
<div className="flex h-full w-full items-center justify-center bg-components-panel-bg">
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
import { renderHook, waitFor } from '@testing-library/react'
|
||||
import { useFileNodeViewState } from './use-file-node-view-state'
|
||||
|
||||
type HookProps = {
|
||||
fileTabId: string | null
|
||||
hasCurrentFileNode: boolean
|
||||
isNodeMapLoading: boolean
|
||||
isNodeMapFetching: boolean
|
||||
isNodeMapFetched: boolean
|
||||
}
|
||||
|
||||
const createProps = (overrides: Partial<HookProps> = {}): HookProps => ({
|
||||
fileTabId: 'file-1',
|
||||
hasCurrentFileNode: false,
|
||||
isNodeMapLoading: true,
|
||||
isNodeMapFetching: true,
|
||||
isNodeMapFetched: false,
|
||||
...overrides,
|
||||
})
|
||||
|
||||
describe('useFileNodeViewState', () => {
|
||||
describe('resolution lifecycle', () => {
|
||||
it('should return ready when there is no active file tab', () => {
|
||||
const { result } = renderHook(() => useFileNodeViewState(createProps({
|
||||
fileTabId: null,
|
||||
})))
|
||||
|
||||
expect(result.current).toBe('ready')
|
||||
})
|
||||
|
||||
it('should return resolving during initial node resolution', () => {
|
||||
const { result } = renderHook(() => useFileNodeViewState(createProps()))
|
||||
|
||||
expect(result.current).toBe('resolving')
|
||||
})
|
||||
|
||||
it('should return missing when query settles without a matching node', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
(props: HookProps) => useFileNodeViewState(props),
|
||||
{ initialProps: createProps() },
|
||||
)
|
||||
|
||||
rerender(createProps({
|
||||
isNodeMapLoading: false,
|
||||
isNodeMapFetching: false,
|
||||
isNodeMapFetched: true,
|
||||
}))
|
||||
|
||||
expect(result.current).toBe('missing')
|
||||
})
|
||||
|
||||
it('should stay missing during background refetch after missing is resolved', async () => {
|
||||
const { result, rerender } = renderHook(
|
||||
(props: HookProps) => useFileNodeViewState(props),
|
||||
{ initialProps: createProps() },
|
||||
)
|
||||
|
||||
rerender(createProps({
|
||||
isNodeMapLoading: false,
|
||||
isNodeMapFetching: false,
|
||||
isNodeMapFetched: true,
|
||||
}))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(result.current).toBe('missing')
|
||||
})
|
||||
|
||||
rerender(createProps({
|
||||
isNodeMapLoading: false,
|
||||
isNodeMapFetching: true,
|
||||
isNodeMapFetched: true,
|
||||
}))
|
||||
|
||||
expect(result.current).toBe('missing')
|
||||
})
|
||||
|
||||
it('should become ready once the target node appears', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
(props: HookProps) => useFileNodeViewState(props),
|
||||
{ initialProps: createProps() },
|
||||
)
|
||||
|
||||
rerender(createProps({
|
||||
hasCurrentFileNode: true,
|
||||
isNodeMapLoading: false,
|
||||
isNodeMapFetching: false,
|
||||
isNodeMapFetched: true,
|
||||
}))
|
||||
|
||||
expect(result.current).toBe('ready')
|
||||
})
|
||||
|
||||
it('should reset to resolving when switching to another file tab', () => {
|
||||
const { result, rerender } = renderHook(
|
||||
(props: HookProps) => useFileNodeViewState(props),
|
||||
{ initialProps: createProps({
|
||||
isNodeMapLoading: false,
|
||||
isNodeMapFetching: false,
|
||||
isNodeMapFetched: true,
|
||||
}) },
|
||||
)
|
||||
|
||||
expect(result.current).toBe('missing')
|
||||
|
||||
rerender(createProps({
|
||||
fileTabId: 'file-2',
|
||||
isNodeMapLoading: false,
|
||||
isNodeMapFetching: true,
|
||||
isNodeMapFetched: true,
|
||||
}))
|
||||
|
||||
expect(result.current).toBe('resolving')
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,73 @@
|
||||
import { useRef } from 'react'
|
||||
|
||||
export type FileNodeViewState = 'resolving' | 'ready' | 'missing'
|
||||
|
||||
type ResolveFileNodeViewStateParams = {
|
||||
hasFileTabId: boolean
|
||||
hasCurrentFileNode: boolean
|
||||
isNodeMapLoading: boolean
|
||||
isNodeMapFetching: boolean
|
||||
isNodeMapFetched: boolean
|
||||
isNodeResolutionPending: boolean
|
||||
}
|
||||
|
||||
type UseFileNodeViewStateParams = {
|
||||
fileTabId: string | null
|
||||
hasCurrentFileNode: boolean
|
||||
isNodeMapLoading: boolean
|
||||
isNodeMapFetching: boolean
|
||||
isNodeMapFetched: boolean
|
||||
}
|
||||
|
||||
export const resolveFileNodeViewState = ({
|
||||
hasFileTabId,
|
||||
hasCurrentFileNode,
|
||||
isNodeMapLoading,
|
||||
isNodeMapFetching,
|
||||
isNodeMapFetched,
|
||||
isNodeResolutionPending,
|
||||
}: ResolveFileNodeViewStateParams): FileNodeViewState => {
|
||||
if (!hasFileTabId || hasCurrentFileNode)
|
||||
return 'ready'
|
||||
|
||||
if (isNodeResolutionPending && (!isNodeMapFetched || isNodeMapLoading || isNodeMapFetching))
|
||||
return 'resolving'
|
||||
|
||||
return 'missing'
|
||||
}
|
||||
|
||||
export function useFileNodeViewState({
|
||||
fileTabId,
|
||||
hasCurrentFileNode,
|
||||
isNodeMapLoading,
|
||||
isNodeMapFetching,
|
||||
isNodeMapFetched,
|
||||
}: UseFileNodeViewStateParams): FileNodeViewState {
|
||||
const hasFileTabId = Boolean(fileTabId)
|
||||
const resolutionRef = useRef<{ tabId: string | null, pending: boolean }>({
|
||||
tabId: fileTabId,
|
||||
pending: hasFileTabId,
|
||||
})
|
||||
|
||||
if (resolutionRef.current.tabId !== fileTabId) {
|
||||
resolutionRef.current = {
|
||||
tabId: fileTabId,
|
||||
pending: hasFileTabId,
|
||||
}
|
||||
}
|
||||
|
||||
if (fileTabId && resolutionRef.current.pending) {
|
||||
const isQuerySettled = isNodeMapFetched && !isNodeMapLoading && !isNodeMapFetching
|
||||
if (hasCurrentFileNode || isQuerySettled)
|
||||
resolutionRef.current.pending = false
|
||||
}
|
||||
|
||||
return resolveFileNodeViewState({
|
||||
hasFileTabId,
|
||||
hasCurrentFileNode,
|
||||
isNodeMapLoading,
|
||||
isNodeMapFetching,
|
||||
isNodeMapFetched,
|
||||
isNodeResolutionPending: resolutionRef.current.pending,
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,86 @@
|
||||
import { renderHook } from '@testing-library/react'
|
||||
import { useSkillFileData } from './use-skill-file-data'
|
||||
|
||||
const {
|
||||
mockUseGetAppAssetFileContent,
|
||||
mockUseGetAppAssetFileDownloadUrl,
|
||||
} = vi.hoisted(() => ({
|
||||
mockUseGetAppAssetFileContent: vi.fn(),
|
||||
mockUseGetAppAssetFileDownloadUrl: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/service/use-app-asset', () => ({
|
||||
useGetAppAssetFileContent: (...args: unknown[]) => mockUseGetAppAssetFileContent(...args),
|
||||
useGetAppAssetFileDownloadUrl: (...args: unknown[]) => mockUseGetAppAssetFileDownloadUrl(...args),
|
||||
}))
|
||||
|
||||
describe('useSkillFileData', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockUseGetAppAssetFileContent.mockReturnValue({
|
||||
data: undefined,
|
||||
isLoading: false,
|
||||
error: null,
|
||||
})
|
||||
mockUseGetAppAssetFileDownloadUrl.mockReturnValue({
|
||||
data: undefined,
|
||||
isLoading: false,
|
||||
error: null,
|
||||
})
|
||||
})
|
||||
|
||||
describe('mode control', () => {
|
||||
it('should disable both queries when mode is none', () => {
|
||||
const { result } = renderHook(() => useSkillFileData('app-1', 'node-1', 'none'))
|
||||
|
||||
expect(mockUseGetAppAssetFileContent).toHaveBeenCalledWith('app-1', 'node-1', { enabled: false })
|
||||
expect(mockUseGetAppAssetFileDownloadUrl).toHaveBeenCalledWith('app-1', 'node-1', { enabled: false })
|
||||
expect(result.current.isLoading).toBe(false)
|
||||
expect(result.current.error).toBeNull()
|
||||
})
|
||||
|
||||
it('should fetch content data when mode is content', () => {
|
||||
const contentError = new Error('content-error')
|
||||
mockUseGetAppAssetFileContent.mockReturnValue({
|
||||
data: { content: 'hello' },
|
||||
isLoading: true,
|
||||
error: contentError,
|
||||
})
|
||||
mockUseGetAppAssetFileDownloadUrl.mockReturnValue({
|
||||
data: { download_url: 'https://example.com/file' },
|
||||
isLoading: true,
|
||||
error: new Error('download-error'),
|
||||
})
|
||||
|
||||
const { result } = renderHook(() => useSkillFileData('app-1', 'node-1', 'content'))
|
||||
|
||||
expect(mockUseGetAppAssetFileContent).toHaveBeenCalledWith('app-1', 'node-1', { enabled: true })
|
||||
expect(mockUseGetAppAssetFileDownloadUrl).toHaveBeenCalledWith('app-1', 'node-1', { enabled: false })
|
||||
expect(result.current.fileContent).toEqual({ content: 'hello' })
|
||||
expect(result.current.isLoading).toBe(true)
|
||||
expect(result.current.error).toBe(contentError)
|
||||
})
|
||||
|
||||
it('should fetch download URL data when mode is download', () => {
|
||||
const downloadError = new Error('download-error')
|
||||
mockUseGetAppAssetFileContent.mockReturnValue({
|
||||
data: { content: 'hello' },
|
||||
isLoading: true,
|
||||
error: new Error('content-error'),
|
||||
})
|
||||
mockUseGetAppAssetFileDownloadUrl.mockReturnValue({
|
||||
data: { download_url: 'https://example.com/file' },
|
||||
isLoading: true,
|
||||
error: downloadError,
|
||||
})
|
||||
|
||||
const { result } = renderHook(() => useSkillFileData('app-1', 'node-1', 'download'))
|
||||
|
||||
expect(mockUseGetAppAssetFileContent).toHaveBeenCalledWith('app-1', 'node-1', { enabled: false })
|
||||
expect(mockUseGetAppAssetFileDownloadUrl).toHaveBeenCalledWith('app-1', 'node-1', { enabled: true })
|
||||
expect(result.current.downloadUrlData).toEqual({ download_url: 'https://example.com/file' })
|
||||
expect(result.current.isLoading).toBe(true)
|
||||
expect(result.current.error).toBe(downloadError)
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,7 @@
|
||||
import { useGetAppAssetFileContent, useGetAppAssetFileDownloadUrl } from '@/service/use-app-asset'
|
||||
|
||||
export type SkillFileDataMode = 'none' | 'content' | 'download'
|
||||
|
||||
export type SkillFileDataResult = {
|
||||
fileContent: ReturnType<typeof useGetAppAssetFileContent>['data']
|
||||
downloadUrlData: ReturnType<typeof useGetAppAssetFileDownloadUrl>['data']
|
||||
@@ -9,19 +11,22 @@ export type SkillFileDataResult = {
|
||||
|
||||
/**
|
||||
* Hook to fetch file data for skill documents.
|
||||
* Fetches content for editable files and download URL for non-editable files.
|
||||
* Uses explicit mode to control data fetching:
|
||||
* - 'content': fetch editable file content
|
||||
* - 'download': fetch non-editable file download URL
|
||||
* - 'none': skip file-related requests while node metadata is unresolved
|
||||
*/
|
||||
export function useSkillFileData(
|
||||
appId: string,
|
||||
nodeId: string | null | undefined,
|
||||
isEditable: boolean,
|
||||
mode: SkillFileDataMode,
|
||||
): SkillFileDataResult {
|
||||
const {
|
||||
data: fileContent,
|
||||
isLoading: isContentLoading,
|
||||
error: contentError,
|
||||
} = useGetAppAssetFileContent(appId, nodeId || '', {
|
||||
enabled: isEditable,
|
||||
enabled: mode === 'content',
|
||||
})
|
||||
|
||||
const {
|
||||
@@ -29,11 +34,19 @@ export function useSkillFileData(
|
||||
isLoading: isDownloadUrlLoading,
|
||||
error: downloadUrlError,
|
||||
} = useGetAppAssetFileDownloadUrl(appId, nodeId || '', {
|
||||
enabled: !isEditable && !!nodeId,
|
||||
enabled: mode === 'download' && !!nodeId,
|
||||
})
|
||||
|
||||
const isLoading = isEditable ? isContentLoading : isDownloadUrlLoading
|
||||
const error = isEditable ? contentError : downloadUrlError
|
||||
const isLoading = mode === 'content'
|
||||
? isContentLoading
|
||||
: mode === 'download'
|
||||
? isDownloadUrlLoading
|
||||
: false
|
||||
const error = mode === 'content'
|
||||
? contentError
|
||||
: mode === 'download'
|
||||
? downloadUrlError
|
||||
: null
|
||||
|
||||
return {
|
||||
fileContent,
|
||||
|
||||
Reference in New Issue
Block a user