From 1b81ac033fd52a522901e54bc83ca58a667f0214 Mon Sep 17 00:00:00 2001 From: Stephen Zhou Date: Wed, 24 Jun 2026 20:30:11 +0800 Subject: [PATCH] chore(knip): add mdx support clean unused code (#37882) Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .github/workflows/style.yml | 4 + eslint-suppressions.json | 7 +- .../develop/__tests__/code.spec.tsx | 49 +----- .../components/develop/__tests__/md.spec.tsx | 79 +-------- web/app/components/develop/code.tsx | 56 ++----- web/app/components/develop/md.tsx | 7 - .../state/__tests__/dsl-enabled.spec.ts | 2 +- .../state/__tests__/index.spec.ts | 16 +- .../deployments/create-release/state/index.ts | 1 - web/knip.config.ts | 7 +- web/models/app.ts | 5 - web/package.json | 1 + ...check-production-unused-after-knip-fix.mjs | 157 ++++++++++++++++++ web/service/common.ts | 32 +--- 14 files changed, 189 insertions(+), 234 deletions(-) create mode 100644 web/scripts/check-production-unused-after-knip-fix.mjs diff --git a/.github/workflows/style.yml b/.github/workflows/style.yml index c80e1f3d2be..623703c6d35 100644 --- a/.github/workflows/style.yml +++ b/.github/workflows/style.yml @@ -109,6 +109,10 @@ jobs: if: steps.changed-files.outputs.any_changed == 'true' run: vp run knip:production + - name: Web production unused declarations check + if: steps.changed-files.outputs.any_changed == 'true' + run: vp run knip:production-unused-check + ts-common-style: name: TS Common runs-on: depot-ubuntu-24.04 diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 796b8166c73..b7a6eb5f16a 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -3274,7 +3274,7 @@ }, "web/app/components/develop/code.tsx": { "ts/no-explicit-any": { - "count": 7 + "count": 6 } }, "web/app/components/develop/doc.tsx": { @@ -3286,9 +3286,6 @@ "jsx-a11y/no-redundant-roles": { "count": 1 }, - "ts/no-empty-object-type": { - "count": 1 - }, "ts/no-explicit-any": { "count": 2 } @@ -7227,7 +7224,7 @@ "count": 1 }, "ts/no-explicit-any": { - "count": 13 + "count": 9 } }, "web/service/datasets.ts": { diff --git a/web/app/components/develop/__tests__/code.spec.tsx b/web/app/components/develop/__tests__/code.spec.tsx index 34a1f223802..8508095411c 100644 --- a/web/app/components/develop/__tests__/code.spec.tsx +++ b/web/app/components/develop/__tests__/code.spec.tsx @@ -1,6 +1,6 @@ import { act, render, screen, waitFor } from '@testing-library/react' import userEvent from '@testing-library/user-event' -import { Code, CodeGroup, Embed, Pre } from '../code' +import { CodeGroup, Embed } from '../code' vi.mock('@/utils/clipboard', () => ({ writeTextToClipboard: vi.fn().mockResolvedValue(undefined), @@ -21,31 +21,6 @@ describe('code.tsx components', () => { vi.restoreAllMocks() }) - describe('Code', () => { - it('should render children as a code element', () => { - render(const x = 1) - const codeElement = screen.getByText('const x = 1') - expect(codeElement.tagName).toBe('CODE') - }) - - it('should pass through additional props', () => { - render(snippet) - const codeElement = screen.getByTestId('custom-code') - expect(codeElement).toHaveClass('custom-class') - }) - - it('should render with complex children', () => { - render( - - part1 - part2 - , - ) - expect(screen.getByText('part1')).toBeInTheDocument() - expect(screen.getByText('part2')).toBeInTheDocument() - }) - }) - describe('Embed', () => { it('should render value prop as a span element', () => { render(ignored children) @@ -277,28 +252,6 @@ describe('code.tsx components', () => { }) }) - describe('Pre', () => { - it('should wrap children in CodeGroup when outside CodeGroup context', () => { - render( - 
-          
code

-
, - ) - expect(screen.getByText('Pre Title')).toBeInTheDocument() - }) - - it('should return children directly when inside CodeGroup context', () => { - render( - - 
-            inner code
-
- , - ) - expect(screen.getByText('outer code')).toBeInTheDocument() - }) - }) - describe('CodePanelHeader (via CodeGroup)', () => { it('should render when tag is provided', () => { render( diff --git a/web/app/components/develop/__tests__/md.spec.tsx b/web/app/components/develop/__tests__/md.spec.tsx index 65dd9c5738c..bdeed87c0cb 100644 --- a/web/app/components/develop/__tests__/md.spec.tsx +++ b/web/app/components/develop/__tests__/md.spec.tsx @@ -1,5 +1,5 @@ import { render, screen } from '@testing-library/react' -import { Col, Heading, Properties, Property, PropertyInstruction, Row, SubProperty } from '../md' +import { Col, Heading, Properties, Property, Row, SubProperty } from '../md' describe('md.tsx components', () => { describe('Heading', () => { @@ -540,67 +540,6 @@ describe('md.tsx components', () => { }) }) - describe('PropertyInstruction', () => { - it('should render children', () => { - render( - - This is an instruction - , - ) - expect(screen.getByText('This is an instruction')).toBeInTheDocument() - }) - - it('should render as li element', () => { - const { container } = render( - - Instruction text - , - ) - expect(container.querySelector('li')).toBeInTheDocument() - }) - - it('should have m-0 class', () => { - const { container } = render( - - Instruction - , - ) - const li = container.querySelector('li')! - expect(li.className).toContain('m-0') - }) - - it('should have padding classes', () => { - const { container } = render( - - Instruction - , - ) - const li = container.querySelector('li')! - expect(li.className).toContain('px-0') - expect(li.className).toContain('py-4') - }) - - it('should have italic class', () => { - const { container } = render( - - Instruction - , - ) - const li = container.querySelector('li')! - expect(li.className).toContain('italic') - }) - - it('should have first:pt-0 class', () => { - const { container } = render( - - Instruction - , - ) - const li = container.querySelector('li')! - expect(li.className).toContain('first:pt-0') - }) - }) - describe('integration tests', () => { it('should render Property inside Properties', () => { render( @@ -635,21 +574,5 @@ describe('md.tsx components', () => { expect(screen.getByText('Left column')).toBeInTheDocument() expect(screen.getByText('Right column')).toBeInTheDocument() }) - - it('should render PropertyInstruction inside Properties', () => { - render( - - - Note: All fields are required - - - A required field - - , - ) - - expect(screen.getByText('Note: All fields are required')).toBeInTheDocument() - expect(screen.getByText('required_field')).toBeInTheDocument() - }) }) }) diff --git a/web/app/components/develop/code.tsx b/web/app/components/develop/code.tsx index 126d2df43c8..010c37160a4 100644 --- a/web/app/components/develop/code.tsx +++ b/web/app/components/develop/code.tsx @@ -1,5 +1,5 @@ 'use client' -import type { PropsWithChildren, ReactElement, ReactNode } from 'react' +import type { PropsWithChildren, ReactElement } from 'react' import { cn } from '@langgenius/dify-ui/cn' import { Tabs, @@ -9,8 +9,6 @@ import { } from '@langgenius/dify-ui/tabs' import { Children, - createContext, - use, useEffect, useRef, useState, @@ -269,8 +267,6 @@ function useTabGroupProps(tabValues: string[]) { } } -const CodeGroupContext = createContext(false) - type CodeGroupProps = PropsWithChildren<{ /** Code example(s) to display */ targetCode?: string | CodeExample[] @@ -297,42 +293,20 @@ export function CodeGroup({ children, title, targetCode, ...props }: CodeGroupPr ) - return ( - - {hasTabs - ? ( - - {content} - - ) - : ( -
- {content} -
- )} - - ) -} - -type IChildProps = { - children: ReactNode - [key: string]: any -} - -export function Code({ children, ...props }: IChildProps) { - return {children} -} - -export function Pre({ children, ...props }: IChildrenProps) { - const isGrouped = use(CodeGroupContext) - - if (isGrouped) - return children - - return {children} + return hasTabs + ? ( + + {content} + + ) + : ( +
+ {content} +
+ ) } export function Embed({ value, ...props }: IChildrenProps) { diff --git a/web/app/components/develop/md.tsx b/web/app/components/develop/md.tsx index 9e07effb862..d10abe3268a 100644 --- a/web/app/components/develop/md.tsx +++ b/web/app/components/develop/md.tsx @@ -1,5 +1,4 @@ 'use client' -import type { PropsWithChildren } from 'react' import { cn } from '@langgenius/dify-ui/cn' type IChildrenProps = { @@ -140,9 +139,3 @@ export function SubProperty({ name, type, children }: ISubProperty) { ) } - -export function PropertyInstruction({ children }: PropsWithChildren<{ }>) { - return ( -
  • {children}
    • - ) -} diff --git a/web/features/deployments/create-release/state/__tests__/dsl-enabled.spec.ts b/web/features/deployments/create-release/state/__tests__/dsl-enabled.spec.ts index bacafb08696..609b120b4f8 100644 --- a/web/features/deployments/create-release/state/__tests__/dsl-enabled.spec.ts +++ b/web/features/deployments/create-release/state/__tests__/dsl-enabled.spec.ts @@ -139,7 +139,7 @@ async function mountedStore() { }, }) store.set(queryClientAtom, queryClient) - const unsubscribe = store.sub(state.createReleaseFormValuesAtom, () => undefined) + const unsubscribe = store.sub(state.createReleaseFormIsSubmittingAtom, () => undefined) return { state, diff --git a/web/features/deployments/create-release/state/__tests__/index.spec.ts b/web/features/deployments/create-release/state/__tests__/index.spec.ts index dfb1bb12264..3ed7abae1dd 100644 --- a/web/features/deployments/create-release/state/__tests__/index.spec.ts +++ b/web/features/deployments/create-release/state/__tests__/index.spec.ts @@ -126,7 +126,7 @@ async function mountedStore() { }, }) store.set(queryClientAtom, queryClient) - const unsubscribe = store.sub(state.createReleaseFormValuesAtom, () => undefined) + const unsubscribe = store.sub(state.createReleaseFormIsSubmittingAtom, () => undefined) return { queryClient, @@ -232,20 +232,6 @@ describe('create release state', () => { } }) - it('should keep default form values before editing', async () => { - const { state, store, unsubscribe } = await mountedStore() - - expect(store.get(state.createReleaseFormValuesAtom)).toEqual({ - dslFile: undefined, - releaseDescription: '', - releaseName: '', - releaseSourceMode: 'sourceApp', - sourceApp: undefined, - }) - - unsubscribe() - }) - it('should validate release name only when submitting', async () => { const { state, store, unsubscribe } = await mountedStore() diff --git a/web/features/deployments/create-release/state/index.ts b/web/features/deployments/create-release/state/index.ts index ce9daa7b7dc..c88c54cccf9 100644 --- a/web/features/deployments/create-release/state/index.ts +++ b/web/features/deployments/create-release/state/index.ts @@ -106,7 +106,6 @@ export const createReleaseFormAtom = atomWithForm({ const createReleaseFormAtoms = createFormAtoms(createReleaseFormAtom) -export const createReleaseFormValuesAtom = createReleaseFormAtoms.valuesAtom export const createReleaseFormIsSubmittingAtom = createReleaseFormAtoms.isSubmittingAtom export const createReleaseSourceModeFieldAtom = createReleaseFormAtoms.fieldAtom('releaseSourceMode') export const createReleaseSourceAppFieldAtom = createReleaseFormAtoms.fieldAtom('sourceApp') diff --git a/web/knip.config.ts b/web/knip.config.ts index 16ed6ef8cc0..c9518585093 100644 --- a/web/knip.config.ts +++ b/web/knip.config.ts @@ -4,6 +4,9 @@ import type { KnipConfig } from 'knip' * @see https://knip.dev/reference/configuration */ const config: KnipConfig = { + compilers: { + mdx: true, + }, entry: [ 'scripts/**/*.{js,ts,mjs}', 'bin/**/*.{js,ts,mjs}', @@ -11,12 +14,12 @@ const config: KnipConfig = { 'dev-proxy.config.ts', ], project: [ - '**/*.{js,mjs,cjs,jsx,ts,tsx,mts,cts,css}!', + '**/*.{js,mjs,cjs,jsx,ts,tsx,mts,cts,css,mdx}!', '!**/*.{bench,test,test-d,spec,spec-d}.?(c|m)[jt]s?(x)!', '!**/*.test-utils.{ts,tsx}!', '!**/__mocks__/**!', '!**/__tests__/**!', - '!**/*.stories.{js,jsx,ts,tsx}!', + '!**/*.stories.{js,jsx,ts,tsx,mdx}!', '!.storybook/**!', '!context/provider-context-mock.tsx!', '!eslint.constants.mjs!', diff --git a/web/models/app.ts b/web/models/app.ts index f123b4c8a19..a01c3927017 100644 --- a/web/models/app.ts +++ b/web/models/app.ts @@ -93,11 +93,6 @@ export type CreateApiKeyResponse = { created_at: string } -export type ValidateOpenAIKeyResponse = { - result: string - error?: string -} - export type AppVoicesListResponse = [{ name: string value: string diff --git a/web/package.json b/web/package.json index a5da6a61cb1..41dd2cdc17a 100644 --- a/web/package.json +++ b/web/package.json @@ -35,6 +35,7 @@ "i18n:check": "tsx ./scripts/check-i18n.js", "knip": "knip", "knip:production": "knip --production --include files", + "knip:production-unused-check": "node ./scripts/check-production-unused-after-knip-fix.mjs", "lint:tss": "tsslint --project tsconfig.json", "preinstall": "npx only-allow pnpm", "refactor-component": "node ./scripts/refactor-component.js", diff --git a/web/scripts/check-production-unused-after-knip-fix.mjs b/web/scripts/check-production-unused-after-knip-fix.mjs new file mode 100644 index 00000000000..fe4625807fb --- /dev/null +++ b/web/scripts/check-production-unused-after-knip-fix.mjs @@ -0,0 +1,157 @@ +import { spawn } from 'node:child_process' +import path from 'node:path' +import { fileURLToPath } from 'node:url' + +const scriptDir = path.dirname(fileURLToPath(import.meta.url)) +const repoRoot = path.resolve(scriptDir, '../..') +const webDir = path.join(repoRoot, 'web') + +function commandName(name) { + return process.platform === 'win32' ? `${name}.cmd` : name +} + +function run(command, args, options = {}) { + return new Promise((resolve, reject) => { + const child = spawn(command, args, { + cwd: options.cwd, + env: process.env, + shell: false, + }) + + let stdout = '' + let stderr = '' + child.stdout.on('data', data => stdout += data) + child.stderr.on('data', data => stderr += data) + child.on('error', reject) + child.on('close', status => resolve({ status, stdout, stderr })) + }) +} + +function parseVpLintJson(stdout) { + const jsonStart = stdout.indexOf('{') + if (jsonStart === -1) + return { diagnostics: [] } + + return JSON.parse(stdout.slice(jsonStart)) +} + +function relativeDiagnostic(diagnostic) { + const label = diagnostic.labels?.[0] + const diagnosticFile = label?.file ?? diagnostic.filename + const filePath = diagnosticFile + ? path.relative(webDir, path.isAbsolute(diagnosticFile) ? diagnosticFile : path.join(webDir, diagnosticFile)) + : '' + const span = label?.span ? `:${label.span.line}:${label.span.column}` : '' + + return `${filePath}${span} ${diagnostic.code ?? ''} ${diagnostic.message ?? ''}`.trim() +} + +async function ensureCleanWorktree() { + const result = await run('git', ['status', '--porcelain=v1'], { cwd: repoRoot }) + if (result.status !== 0) { + process.stderr.write(result.stderr) + throw new Error('Failed to check git status.') + } + + if (!result.stdout.trim()) + return + + console.error('This check runs knip --fix and must start from a clean worktree.') + console.error('Commit or stash your changes first, then run it again.') + console.error(result.stdout) + process.exit(1) +} + +async function restoreWorktree() { + const restoreResult = await run('git', ['restore', '--staged', '--worktree', '.'], { cwd: repoRoot }) + if (restoreResult.status !== 0) { + process.stdout.write(restoreResult.stdout) + process.stderr.write(restoreResult.stderr) + throw new Error('Failed to restore tracked files after knip --fix.') + } + + const cleanResult = await run('git', ['clean', '-fd', '--', 'web', '.eslintcache'], { cwd: repoRoot }) + if (cleanResult.status !== 0) { + process.stdout.write(cleanResult.stdout) + process.stderr.write(cleanResult.stderr) + throw new Error('Failed to clean untracked files after knip --fix.') + } +} + +const knip = path.join(webDir, 'node_modules', '.bin', commandName('knip')) +const vp = path.join(repoRoot, 'node_modules', '.bin', commandName('vp')) +let shouldRestore = false +let hasUnusedMessages = false + +try { + await ensureCleanWorktree() + shouldRestore = true + + console.log('Running knip --production --fix...') + const knipResult = await run(knip, ['--production', '--fix'], { cwd: webDir }) + if (knipResult.status !== 0) { + process.stdout.write(knipResult.stdout) + process.stderr.write(knipResult.stderr) + throw new Error('knip --production --fix failed.') + } + + console.log('Running Vite+ unused checks after knip --fix...') + const lintResult = await run(vp, [ + 'lint', + '-A', + 'all', + '-D', + 'no-unused-vars', + '--format', + 'json', + '--ignore-pattern', + 'public/**', + '--ignore-pattern', + 'coverage/**', + '--ignore-pattern', + '.next/**', + '--ignore-pattern', + '**/__tests__/**', + '--ignore-pattern', + '**/*.spec.ts', + '--ignore-pattern', + '**/*.spec.tsx', + '--ignore-pattern', + '**/*.test.ts', + '--ignore-pattern', + '**/*.test.tsx', + '.', + ], { cwd: webDir }) + + let lintOutput + try { + lintOutput = parseVpLintJson(lintResult.stdout) + } + catch { + process.stdout.write(lintResult.stdout) + process.stderr.write(lintResult.stderr) + throw new Error('Failed to parse Vite+ lint JSON output.') + } + + const unusedMessages = (lintOutput.diagnostics ?? []).map(relativeDiagnostic) + + if (unusedMessages.length > 0) { + hasUnusedMessages = true + console.error('Unused declarations remain after applying knip --production --fix.') + console.error('Remove these declarations; if they are only referenced by tests, remove the matching tests too.') + for (const message of unusedMessages) + console.error(message) + } + else { + console.log('No Vite+ unused declarations remain after knip --production --fix.') + } +} +finally { + if (shouldRestore) { + console.log('Restoring checkout after knip --fix...') + await restoreWorktree() + } +} + +if (hasUnusedMessages) + process.exit(1) diff --git a/web/service/common.ts b/web/service/common.ts index ece28057184..f43c9a13b91 100644 --- a/web/service/common.ts +++ b/web/service/common.ts @@ -2,13 +2,9 @@ import type { DefaultModelResponse, Model, ModelItem, - ModelLoadBalancingConfig, ModelParameterRule, ModelTypeEnum, } from '@/app/components/header/account-setting/model-provider-page/declarations' -import type { - ValidateOpenAIKeyResponse, -} from '@/models/app' import type { CommonResponse, ICurrentWorkspace, @@ -104,13 +100,7 @@ export const invitationCheck = ({ url, params }: { url: string, params: { worksp export const activateMember = ({ url, body }: { url: string, body: ActivateMemberBody }): Promise => { return post(url, { body }) } -export type ModelProviderCredentials = { - credentials?: Record - load_balancing: ModelLoadBalancingConfig -} -export const fetchModelProviderCredentials = (url: string): Promise => { - return get(url) -} + export const fetchModelProviderModelList = (url: string): Promise<{ data: ModelItem[] }> => { return get<{ data: ModelItem[] }>(url) } @@ -119,26 +109,6 @@ export const fetchModelList = (url: string): Promise<{ data: Model[] }> => { return get<{ data: Model[] }>(url) } -export const validateModelProvider = ({ url, body }: { url: string, body: any }): Promise => { - return post(url, { body }) -} - -export const validateModelLoadBalancingCredentials = ({ url, body }: { url: string, body: any }): Promise => { - return post(url, { body }) -} - -export const setModelProvider = ({ url, body }: { url: string, body: any }): Promise => { - return post(url, { body }) -} - -export const deleteModelProvider = ({ url, body }: { url: string, body?: any }): Promise => { - return del(url, { body }) -} - -export const getPayUrl = (url: string): Promise<{ url: string }> => { - return get<{ url: string }>(url) -} - export const fetchDefaultModal = (url: string): Promise<{ data: DefaultModelResponse }> => { return get<{ data: DefaultModelResponse }>(url) }