Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions lint-staged.config.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import path from 'node:path'

export default {
'tests-ui/**': () => 'echo "Files in tests-ui/ are deprecated. Colocate tests with source files." && exit 1',

Comment on lines 3 to +5
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n lint-staged.config.mjs

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 196


🏁 Script executed:

# Check if there are other lint-staged configs or documentation in the repo
fd -t f "lint-staged" --or -name "*lint*" -type f | grep -E "\.(mjs|js|json|md)$" | head -20

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 196


🏁 Script executed:

# Search for git diff usage in the codebase to understand patterns
rg "git diff.*--cached" -A 2 -B 2

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 196


🌐 Web query:

lint-staged glob pattern function return value behavior

💡 Result:

Short answer: lint-staged lets you use functions in JS configs; those functions receive the (staged) filenames and must return either

  • a string (one shell command),
  • an array of strings (multiple commands), or
  • a Promise resolving to one of the above.

Important details

  • If you export a function as the whole config, it receives ALL staged filenames. If you use a function as a value for a specific glob key, it receives only the staged files that match that glob. [1][2]
  • Returned command strings are treated as complete shell commands. They do NOT have filenames automatically appended — include the filenames in the returned string(s) yourself (or return commands that already incorporate the filenames). [2]
  • Functions may be async (return a Promise); use awaiting/filtering inside the function when needed (examples shown in the docs). [2][3]

Examples (conceptual)

  • Export-as-function:
    (filenames) => eslint --fix ${filenames.join(' ')} // receives all staged files
  • Per-glob function:
    '*.js': (files) => files.length ? [eslint --fix ${files.join(' ')}] : [] // receives only matched files

Sources

  • lint-staged docs (JS config / function signature & examples). [1]
  • lint-staged README on GitHub (filtering + behavior for glob keys). [2]

Scope the tests-ui lint-staged block to new files only.

The rule currently blocks any staged change under tests-ui/ (including deletions and renames), preventing migrations without --no-verify. Restrict the block to new file additions only.

💡 Proposed fix (block only added files)
-import path from 'node:path'
+import path from 'node:path'
+import { execSync } from 'node:child_process'

 export default {
-  'tests-ui/**': () => 'echo "Files in tests-ui/ are deprecated. Colocate tests with source files." && exit 1',
+  'tests-ui/**': () => {
+    const added = execSync(
+      'git diff --cached --name-status --diff-filter=A -- tests-ui',
+      { encoding: 'utf8' }
+    )
+    return added.trim()
+      ? 'echo "Files in tests-ui/ are deprecated. Colocate tests with source files." && exit 1'
+      : []
+  },
🤖 Prompt for AI Agents
In `@lint-staged.config.mjs` around lines 3 - 5, The current lint-staged rule for
'tests-ui/**' blocks any staged change; replace the static rule with a function
that only fails when newly added files under that pattern are staged by running
a git cached diff filtered for added files (e.g. using "git diff --cached
--name-status --diff-filter=A") and checking whether any of those added paths
match 'tests-ui/**'; if matches exist, print the deprecation message and exit 1,
otherwise return a no-op so renames/deletions/edits don't trigger the check.
Ensure you update the rule for 'tests-ui/**' in the lint-staged config to use
this function-based approach.

'./**/*.js': (stagedFiles) => formatAndEslint(stagedFiles),

'./**/*.{ts,tsx,vue,mts}': (stagedFiles) => [
Expand Down
28 changes: 25 additions & 3 deletions vite.config.mts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import IconsResolver from 'unplugin-icons/resolver'
import Icons from 'unplugin-icons/vite'
import Components from 'unplugin-vue-components/vite'
import typegpuPlugin from 'unplugin-typegpu/vite'
import { defineConfig } from 'vite'
import type { ProxyOptions, UserConfig } from 'vite'
import { defineConfig } from 'vitest/config'
import type { ProxyOptions } from 'vite'
import { createHtmlPlugin } from 'vite-plugin-html'
import vueDevTools from 'vite-plugin-vue-devtools'

Expand Down Expand Up @@ -511,5 +511,27 @@ export default defineConfig({
optimizeDeps: {
exclude: ['@comfyorg/comfyui-electron-types'],
entries: ['index.html']
},

test: {
globals: true,
environment: 'happy-dom',
setupFiles: ['./vitest.setup.ts'],
retry: process.env.CI ? 2 : 0,
include: [
'src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}',
'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'
],
coverage: {
reporter: ['text', 'json', 'html']
},
exclude: [
'**/node_modules/**',
'**/dist/**',
'**/cypress/**',
'**/.{idea,git,cache,output,temp}/**',
'**/{karma,rollup,webpack,vite,vitest,jest,ava,babel,nyc,cypress,tsup,build,eslint,prettier}.config.*'
],
silent: 'passed-only'
Comment on lines +516 to +535
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 17, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if tests-ui directory exists
if [ -d tests-ui ]; then
  echo "=== tests-ui directory exists ==="
  # Find test files in tests-ui
  fd -a -e ts -e tsx -e js -e jsx -e mts -e cts -e mjs -e cjs '(\.test|\.spec)\.' tests-ui | head -20
else
  echo "tests-ui directory does not exist"
fi

# Check current vite.config.mts include patterns
echo -e "\n=== Vite config include patterns ===" 
sed -n '516,535p' vite.config.mts

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 905


Add tests-ui/** to Vitest include patterns.

Tests in tests-ui/ won't run with the current configuration. Real test files exist at tests-ui/stores/templateRankingStore.test.ts and tests-ui/tests/composables/graph/useGraphNodeManager.test.ts, but Vitest's include patterns only cover src/** and packages/**. This reduces coverage and misses regressions for unit/component tests in the tests-ui directory.

Add tests-ui to include
     include: [
       'src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}',
-      'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'
+      'packages/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}',
+      'tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'
     ],
🤖 Prompt for AI Agents
In `@vite.config.mts` around lines 516 - 535, The Vitest configuration in the test
block (the test object in vite.config.mts) currently only includes 'src/**' and
'packages/**' patterns; add an include pattern for the tests-ui folder so files
like tests-ui/stores/templateRankingStore.test.ts and
tests-ui/tests/composables/graph/useGraphNodeManager.test.ts are picked up.
Update the test.include array to also contain a pattern for
tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx} so Vitest will run
those test files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does tests-ui exist?

Copy link
Contributor

Choose a reason for hiding this comment

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

@DrJKL Yes, the tests-ui directory exists. The script output in my previous analysis shows:

=== tests-ui directory exists ===
tests-ui/stores/templateRankingStore.test.ts
tests-ui/tests/composables/graph/useGraphNodeManager.test.ts

The directory contains at least two test files that won't be picked up by the current Vitest configuration since the include patterns only cover src/** and packages/**. Adding tests-ui/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx} to the include array would ensure these tests run.


🧠 Learnings used
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : For mocking in tests, leverage Vitest's utilities; keep module mocks contained without global mutable state

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : For component testing, use Vue Test Utils and make components easy to test

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Be parsimonious in testing; do not write redundant tests

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Do not write tests dependent on non-behavioral features like utility classes or styles

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Do not mock what you don't own; be selective with external dependencies

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Aim for behavioral coverage of critical and new features

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Unit and component test files must use `.test.ts` extension and be located in `tests-ui/` or `src/**/*.test.ts`

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Use `vi.hoisted()` if necessary to allow per-test Arrange phase manipulation of mock state

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src|browser_tests)/**/*.+(test.ts|spec.ts) : Write tests for all changes, especially bug fixes to catch regressions

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to src/**/*.{ts,tsx,vue} : ESLint rules: Vue + TS rules, no floating promises, unused imports disallowed, i18n raw text restrictions in templates

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T21:33:34.380Z
Learning: Applies to +(tests-ui|src)/**/*.test.ts : Do not write tests that just test the mocks; ensure tests fail when code behaves unexpectedly

Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7303
File: src/components/topbar/CurrentUserPopover.test.ts:199-205
Timestamp: 2025-12-10T03:09:19.636Z
Learning: For test files in the Comfy-Org/ComfyUI_frontend repository: When writing tests, prefer selecting elements by accessible properties (text content, aria-label, role, accessible name) over data-testid attributes. This ensures tests verify actual user-facing behavior and accessibility compliance.

Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7649
File: tests-ui/tests/platform/cloud/subscription/components/SubscriptionPanel.test.ts:189-194
Timestamp: 2025-12-22T21:36:46.909Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: Do not stub primitive UI components or customized primitive components (e.g., Button). Instead, import and register the real components in test setup. This ensures tests accurately reflect production behavior and component API usage.

Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7894
File: src/renderer/extensions/vueNodes/widgets/components/WidgetToggleSwitch.test.ts:11-14
Timestamp: 2026-01-08T02:40:22.621Z
Learning: In the Comfy-Org/ComfyUI_frontend repository test files: When testing components, import the real type definitions from the component files instead of duplicating interface definitions in the test files. This prevents type drift and maintains consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, and @AustinMroz !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a lint-staged check to start.

}
}) satisfies UserConfig as UserConfig
})
48 changes: 0 additions & 48 deletions vitest.config.ts

This file was deleted.