Feat/vue #1655
Feat/vue #1655
Conversation
WalkthroughThis PR migrates the web package from Nuxt to a Vite-based standalone app and introduces a new component auto-mounting system with a centralized registry. It updates build/deploy pipelines and artifacts, adds public test pages, revises UI/theming tokens, adjusts GraphQL types/clients, and enhances the plugin builder with standalone manifest validation. Several Vue components are added, reorganized, or deleted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Browser
participant Auto as auto-mount.ts
participant Reg as component-registry.ts
participant Eng as mount-engine.ts
participant DOM as Document
participant GQL as Apollo Client
participant Theme as Theme Initializer
User->>Auto: Load (DOMContentLoaded or immediate)
Auto->>GQL: provideApolloClient(apolloClient)
Auto->>Theme: initializeTheme()
Auto->>Eng: expose {mountVueApp, getMountedApp, autoMountComponent}
Auto->>Eng: autoMountAllComponents()
Eng->>Reg: componentMappings
loop For each mapping
Eng->>DOM: query selector(s)
alt found & ready
Eng->>Eng: create wrapper (UApp + Standalone wrapper)
Eng->>DOM: mount component (static or via loader)
else waitForElement
Eng->>DOM: poll until present then mount
end
end
sequenceDiagram
autonumber
participant Dev as web/package.json (Vite)
participant Vite as Vite Build
participant Man as add-timestamp-standalone-manifest.js
participant Dist as web/dist
participant Plugin as plugin/builder/build-txz.ts
participant Val as manifest-validator.ts
Dev->>Vite: vite build
Vite->>Dist: emit JS/CSS/assets
Vite->>Man: run manifest timestamp
Man->>Dist: write standalone.manifest.json (+ts)
Plugin->>Val: getStandaloneManifestPath(...)
Val-->>Plugin: path (standalone/standalone.manifest.json)
Plugin->>Val: validateStandaloneManifest(path)
Val-->>Plugin: {isValid, errors, warnings}
Plugin->>Plugin: storeVendorArchiveInfo(...)
Plugin->>Plugin: package txz
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
Comment |
elibosley
changed the base branch from
feat/build-docker-detail-layout
to
main
|
Claude finished @elibosley's task —— View job I'll analyze this and get back to you. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web/pages/index.vue (2)
29-31: Mock session cookie is set unconditionally — gate to devThis will override real sessions in production. Guard with a dev check.
-onMounted(() => { - document.cookie = 'unraid_session_cookie=mockusersession'; -}); +onMounted(() => { + if (import.meta.env.DEV) { + document.cookie = 'unraid_session_cookie=mockusersession'; + } +});
146-147: Fix user-facing typo: “DowngraadeOsCe”Correct the heading label.
- <h3 class="text-lg font-semibold font-mono">DowngraadeOsCe</h3> + <h3 class="text-lg font-semibold font-mono">DowngradeOsCe</h3>web/components/Wrapper/web-component-plugins.ts (1)
19-31: Fix locale fallback; avoid selecting empty locale ''.When
Object.keys(parsedMessages)is empty,parsedLocalebecomes'', which setslocale: ''and togglesnonDefaultLocaletrue unnecessarily. Fall back todefaultLocalewhen no key exists.- let parsedMessages = {}; + let parsedMessages: Record<string, unknown> = {}; let nonDefaultLocale = false; @@ - parsedMessages = JSON.parse(decodeURIComponent(windowLocaleData)); - parsedLocale = Object.keys(parsedMessages)[0] || ''; - nonDefaultLocale = parsedLocale !== defaultLocale; + parsedMessages = JSON.parse(decodeURIComponent(windowLocaleData)); + const keys = Object.keys(parsedMessages as Record<string, unknown>); + parsedLocale = keys.length ? keys[0] : defaultLocale; + nonDefaultLocale = keys.length ? parsedLocale !== defaultLocale : false;
♻️ Duplicate comments (1)
web/vite.web-component.ts (1)
2-2: Remove unused import 'path'.Matches the CodeQL finding.
-import path from 'path';
| "typescript": "5.9.2", | ||
| "pify": "^6.1.0" | ||
| }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Pin devDependency version and confirm it’s actually needed.
- All other devDependencies here are pinned;
pifyuses a caret. For consistency and reproducible builds, pin it. - Also, verify if
pifyis used at all in this package. If not, drop it. If you just need to promisify Node APIs in build/test, preferutil.promisifyfrom Node core over adding a new dep.
Apply to pin the version:
- "pify": "^6.1.0"
+ "pify": "6.1.0"Check for usage across the repo:
🏁 Script executed:
#!/bin/bash
# Where is pify imported/required?
rg -nP --glob '!**/dist/**' '(from\s+["'\'']pify["'\'']|require\(["'\'']pify["'\'']\))' -C2Length of output: 93
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1. Check for import/require usage of pify (excluding dist)
rg -nP --glob '!**/dist/**' "(require\(['\"]pify['\"]\)|import\s+pify\s+from\s+['\"]pify['\"])" -C2 || echo "No require/import usage found."
# 2. Fallback: any reference to "pify" in source (excluding dist)
rg -nP --glob '!**/dist/**' -n "pify" -C2 || echo "No references to pify found."Length of output: 5424
Remove unused pify devDependency
Drop "pify": "^6.1.0" from devDependencies—it's not imported or referenced in this package.
🤖 Prompt for AI Agents
In packages/unraid-api-plugin-health/package.json around lines 24 to 26, remove
the unused devDependency "pify": "^6.1.0" from the devDependencies block; after
deleting the entry, update the lockfile (npm/yarn/pnpm) or run an install to
ensure the dependency graph is consistent.
| "graphql-ws": "6.0.6", | ||
| "lodash-es": "4.17.21", | ||
| "nest-authz": "2.17.0", | ||
| "pify": "^6.1.0", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Double-check necessity; prefer Node’s util.promisify if possible.
If pify isn’t directly used in shared’s build/tests, remove it to keep the surface minimal. Otherwise, keep and pin.
Repo-wide usage check:
🏁 Script executed:
#!/bin/bash
rg -nP --glob '!**/dist/**' '(from\s+["'\'']pify["'\'']|require\(["'\'']pify["'\'']\))' -C2Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for any occurrence of "pify" outside of dist and the package.json itself
rg -n --glob '!**/dist/**' --glob '!packages/unraid-shared/package.json' pify -C2Length of output: 4919
Remove unused pify dependency from packages/unraid-shared/package.json.
pify isn’t imported or required anywhere in this package, so it can be safely removed.
🤖 Prompt for AI Agents
In packages/unraid-shared/package.json around line 47, the dependency "pify":
"^6.1.0" is unused in this package; remove the "pify" entry from the
dependencies block in package.json and run yarn/npm install (or update lockfile)
to ensure the lockfile is refreshed; commit the modified package.json and
updated lockfile.
| declare global { | ||
| const avatarGroupInjectionKey: typeof import('../node_modules/.pnpm/@nuxt+ui@4.0.0-alpha.0_@babel+parser@7.28.3_@netlify+blobs@9.1.2_change-case@5.4.4_db0@_717626353d7e2aa0e50ed397345224b8/node_modules/@nuxt/ui/dist/runtime/composables/useAvatarGroup.js')['avatarGroupInjectionKey'] | ||
| const defineLocale: typeof import('../node_modules/.pnpm/@nuxt+ui@4.0.0-alpha.0_@babel+parser@7.28.3_@netlify+blobs@9.1.2_change-case@5.4.4_db0@_717626353d7e2aa0e50ed397345224b8/node_modules/@nuxt/ui/dist/runtime/composables/defineLocale.js')['defineLocale'] | ||
| const defineShortcuts: typeof import('../node_modules/.pnpm/@nuxt+ui@4.0.0-alpha.0_@babel+parser@7.28.3_@netlify+blobs@9.1.2_change-case@5.4.4_db0@_717626353d7e2aa0e50ed397345224b8/node_modules/@nuxt/ui/dist/runtime/composables/defineShortcuts.js')['defineShortcuts'] |
There was a problem hiding this comment.
Name collision: defineShortcuts clashes with your local defineShortcuts composable.
Auto-import may resolve to @nuxt/ui’s defineShortcuts instead of your implementation.
Action: remove defineShortcuts from auto-imports, or rename your local helper (e.g., bindShortcuts).
I can adjust the AutoImport config to exclude these names.
🤖 Prompt for AI Agents
In web/auto-imports.d.ts around line 11, the auto-imported identifier
defineShortcuts conflicts with a local composable of the same name; remove
defineShortcuts from the auto-imports config or rename the local helper (e.g.,
bindShortcuts) so the auto-import cannot shadow your implementation, and update
any imports/usages accordingly; alternatively, adjust your AutoImport config to
explicitly exclude 'defineShortcuts' to ensure your local composable is used.
…tyles in unraid-next layout
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1655 +/- ##
==========================================
+ Coverage 54.56% 58.48% +3.92%
==========================================
Files 809 624 -185
Lines 45457 30710 -14747
Branches 4675 3596 -1079
==========================================
- Hits 24804 17962 -6842
+ Misses 20616 12711 -7905
Partials 37 37 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <b>ID:</b> {{ key.id.split(':')[1] }} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against missing “:” in ID
split(':')[1] can be undefined. Show a safe fallback.
- <div class="max-w-[250px] truncate text-sm md:max-w-md">
- <b>ID:</b> {{ key.id.split(':')[1] }}
+ <div class="max-w-[250px] truncate text-sm md:max-w-md">
+ <b>ID:</b> {{ key.id.includes(':') ? key.id.split(':')[1] : key.id }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <b>ID:</b> {{ key.id.split(':')[1] }} | |
| </div> | |
| <div class="max-w-[250px] truncate text-sm md:max-w-md"> | |
| <b>ID:</b> {{ key.id.includes(':') ? key.id.split(':')[1] : key.id }} | |
| </div> |
🤖 Prompt for AI Agents
In web/src/components/ApiKey/ApiKeyManager.vue around lines 256-257, the
template uses key.id.split(':')[1] which can be undefined if key.id is missing
or contains no colon; update the expression to safely handle these cases by
checking that key.id is a string and contains a colon (or use indexOf) and
render a fallback (e.g. the whole id or a placeholder like '—') when the split
second part is absent.
| <div v-if="key.roles.length" class="flex flex-wrap items-center gap-2"> | ||
| <span class="text-sm"><b>Roles:</b></span> | ||
| <Badge v-for="role in key.roles" :key="role" variant="blue" size="xs">{{ | ||
| role | ||
| }}</Badge> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
roles may be undefined — add optional chaining
Elsewhere you use optional chaining; keep it consistent here to avoid runtime errors.
- <div v-if="key.roles.length" class="flex flex-wrap items-center gap-2">
+ <div v-if="key.roles?.length" class="flex flex-wrap items-center gap-2">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div v-if="key.roles.length" class="flex flex-wrap items-center gap-2"> | |
| <span class="text-sm"><b>Roles:</b></span> | |
| <Badge v-for="role in key.roles" :key="role" variant="blue" size="xs">{{ | |
| role | |
| }}</Badge> | |
| </div> | |
| <div v-if="key.roles?.length" class="flex flex-wrap items-center gap-2"> | |
| <span class="text-sm"><b>Roles:</b></span> | |
| <Badge | |
| v-for="role in key.roles" | |
| :key="role" | |
| variant="blue" | |
| size="xs" | |
| > | |
| {{ role }} | |
| </Badge> | |
| </div> |
🤖 Prompt for AI Agents
In web/src/components/ApiKey/ApiKeyManager.vue around lines 262 to 267, the
template assumes key.roles is always defined; change the conditional and loop to
use optional chaining or a fallback array to avoid runtime errors. Replace
v-if="key.roles.length" with v-if="key.roles?.length" and change v-for="role in
key.roles" to v-for="role in key.roles ?? []" so the badge rendering safely
handles undefined roles.
| <span>root@{{ item.id }}:/# </span> | ||
| <input | ||
| v-model="command" | ||
| class="ml-1 flex-1 bg-transparent outline-none" | ||
| type="text" | ||
| @keyup.enter="executeCommand" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Fix undefined template var: use props.item.id (or destructure) instead of item.id
item is not defined in <script setup>; this will render blank or throw. Use props.item.id or destructure from defineProps.
- <span>root@{{ item.id }}:/# </span>
+ <span>root@{{ props.item.id }}:/# </span>🤖 Prompt for AI Agents
In web/src/components/Docker/Console.vue around lines 49 to 56, the template
references item.id but item is not defined in the <script setup>; replace
item.id with props.item.id or destructure item from defineProps (e.g., const {
item } = defineProps(...)) in the script so the template binds to a defined
prop; update the script accordingly and ensure the prop is declared with the
correct type.
| <UButton color="primary" variant="outline">Cancel</UButton> | ||
| <UButton color="primary">Save Changes</UButton> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wire up Cancel/Save actions
Buttons are inert. Emit events so the parent can handle dismiss/persist.
- <UButton color="primary" variant="outline">Cancel</UButton>
- <UButton color="primary">Save Changes</UButton>
+ <UButton color="primary" variant="outline" @click="onCancel">Cancel</UButton>
+ <UButton color="primary" @click="onSave">Save Changes</UButton>Add in <script setup>:
const emit = defineEmits<{
(e: 'cancel'): void;
(e: 'save', payload: ContainerConfig): void;
}>();
function onCancel() {
emit('cancel');
}
function onSave() {
emit('save', structuredClone(config.value));
}🤖 Prompt for AI Agents
In web/src/components/Docker/Edit.vue around lines 39 to 41, the Cancel and Save
buttons are inert; wire them to emit events so the parent can handle
dismiss/persist. Add a defineEmits signature for 'cancel' and 'save' (save
carries a ContainerConfig payload), implement onCancel() to emit 'cancel' and
onSave() to emit 'save' passing a structured clone of config.value, and attach
onCancel to the Cancel button and onSave to the Save Changes button.
| const props = defineProps<Props>(); | ||
| const previewUrl = props.port ? `http://localhost:${props.port}` : null; | ||
| </script> |
There was a problem hiding this comment.
previewUrl isn’t reactive to prop changes
Compute it so it updates when port changes.
-const props = defineProps<Props>();
-const previewUrl = props.port ? `http://localhost:${props.port}` : null;
+const props = defineProps<Props>();
+const previewUrl = computed(() => (props.port ? `http://localhost:${props.port}` : null));Remember to import computed from 'vue'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const props = defineProps<Props>(); | |
| const previewUrl = props.port ? `http://localhost:${props.port}` : null; | |
| </script> | |
| <script setup lang="ts"> | |
| import { computed } from 'vue'; | |
| const props = defineProps<Props>(); | |
| const previewUrl = computed(() => (props.port ? `http://localhost:${props.port}` : null)); | |
| </script> |
🤖 Prompt for AI Agents
In web/src/components/Docker/Preview.vue around lines 12 to 14, the previewUrl
is computed once from props and doesn't update when props.port changes; import
computed from 'vue' and replace the plain const assignment with a computed that
returns props.port ? `http://localhost:${props.port}` : null so the value is
reactive to prop updates.
| import { ref } from 'vue'; | ||
| import { storeToRefs } from 'pinia'; | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import computed; we’ll use it to gate actions based on window APIs.
Preps for disabling buttons when the global hooks are unavailable.
-import { ref } from 'vue';
+import { ref, computed } from 'vue';
import { storeToRefs } from 'pinia';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ref } from 'vue'; | |
| import { storeToRefs } from 'pinia'; | |
| import { ref, computed } from 'vue'; | |
| import { storeToRefs } from 'pinia'; |
🤖 Prompt for AI Agents
In web/src/components/UpdateOs/Downgrade.vue around lines 2 to 4, the component
imports ref from 'vue' but not computed; add computed to the import statement
(import { ref, computed } from 'vue') so you can create computed properties to
gate actions based on window APIs and disable buttons when global hooks are
unavailable.
| // Check if content contains ANSI escape sequences | ||
| // eslint-disable-next-line no-control-regex | ||
| const hasAnsiSequences = /\x1b\[/.test(content); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Satisfy Biome noControlCharactersInRegex
Replace the regex literal with a string includes check to avoid control chars in a regex.
Apply:
- // Check if content contains ANSI escape sequences
- // eslint-disable-next-line no-control-regex
- const hasAnsiSequences = /\x1b\[/.test(content);
+ // Check if content contains ANSI escape sequences (ESC [)
+ const hasAnsiSequences = content.includes('\u001b[');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if content contains ANSI escape sequences | |
| // eslint-disable-next-line no-control-regex | |
| const hasAnsiSequences = /\x1b\[/.test(content); | |
| // Check if content contains ANSI escape sequences (ESC [) | |
| const hasAnsiSequences = content.includes('\u001b['); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 53-53: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🤖 Prompt for AI Agents
In web/src/composables/useContentHighlighting.ts around lines 51 to 54, the
check for ANSI escape sequences uses a regex literal with control characters
which trips the Biome noControlCharactersInRegex rule; replace the regex test
with a plain string check (e.g., use includes or indexOf) to detect the
substring "\x1b[" in content, remove the eslint-disable comment, and keep the
logic otherwise unchanged so the code still detects ANSI sequences without using
a control-character regex.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/__test__/components/Wrapper/mount-engine.test.ts (2)
353-359: Don’t assert exact error messages; assert throwing.Align with testing rules: avoid exact message strings.
-}).toThrow('Component error'); +}).toThrow(Error);
169-183: Align test name with implemented behavior or update to assert JSON parsing
- Rename the test to reflect that JSON-like attribute values remain strings and assert the rendered output, e.g.:
- it('should parse JSON props from attributes', () => { + it('should treat JSON-like attribute values as strings (no JSON parsing)', () => { // … - expect(element.getAttribute('message')).toBe('{"text": "JSON Message"}'); + expect(element.textContent).toBe('{"text": "JSON Message"}'); });- Or, if the intent is to parse JSON into props, implement JSON parsing in mountVueApp and update this test to assert the rendered object field(s).
🧹 Nitpick comments (14)
web/__test__/components/Wrapper/mount-engine.test.ts (13)
17-23: Drop pass-through mock of 'vue'; spy only where needed.Mocking 'vue' just to return the actual module is redundant and can complicate ESM mocking. Remove this block and use vi.spyOn on the real module in tests that need it.
-// We'll manually mock createApp only in specific tests that need it -vi.mock('vue', async () => { - const actual = await vi.importActual<typeof import('vue')>('vue'); - return { - ...actual, - }; -}); +// Use vi.spyOn(require('vue'), 'createApp') in specific tests that need it
77-107: Clear mocks before creating spies.Call vi.clearAllMocks() at the start of beforeEach to avoid wiping newly-created spies and to make intent explicit.
beforeEach(async () => { + vi.clearAllMocks(); const module = await import('~/components/Wrapper/mount-engine'); @@ - vi.clearAllMocks();
184-198: Same issue for HTML-encoded JSON test.Verify rendered output or clarify that values remain strings; attribute equality alone isn’t meaningful.
-it('should handle HTML-encoded JSON in attributes', () => { +it('should treat HTML-encoded JSON in attributes as raw string', () => { @@ - expect(app).toBeTruthy(); - expect(element.getAttribute('message')).toBe('{"text": "Encoded"}'); + expect(app).toBeTruthy(); + expect(element.textContent).toBe('{"text": "Encoded"}');
260-263: Avoid brittle assertions on exact log strings.Per repo testing rules, assert behavior not exact log text. Use stringContaining or just verify the call.
- expect(consoleWarnSpy).toHaveBeenCalledWith('[VueMountApp] App test-app is already mounted'); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('already mounted')); - expect(consoleDebugSpy).toHaveBeenCalledWith( - '[VueMountApp] Modals component already mounted as modals, skipping unraid-modals' - ); + expect(consoleDebugSpy).toHaveBeenCalledWith(expect.stringContaining('Modals component already mounted')); - expect(consoleWarnSpy).toHaveBeenCalledWith( - '[VueMountApp] Element #app has Vue attributes but no content, cleaning up' - ); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('cleaning up')); - expect(consoleWarnSpy).toHaveBeenCalledWith( - '[VueMountApp] Cleaning up problematic nodes in #app before mounting' - ); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('Cleaning up problematic nodes')); - expect(consoleWarnSpy).toHaveBeenCalledWith( - '[VueMountApp] No elements found for selector: #non-existent' - ); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('#non-existent')); - expect(consoleWarnSpy).toHaveBeenCalledWith('[VueMountApp] No app found with id: non-existent'); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('No app found with id')); - expect(consoleDebugSpy).toHaveBeenCalledWith( - '[VueMountApp] Modals component already mounted, skipping #modals' - ); + expect(consoleDebugSpy).toHaveBeenCalledWith(expect.stringContaining('Modals component already mounted'));Also applies to: 286-291, 307-309, 325-327, 337-340, 498-499, 609-611
613-623: Immediate mount doesn’t need timers.If mounting is synchronous for this selector, drop runAllTimersAsync and the async test wrapper.
-it('should mount immediately for all selectors', async () => { +it('should mount immediately for all selectors', () => { @@ - // Component should mount immediately without delay - await vi.runAllTimersAsync(); expect(element.querySelector('.test-component')).toBeTruthy();
636-639: Avoid string-coupled negative assertions on logs.Prefer verifying no warnings were emitted at all to make the test resilient.
-expect(consoleWarnSpy).not.toHaveBeenCalledWith( - expect.stringContaining('No valid DOM elements found') -); +expect(consoleWarnSpy).not.toHaveBeenCalled();
569-592: Restore document.readyState after mutation.Modifying document.readyState can leak across tests. Save/restore the original descriptor within the test.
-it('should wait for DOMContentLoaded if document is loading', async () => { +it('should wait for DOMContentLoaded if document is loading', async () => { + const original = Object.getOwnPropertyDescriptor(document, 'readyState'); Object.defineProperty(document, 'readyState', { value: 'loading', writable: true, }); @@ document.dispatchEvent(new Event('DOMContentLoaded')); await vi.runAllTimersAsync(); expect(element.querySelector('.test-component')).toBeTruthy(); + if (original) Object.defineProperty(document, 'readyState', original); });
818-826: Avoid brittle error-message matching; assert error type.Use toThrow(TypeError) instead of matching the exact string.
-}).toThrow('Cannot read property nextSibling of null'); +}).toThrow(TypeError);
75-76: Remove unused testContainer.testContainer is created/attached but never used.
- let testContainer: HTMLDivElement; @@ - testContainer = document.createElement('div'); - testContainer.id = 'test-container'; - document.body.appendChild(testContainer); + // (removed unused container)Also applies to: 102-105
709-716: Add ambient Window typings for mountedApps/globalPinia/LOCALE_DATA to drop casts.Avoid “as unknown as …” and implicit anys by augmenting Window in tests.
Add this test-only d.ts (e.g., web/test/types/window.d.ts):
declare global { interface Window { mountedApps: Map<string, unknown>; globalPinia: unknown; LOCALE_DATA?: string; } } export {};Then remove casts like (window as unknown as Record<string, unknown>).
Also applies to: 831-840, 108-112, 114-121
696-708: Strengthen i18n tests with observable assertions.Currently “parse window locale data” just sets/clears data. Assert i18n was (re)initialized or messages applied.
mountVueApp({ component: TestComponent, selector: '#i18n-app', }); - delete (window as unknown as Record<string, unknown>).LOCALE_DATA; + expect(mockI18n.install).toHaveBeenCalled(); + delete (window as unknown as Record<string, unknown>).LOCALE_DATA;If you want to assert messages, also vi.spyOn(createI18n, 'mock') and inspect args.
Also applies to: 709-727
392-415: Multi-mount: assert disconnected element was not mounted.Add a negative assertion to ensure only connected nodes receive mounts.
const app = mountVueApp({ component: TestComponent, selector: '.multi', }); expect(app).toBeTruthy(); // The app should mount only to the connected element expect(validElement.querySelector('.test-component')).toBeTruthy(); +expect(disconnectedElement.querySelector('.test-component')).toBeFalsy();
521-525: Loosen unmount error log assertion.Keep the Error instance check but avoid coupling to exact prefix.
-expect(consoleWarnSpy).toHaveBeenCalledWith( - '[VueMountApp] Error unmounting app test-app:', - expect.any(Error) -); +expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Error unmounting app'), + expect.any(Error) +);@tailwind-shared/css-variables.css (1)
19-27: Fix HSL↔hex mismatch for orange and keep ring in sync.
24 100% 50%maps to#ff7f00, not#ff8c2fas the comment states. If the intent is brand orange#ff8c2f, use ≈27 100% 59%in both--primaryand--ringto avoid drift between code and docs.Apply:
- --primary: 24 100% 50%; /* Orange #ff8c2f in HSL */ + --primary: 27 100% 59%; /* Orange #ff8c2f in HSL */ @@ - --ring: 24 100% 50%; /* Orange ring to match primary */ + --ring: 27 100% 59%; /* Orange ring to match primary */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
@tailwind-shared/css-variables.css(2 hunks)web/__test__/components/Wrapper/mount-engine.test.ts(13 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}: Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Use vi.spyOn() to specify return values or behavior of methods under test
Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
**/__test__/components/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/__test__/components/**/*.ts: Component tests should use mount from @vue/test-utils
Stub complex child components that are not the focus of a component test
Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Use createTestingPinia() to mock Pinia stores when testing Vue components
Test that expected elements are rendered and verify component output rather than internals
Test component interactions (clicks, inputs) using await element.trigger and setValue, and verify emitted events via wrapper.emitted()
Prefer semantic queries like find('button') or CSS selectors; avoid relying on data-test IDs unless necessary
Assert rendered text with wrapper.text(), check attributes via element.attributes(), and verify element existence with expect(element.exists()).toBe(true)
In component tests that use real store implementations, configure createTestingPinia with stubActions: false when you need real action behavior
Use findComponent(ComponentName) for locating child components and findAll for collections
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
{api,web}/**/*.{spec,test}.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{spec,test}.@(ts|tsx|js|jsx): When testing for errors, use .rejects.toThrow() without asserting specific message strings
Focus tests on behavior, not implementation details like exact error messages
Avoid brittle tests that depend on non-essential details (e.g., log formats)
Use mocks as nouns (objects), not verbs (functions)
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
web/__test__/**/*.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
Web tests are located under web/test
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
web/**/*.{spec,test}.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{spec,test}.@(ts|tsx|js|jsx): Use mount from Vue Test Utils for component tests
Stub complex child components that are not the focus of the test
Mock external dependencies and services in component tests
Test component behavior and rendered output, not internal implementation details
Use createTestingPinia() when mocking stores in component tests
Prefer semantic element queries like find('button') over data-test IDs
Use await nextTick() for DOM updates before assertions
Always await async operations before making assertions
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use the any type; prefer proper typing
Avoid using type assertions/casting; prefer proper typing from the start
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Use mount from Vue Test Utils for component tests
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Component tests should use mount from vue/test-utils
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Use mount from Vue Test Utils for component tests
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Test that expected elements are rendered and verify component output rather than internals
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Test component behavior and rendered output, not internal implementation details
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Test component interactions (clicks, inputs) using await element.trigger and setValue, and verify emitted events via wrapper.emitted()
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/components/**/*.vue : Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Use createTestingPinia() to mock Pinia stores when testing Vue components
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Assert rendered text with wrapper.text(), check attributes via element.attributes(), and verify element existence with expect(element.exists()).toBe(true)
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Use await nextTick() for DOM updates before assertions
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
🔇 Additional comments (1)
@tailwind-shared/css-variables.css (1)
19-27: Verify token alignment across theme layers.Ensure these primary/ring changes match:
@tailwind-shared/unraid-theme.cssand@tailwind-shared/theme-variants.csstoken mappings.- Any app-level config (e.g.,
web/app.config.ts) that sets UI primary surfaces.- Tests under
web/__test__/store/theme.test.ts.Mismatches will cause inconsistent focus rings and button text colors across themes.
Also applies to: 51-59
- Added comprehensive CSS variables for Nuxt UI color system, including primary and neutral color shades for both light and dark modes. - Updated theme variant styles to improve consistency across different themes, including adjustments to header styles and background colors. - Introduced new button styles to override Unraid's default button behavior for better integration with Nuxt UI components. - Enhanced type declarations for new components, ensuring proper integration within the Vue ecosystem.
…orts - Removed references to unused components `UModal` and `USlideover` from type declarations in `components.d.ts`. - Simplified the `tsconfig.json` by removing unnecessary references to other configuration files.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
@tailwind-shared/css-variables.css (2)
69-77: Fix HSL mismatch and WCAG contrast for --primary (light mode).
--primary: 24 100% 50%doesn’t match#ff8c2f(27° 100% 59%) and white--primary-foregroundfails 4.5:1 contrast on this orange. Align HSL and swap foreground to near-black for compliance.Apply:
- --primary: 24 100% 50%; /* Orange #ff8c2f in HSL */ - --primary-foreground: 0 0% 98%; + --primary: 27 100% 59%; /* Orange #ff8c2f in HSL */ + --primary-foreground: 0 0% 10%; /* ≥4.5:1 on bright orange */ @@ - --ring: 24 100% 50%; /* Orange ring to match primary */ + --ring: 27 100% 59%; /* Orange ring to match primary */If you must keep white text, instead lower
--primarylightness to ≤40% and re-check.Run to verify contrast in both modes:
#!/usr/bin/env bash set -euo pipefail f=$(fd -a '@tailwind-shared/css-variables.css' || true) [[ -z "${f:-}" ]] && { echo "file not found"; exit 1; } python3 - "$f" << 'PY' import re, sys, colorsys, json css=open(sys.argv[1]).read() def pick(block, var): m=re.search(rf"{block}.*?{var}:\s*([\d.]+)\s+([\d.]+)%\s+([\d.]+)%", css, re.S) return tuple(map(float, m.groups())) if m else None def hsl_to_rgb(h,s,l): r,g,b=colorsys.hls_to_rgb(h/360.0,l/100.0,s/100.0); return (r*255,g*255,b*255) def rl(rgb): def c(x): x/=255; return x/12.92 if x<=0.03928 else ((x+0.055)/1.055)**2.4 r,g,b=rgb; return 0.2126*c(r)+0.7152*c(g)+0.0722*c(b) def contrast(a,b): L1,L2=rl(a),rl(b); return (max(L1,L2)+0.05)/(min(L1,L2)+0.05) for block,name in [(":root","light"), (r"\.dark\s*\{","dark")]: hsl_p=pick(block,"--primary"); hsl_pf=pick(block,"--primary-foreground") if not hsl_p or not hsl_pf: print(name, "missing vars"); continue cr=contrast(hsl_to_rgb(*hsl_p), hsl_to_rgb(*hsl_pf)) print(json.dumps({"mode":name,"primary":hsl_p,"foreground":hsl_pf,"contrast":round(cr,2),"meets_aa":cr>=4.5})) PY
122-130: Dark mode: same HSL mismatch and insufficient contrast.Mirror the light-mode fix; white on bright orange also fails here.
- --primary: 24 100% 50%; /* Orange #ff8c2f in HSL */ - --primary-foreground: 0 0% 98%; + --primary: 27 100% 59%; /* Orange #ff8c2f in HSL */ + --primary-foreground: 0 0% 10%; /* ≥4.5:1 on bright orange */ @@ - --ring: 24 100% 50%; /* Orange ring to match primary */ + --ring: 27 100% 59%; /* Orange ring to match primary */Alternatively keep white text by lowering primary lightness to ≤40% in dark mode as well.
🧹 Nitpick comments (6)
web/src/components/ResponsiveModal.vue (2)
15-18: Avoid width classes in contentClass (keep width controlled by the modal container)Per past Modal.vue guidance, width should be managed by the modal/containers, not inner content. Clarify this to prevent misuse.
- // Content class + // Content class (avoid width classes; width is controlled by the modal container/maxWidth) contentClass?: HTMLAttributes['class'];
34-34: Make breakpoint reactive to prop changesPass a getter so useMediaQuery tracks runtime changes to the breakpoint prop.
-const isMobile = useMediaQuery(props.breakpoint); +const isMobile = useMediaQuery(() => props.breakpoint);web/src/assets/main.css (4)
41-47: Avoidtransition: all; limit to paint-only properties.Reduces layout thrash and respects prefers-reduced-motion overrides.
(Handled in the previous diff by specifying properties.)
118-130: Link overrides outside Tailwind layers may unintentionally beat utilities.Placing these outside
@layer+ using a more specific.unapi acan override Tailwind utilities liketext-blue-500 underline. Reduce specificity (or move into a layer) so explicit utilities still win.-/* Override link styles inside .unapi - outside of layer for higher specificity */ -.unapi a, -.unapi a:link, -.unapi a:visited { +/* Override link styles with low specificity to not fight utilities */ +:where(.unapi) a, +:where(.unapi) a:link, +:where(.unapi) a:visited { color: inherit; text-decoration: none; } -.unapi a:hover, -.unapi a:focus { +:where(.unapi) a:hover, +:where(.unapi) a:focus { text-decoration: underline; color: inherit; }If the intent is to always neutralize link visuals, keep it outside layers; otherwise prefer
@layer componentsto ensure utilities can override:+@layer components { + :where(.unapi) a { color: inherit; text-decoration: none; } + :where(.unapi) a:hover, :where(.unapi) a:focus { text-decoration: underline; color: inherit; } +}
86-116: Switch colors should use theme tokens/vars, not fixed hex.Hard-coded colors (#ccc, #ff8c2f, #e5e5e5, #333, #555) won’t adapt across themes. Prefer CSS variables (or Nuxt UI tokens) with sane fallbacks.
.unapi button[role="switch"], .unapi button[role="switch"][data-state="checked"], .unapi button[role="switch"][data-state="unchecked"] { background-color: transparent; background: transparent; - border: 1px solid #ccc; + border: 1px solid var(--ui-border, #d4d4d4); } /* Style for checked state */ .unapi button[role="switch"][data-state="checked"] { - background-color: #ff8c2f; /* Unraid orange */ + background-color: var(--ui-primary, #ff8c2f); /* Unraid orange fallback */ } /* Style for unchecked state */ .unapi button[role="switch"][data-state="unchecked"] { - background-color: #e5e5e5; + background-color: var(--ui-muted, #e5e5e5); } /* Dark mode toggle styles */ .unapi.dark button[role="switch"][data-state="unchecked"], .dark .unapi button[role="switch"][data-state="unchecked"] { - background-color: #333; - border-color: #555; + background-color: var(--ui-muted-900, #333); + border-color: var(--ui-border-700, #555); } /* Toggle thumb/handle */ .unapi button[role="switch"] span { - background-color: white; + background-color: var(--ui-thumb, #fff); }Confirm the actual markup used by your switch (Radix/Reka variants often use data attributes, not necessarily a span). If the thumb isn’t a span, target the correct part:
#!/bin/bash rg -nC2 -P 'role=["'\'']switch["'\'']' --type=vue --type=ts --type=js
9-9: Path robustness for@tailwind-shared/index.css.Workspace-relative
../../../@tailwind-shared/index.csscan be brittle across sub-repos or package moves. Consider publishing@tailwind-sharedas a proper package and importing by package name, or guard the path via Vite alias.Check existence from repo root to avoid build-time errors:
#!/bin/bash set -euo pipefail fd -a -HI --glob '@tailwind-shared/index.css' || echo "Not found: @tailwind-shared/index.css"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
@tailwind-shared/css-variables.css(4 hunks)@tailwind-shared/theme-variants.css(1 hunks)web/components.d.ts(1 hunks)web/src/assets/main.css(3 hunks)web/src/components/ResponsiveModal.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- @tailwind-shared/theme-variants.css
- web/components.d.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
web/src/components/ResponsiveModal.vue
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Use mount from Vue Test Utils for component tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-08-11T15:07:39.222Z
Learning: Applies to api/**/*.{test,spec}.{js,jsx,ts,tsx} : Use Vitest for tests in the api; do not use Jest
📚 Learning: 2025-02-21T18:59:47.977Z
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Applied to files:
web/src/components/ResponsiveModal.vue
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to **/tailwind.config.@(js|ts|mjs|cjs) : Do not add Tailwind config files (using Tailwind v4, config is unnecessary)
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Applied to files:
web/src/assets/main.css
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
PR: unraid/api#972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-03-13T16:17:21.897Z
Learnt from: elibosley
PR: unraid/api#1211
File: unraid-ui/src/components/form/number/NumberFieldInput.vue:1-21
Timestamp: 2025-03-13T16:17:21.897Z
Learning: The unraid-ui is a UI library being published externally, where wrapping third-party components (like those from reka-ui) is an intentional design choice to maintain a consistent interface, standardize styling, and control the exposed API.
Applied to files:
web/src/assets/main.css
📚 Learning: 2024-12-17T14:59:32.458Z
Learnt from: elibosley
PR: unraid/api#972
File: web/store/theme.ts:46-49
Timestamp: 2024-12-17T14:59:32.458Z
Learning: In the `web/store/theme.ts` file of the Unraid web application, the header is intentionally designed to have a light background with dark text in dark mode, and a dark background with light text in light mode.
Applied to files:
@tailwind-shared/css-variables.css
🔇 Additional comments (6)
@tailwind-shared/css-variables.css (1)
31-55: Token scaffolding looks solid.Good separation of semantic UI tokens from palette scales; mappings make sense for light mode.
web/src/components/ResponsiveModal.vue (4)
7-10: Good: explicit attr forwarding patternUsing inheritAttrs: false with explicit v-bind="$attrs" on the rendered variant is the right approach here.
36-39: Nice v-model proxyClean computed proxy for two-way binding from child components up to modelValue.
34-35: Testing: ensure matchMedia is available in Vitest/JSDOMuseMediaQuery depends on window.matchMedia. Verify a test setup/polyfill exists to avoid runtime errors in tests.
#!/bin/bash # Find potential Vitest setup files and check for a matchMedia stub/polyfill. set -euo pipefail echo "Searching for Vitest setup files…" fd -H -a -t f -g "*vitest*.{ts,js}" -g "*setup*.{ts,js}" || true echo echo "Searching for matchMedia usage/polyfills…" rg -nC2 'matchMedia' || true cat <<'NOTE' If no polyfill is present, consider adding in your test setup: --- import { vi } from 'vitest' if (!window.matchMedia) { // minimal stub Object.defineProperty(window, 'matchMedia', { writable: true, value: vi.fn().mockImplementation(q => ({ media: q, matches: false, onchange: null, addListener: vi.fn(), // deprecated removeListener: vi.fn(), // deprecated addEventListener: vi.fn(), removeEventListener: vi.fn(), dispatchEvent: vi.fn(), })), }) } --- NOTE
44-59: No action needed: v-model:open and #body slot usage is correct
UModal/USlideover components control visibility via v-model:open and expose a #body slot for content, matching this implementation.web/src/assets/main.css (1)
5-9: No changes needed—import path and order are correct. The global CSS correctly imports Tailwind first and then@nuxt/uiper Nuxt UI v3 migration docs.
- Added new props to the Button component for improved flexibility, including `as`, `href`, `target`, and `rel`. - Updated the component registry to include new UI components from Nuxt UI. - Refactored the project structure by introducing a new auto-mount entry point and component registry for better organization. - Adjusted TypeScript configurations to include new paths and types for improved type safety. - Removed unused files and components to streamline the codebase, including the default layout and various Docker-related pages. - Enhanced server state management with improved handling of dynamic components and state application. This commit aims to improve the overall usability and maintainability of the UI components while ensuring better integration with the Nuxt framework.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
- Introduced a new script `test:watch` in package.json for easier test execution during development. - Updated Button component to handle optional props `href`, `target`, and `rel` more gracefully by using nullish coalescing. - Enhanced component tests to ensure proper functionality and global exposure of auto-mount features. This commit aims to improve developer experience and component robustness.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
- Moved API key mutation definitions from `apikey.query.ts` to a new file `apikey.mutation.ts` for better organization. - Updated import paths in components to reflect the new structure. - Adjusted GraphQL query and mutation definitions in `gql.ts` to ensure proper type handling and consistency. - Removed unused `ConfigFile` types and related queries to streamline the codebase. This commit enhances the maintainability of API key management and improves the clarity of GraphQL operations.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/__test__/components/Wrapper/mount-engine.test.ts (2)
25-28: Mock the UI plugin default export to fix CI timeouts from app.use(ui).mount-engine calls app.use(ui). The current mock only provides ensureTeleportContainer; without a plugin, Vue can throw/hang in hooks. Add a no-op plugin.
-const mockEnsureTeleportContainer = vi.fn(); -vi.mock('@unraid/ui', () => ({ - ensureTeleportContainer: mockEnsureTeleportContainer, -})); +const mockEnsureTeleportContainer = vi.fn(); +const mockUiPlugin = { install: vi.fn() }; +vi.mock('@unraid/ui', () => ({ + ensureTeleportContainer: mockEnsureTeleportContainer, + default: mockUiPlugin, +}));
75-107: Mock@unraid/uiplugin before imports to avoid CI hook timeouts
- At the very top of
web/__test__/components/Wrapper/mount-engine.test.ts, addvi.mock('@unraid/ui', () => ({ ensureTeleportContainer: () => {}, }));- Move
vi.clearAllMocks()to the start of thebeforeEachso all spies and mocks are reset before plugin imports and spies are initialized.- After mocking, rerun tests; if you still hit timeouts, wrap the dynamic import in a try/catch to surface import errors and ensure no pending timers by calling
await vi.runAllTimersAsync()before each test’s first assertion.
♻️ Duplicate comments (4)
web/public/test-pages/load-manifest.js (2)
8-8: Dev-mode detection still assumes Nuxt (3000); include Vite defaults and a query override.- const isDevMode = window.location.port === '3000' || window.location.hostname === 'localhost'; + const isLocalHost = /^(localhost|127\.0\.0\.1)$/i.test(window.location.hostname); + const isDevPort = ['3000', '5173'].includes(window.location.port); + const isDevQuery = new URLSearchParams(window.location.search).has('dev'); + const isDevMode = isLocalHost || isDevPort || isDevQuery;
32-38: Try an alternate manifest name before falling back to direct load.- const response = await fetch('/dist/manifest.json'); - if (!response.ok) { - console.warn('Manifest not found, attempting direct load'); - // Fallback: try to load files directly without manifest - loadDirectResources(); - return; - } + let response = await fetch('/dist/manifest.json'); + if (!response.ok) { + const alt = await fetch('/dist/standalone.manifest.json'); + if (alt.ok) { + response = alt; + } else { + console.warn('Manifest not found, attempting direct load'); + loadDirectResources(); + return; + } + }web/public/test-pages/test-server-state.js (2)
215-236: Allow opt-out for elements marked data-no-autoserverSkip elements that should not be auto-clobbered by test state.
- elements.forEach((el) => { + elements.forEach((el) => { + if (el.hasAttribute('data-no-autoserver')) return;
151-151: Fix state typo breaking expired flows"EEXPIRED" should be "EXPIRED" to match enums and UI checks.
- state: 'EEXPIRED', + state: 'EXPIRED',
🧹 Nitpick comments (23)
package.json (1)
12-12: Stream watch output and skip missing scripts for a cleaner DXAdd
--streamso watch logs appear live (otherwise output may buffer until exit) and--if-presentto avoid noisy “Missing script” messages in packages without a watch task.- "test:watch": "pnpm -r --parallel test:watch", + "test:watch": "pnpm -r --parallel --stream --if-present test:watch",Optional: cap concurrency for heavy watchers with
--workspace-concurrency=4.unraid-ui/src/components/common/button/Button.vue (2)
11-11: Narrow theasprop type to a union for safety.Constrain usage to supported render modes.
Apply this diff:
- as?: string; + as?: 'a' | 'span';
63-63: Nit:tabindex="0"on anchors is redundant.Anchors with
hrefare natively focusable. Keep-1when disabled; otherwise omit.Apply this diff:
- :tabindex="disabled ? -1 : 0" + :tabindex="disabled ? -1 : undefined"web/src/components/UserProfile.ce.vue (1)
124-124: Minor a11y: add aria-label to ghost button.Screen readers won’t read the title; aria-label improves clarity when the button only shows the server name.
- class="text-header-text-primary flex h-auto items-center p-0 text-base opacity-100 transition-opacity hover:opacity-75 focus:opacity-75" + class="text-header-text-primary flex h-auto items-center p-0 text-base opacity-100 transition-opacity hover:opacity-75 focus:opacity-75" + :aria-label="t('Click to Copy LAN IP {0}', [lanIp])"web/src/components/Modals.ce.vue (1)
22-25: Tighten types and coerce booleans for propsGuarantees boolean for :open and improves TS inference.
Apply:
-const callbackStatus = computed(() => callbackStore.callbackStatus); -const trialModalVisible = computed(() => trialStore.trialModalVisible); -const updateOsModalVisible = computed(() => updateOsStore.updateOsModalVisible); -const changelogModalVisible = computed(() => updateOsStore.changelogModalVisible); +const callbackStatus = computed(() => callbackStore.callbackStatus); // inferred string union if store typed +const trialModalVisible = computed<boolean>(() => !!trialStore.trialModalVisible); +const updateOsModalVisible = computed<boolean>(() => !!updateOsStore.updateOsModalVisible); +const changelogModalVisible = computed<boolean>(() => !!updateOsStore.changelogModalVisible);web/src/components/Wrapper/auto-mount.ts (1)
18-34: Make initialization idempotentPrevents double theme init and duplicate globals if called twice.
Apply:
-// Initialize global dependencies once -function initializeGlobalDependencies() { +let __autoMountInitialized = false; +// Initialize global dependencies once +function initializeGlobalDependencies() { if (typeof window === 'undefined') return; + if (__autoMountInitialized) return; + __autoMountInitialized = true;web/__test__/components/component-registry.test.ts (3)
123-131: Avoid stubbing core DOM APIs; this can mask DOMContentLoaded behavior.The no-op addEventListener stub may suppress listeners the module relies on. Also, stubbing head.appendChild to not actually append can hide regressions. Prefer leaving these as-is unless you need to intercept a specific call.
- // Mock document methods - vi.spyOn(document.head, 'appendChild').mockImplementation(() => document.createElement('style')); - vi.spyOn(document, 'addEventListener').mockImplementation(() => {}); + // Prefer real DOM methods; stub only if a test must intercept a specific call + // (keep default implementations to avoid masking readiness logic)
128-129: Silence static-analysis XSS noise by clearing nodes without innerHTML.Minor test-only improvement.
- document.head.innerHTML = ''; - document.body.innerHTML = ''; + while (document.head.firstChild) document.head.removeChild(document.head.firstChild); + while (document.body.firstChild) document.body.removeChild(document.body.firstChild);
161-185: Assert single invocation to catch accidental double auto-mounts.Prevents regressions where multiple listeners fire.
- expect(mockAutoMountAllComponents).toHaveBeenCalled(); + expect(mockAutoMountAllComponents).toHaveBeenCalledTimes(1);web/__test__/components/Wrapper/mount-engine.test.ts (8)
18-23: Remove pass-through mock of 'vue'; it’s unnecessary and may complicate module resolution.-// We'll manually mock createApp only in specific tests that need it -vi.mock('vue', async () => { - const actual = await vi.importActual<typeof import('vue')>('vue'); - return { - ...actual, - }; -}); +// Use the real 'vue' by default; mock selectively where needed (see recovery test)
218-233: Shadow DOM test: guard for environments without attachShadow.Some CI/JSDOM versions lack Shadow DOM. Skip gracefully to avoid false negatives/timeouts.
- it('should use shadow root when specified', () => { + it('should use shadow root when specified', () => { + if (!(HTMLElement.prototype as any).attachShadow) { + vi.skip(); + return; + }
272-282: Relax exact log assertion to reduce brittleness.Minor improvement against wording changes.
- expect(consoleWarnSpy).toHaveBeenCalledWith( - '[VueMountApp] No elements found for any selector: #non-existent' - ); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('No elements found for any selector') + );
316-321: Prefer replaceChildren over innerHTML for clearing.Prevents static-analysis warnings and is clearer.
- document.body.innerHTML = ''; + document.body.replaceChildren();
416-420: Loosen exact warning match.Keeps the test resilient to wording tweaks.
- expect(consoleWarnSpy).toHaveBeenCalledWith('[VueMountApp] No app found with id: non-existent'); + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining('No app found with id'));
529-531: Loosen exact debug match for singleton guard.- expect(consoleDebugSpy).toHaveBeenCalledWith( - '[VueMountApp] Component already mounted as modals for selector #modals, returning existing instance' - ); + expect(consoleDebugSpy).toHaveBeenCalledWith( + expect.stringContaining('Component already mounted as') + );
541-543: Comment and behavior mismatch.It’s not strictly “immediate”; polling is driven by timers. Remove or reword to avoid confusion.
- // Component should mount immediately without delay - await vi.runAllTimersAsync(); + // Flush polling timers to trigger mount + await vi.runAllTimersAsync();
714-722: Avoid asserting window.mountedApps; prefer the public API.Relying on globals couples tests to implementation details.
- it('should expose mountedApps globally', () => { - expect(window.mountedApps).toBeDefined(); - expect(window.mountedApps).toBeInstanceOf(Map); - }); + it('should track mounted apps via the public API', () => { + // Covered indirectly by getMountedApp/unmountVueApp tests + expect(typeof getMountedApp).toBe('function'); + });web/public/test-pages/load-manifest.js (1)
51-57: Stabilize element IDs to avoid rare collisions.Keys can contain slashes or timestamps. A lightweight hash avoids accidental ID collisions across builds.
- const elementId = `unraid-${key.replace(/[^a-zA-Z0-9-]/g, '-')}`; + const hash = Array.from(key).reduce((h, c) => ((h << 5) - h) + c.charCodeAt(0) | 0, 0); + const elementId = `unraid-${Math.abs(hash)}`;web/public/test-pages/test-server-state.js (1)
173-195: Optionally re-apply state to components on updateConvenience: add a flag to re-apply after emitting the event.
- window.updateTestServerState = function (type = 'default', updates = {}) { + window.updateTestServerState = function (type = 'default', updates = {}, apply = false) { @@ - console.log('Server state updated:', type, updates); + console.log('Server state updated:', type, updates); + if (apply) { + window.applyServerStateToComponents(type); + }web/src/components/Wrapper/mount-engine.ts (4)
575-581: Scope cleanup to this app only; avoid touching other mounted appsCurrent unmount removes data-vue-mounted on all .unapi nodes. Tag targets with data-app-id at mount and scope the cleanup.
@@ - // Clean up data attributes from mounted elements - const elements = document.querySelectorAll(`[data-vue-mounted="true"]`); - elements.forEach((el) => { - if (el.classList.contains('unapi')) { - el.removeAttribute('data-vue-mounted'); - } - }); + // Clean up only elements belonging to this app + const elements = document.querySelectorAll( + `[data-vue-mounted="true"][data-app-id="${appId}"]` + ); + elements.forEach((el) => { + el.removeAttribute('data-vue-mounted'); + el.removeAttribute('data-app-id'); + });Tag direct mounts with appId when mounting:
@@ // Add unapi class for minimal styling and mark as mounted mountTarget.classList.add('unapi'); mountTarget.setAttribute('data-vue-mounted', 'true'); + mountTarget.setAttribute('data-app-id', canonicalAppId);And before mounting main app in direct path, ensure element still present (existing code already checks).
Also applies to: 395-397, 450-461
631-639: Tighten async component typingUse Vue Component instead of object to align with TS guidelines.
-// Type for Vue component module -type VueComponentModule = { default: object } | object; +// Type for Vue component module +type VueComponentModule = { default: Component } | Component;
259-261: Avoid innerHTML clearing to satisfy security lintersFunctionally safe here, but many scanners flag innerHTML writes. Prefer child removal.
Add helper (outside selected ranges):
function clearChildren(el: Element) { while (el.firstChild) el.removeChild(el.firstChild); }Then replace occurrences:
- element.innerHTML = ''; + clearChildren(element);- // Force clear everything if normal cleanup fails - element.innerHTML = ''; + // Force clear everything if normal cleanup fails + clearChildren(element);- // If removal fails, clear the entire content - mountTarget.innerHTML = ''; + // If removal fails, clear the entire content + clearChildren(mountTarget);- mountTarget.innerHTML = ''; + clearChildren(mountTarget);Also applies to: 270-275, 375-378, 389-390
634-649: Make async-component timeout configurableFive seconds may be too short on slow links. Allow override via param.
-export function createAsyncComponent(loader: () => Promise<VueComponentModule>) { +export function createAsyncComponent( + loader: () => Promise<VueComponentModule>, + timeoutMs = 5000 +) { return defineAsyncComponent({ @@ - timeout: 5000, // 5 second timeout + timeout: timeoutMs,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
package.json(1 hunks)unraid-ui/src/components/common/button/Button.vue(2 hunks)web/__test__/components/Activation/ActivationModal.test.ts(1 hunks)web/__test__/components/Activation/WelcomeModal.test.ts(4 hunks)web/__test__/components/ColorSwitcher.test.ts(3 hunks)web/__test__/components/DevSettings.test.ts(1 hunks)web/__test__/components/Wrapper/mount-engine.test.ts(12 hunks)web/__test__/components/component-registry.test.ts(1 hunks)web/components.d.ts(1 hunks)web/layouts/default.vue(0 hunks)web/public/test-pages/load-manifest.js(1 hunks)web/public/test-pages/test-server-state.js(1 hunks)web/server/api/debug/validate-token.post.ts(0 hunks)web/server/tsconfig.json(0 hunks)web/src/components/Docker/HeaderContent.vue(1 hunks)web/src/components/LayoutViews/Detail/DetailContentHeader.vue(1 hunks)web/src/components/Modals.ce.vue(1 hunks)web/src/components/UserProfile.ce.vue(4 hunks)web/src/components/Wrapper/auto-mount.ts(1 hunks)web/src/components/Wrapper/component-registry.ts(1 hunks)web/src/components/Wrapper/mount-engine.ts(13 hunks)
💤 Files with no reviewable changes (3)
- web/server/tsconfig.json
- web/layouts/default.vue
- web/server/api/debug/validate-token.post.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- web/test/components/Activation/WelcomeModal.test.ts
- web/test/components/ColorSwitcher.test.ts
- web/test/components/Activation/ActivationModal.test.ts
- web/src/components/LayoutViews/Detail/DetailContentHeader.vue
- web/test/components/DevSettings.test.ts
- web/src/components/Docker/HeaderContent.vue
- web/components.d.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use the any type; prefer proper typing
Avoid using type assertions/casting; prefer proper typing from the start
Files:
web/src/components/Wrapper/auto-mount.tsweb/src/components/Wrapper/component-registry.tsweb/__test__/components/component-registry.test.tsweb/src/components/Wrapper/mount-engine.tsweb/__test__/components/Wrapper/mount-engine.test.ts
**/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
unraid-ui/src/components/common/button/Button.vueweb/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}: Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Use vi.spyOn() to specify return values or behavior of methods under test
Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
**/__test__/components/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
**/__test__/components/**/*.ts: Component tests should use mount from @vue/test-utils
Stub complex child components that are not the focus of a component test
Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Use createTestingPinia() to mock Pinia stores when testing Vue components
Test that expected elements are rendered and verify component output rather than internals
Test component interactions (clicks, inputs) using await element.trigger and setValue, and verify emitted events via wrapper.emitted()
Prefer semantic queries like find('button') or CSS selectors; avoid relying on data-test IDs unless necessary
Assert rendered text with wrapper.text(), check attributes via element.attributes(), and verify element existence with expect(element.exists()).toBe(true)
In component tests that use real store implementations, configure createTestingPinia with stubActions: false when you need real action behavior
Use findComponent(ComponentName) for locating child components and findAll for collections
Files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
{api,web}/**/*.{spec,test}.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{spec,test}.@(ts|tsx|js|jsx): When testing for errors, use .rejects.toThrow() without asserting specific message strings
Focus tests on behavior, not implementation details like exact error messages
Avoid brittle tests that depend on non-essential details (e.g., log formats)
Use mocks as nouns (objects), not verbs (functions)
Files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
web/__test__/**/*.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
Web tests are located under web/test
Files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
web/**/*.{spec,test}.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
web/**/*.{spec,test}.@(ts|tsx|js|jsx): Use mount from Vue Test Utils for component tests
Stub complex child components that are not the focus of the test
Mock external dependencies and services in component tests
Test component behavior and rendered output, not internal implementation details
Use createTestingPinia() when mocking stores in component tests
Prefer semantic element queries like find('button') over data-test IDs
Use await nextTick() for DOM updates before assertions
Always await async operations before making assertions
Files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
🧠 Learnings (37)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
📚 Learning: 2025-09-02T19:35:14.427Z
Learnt from: elibosley
PR: unraid/api#1639
File: web/components/standalone-mount.ts:38-54
Timestamp: 2025-09-02T19:35:14.427Z
Learning: In the unraid/api project, exposing Apollo client and GraphQL utilities (apolloClient, gql, graphqlParse) on the global window object is intentional to allow developers to interact with GraphQL from the browser console without needing to be inside Vue components. Developer experience is prioritized over minimizing global exposure in this context.
Applied to files:
web/src/components/Wrapper/auto-mount.ts
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
package.json
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Run terminal commands with pnpm within the web directory (e.g., pnpm test)
Applied to files:
package.json
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Run API tests with: pnpm --filter ./api test
Applied to files:
package.json
📚 Learning: 2025-08-11T15:07:39.222Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-08-11T15:07:39.222Z
Learning: Use pnpm only (do not use npm/yarn)
Applied to files:
package.json
📚 Learning: 2025-08-11T15:07:39.222Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-08-11T15:07:39.222Z
Learning: Run tests with pnpm --filter ./api test
Applied to files:
package.json
📚 Learning: 2025-03-14T19:14:10.408Z
Learnt from: pujitm
PR: unraid/api#1211
File: plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page:0-0
Timestamp: 2025-03-14T19:14:10.408Z
Learning: *.ce.vue components in `web/components` get built as custom elements and prefixed with `unraid-`, making them available as HTML custom elements at runtime.
Applied to files:
web/src/components/Wrapper/component-registry.ts
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/src/components/Wrapper/component-registry.tsweb/src/components/Wrapper/mount-engine.ts
📚 Learning: 2025-01-27T14:57:46.617Z
Learnt from: elibosley
PR: unraid/api#1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
Applied to files:
web/public/test-pages/test-server-state.js
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Component tests should use mount from vue/test-utils
Applied to files:
web/__test__/components/component-registry.test.tsweb/src/components/Wrapper/mount-engine.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Use mount from Vue Test Utils for component tests
Applied to files:
web/__test__/components/component-registry.test.tsweb/src/components/Wrapper/mount-engine.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Applied to files:
web/__test__/components/component-registry.test.tsweb/src/components/Wrapper/mount-engine.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Use createTestingPinia() to mock Pinia stores when testing Vue components
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/components/**/*.vue : Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Applied to files:
web/__test__/components/component-registry.test.tsweb/src/components/Wrapper/mount-engine.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Test that expected elements are rendered and verify component output rather than internals
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Mock external dependencies and services in component tests (e.g., vi.mock for helper modules)
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Applied to files:
web/__test__/components/component-registry.test.tsweb/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/{store,stores}/**/*.ts : In store files, explicitly import Vue reactivity utilities (e.g., computed, ref, watchEffect); do not rely on Nuxt auto-imports
Applied to files:
web/src/components/Wrapper/mount-engine.tsweb/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
📚 Learning: 2024-12-17T13:55:42.068Z
Learnt from: elibosley
PR: unraid/api#972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Applied to files:
web/src/components/Wrapper/mount-engine.ts
📚 Learning: 2024-12-09T15:47:29.325Z
Learnt from: pujitm
PR: unraid/api#975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Applied to files:
web/src/components/Wrapper/mount-engine.tsweb/src/components/Modals.ce.vue
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
PR: unraid/api#1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/Wrapper/mount-engine.tsweb/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
📚 Learning: 2024-12-06T17:38:40.999Z
Learnt from: elibosley
PR: unraid/api#974
File: web/components/Loading/Error.vue:1-3
Timestamp: 2024-12-06T17:38:40.999Z
Learning: In Nuxt.js projects, components are automatically imported, so explicit import statements for components like `LoadingSpinner` in `web/components/Loading/Error.vue` are not necessary.
Applied to files:
web/src/components/Wrapper/mount-engine.ts
📚 Learning: 2024-12-09T15:45:46.492Z
Learnt from: pujitm
PR: unraid/api#975
File: web/components/Notifications/TabList.vue:1-4
Timestamp: 2024-12-09T15:45:46.492Z
Learning: In our Nuxt.js setup for the `web` project, it's not necessary to explicitly import `computed` from `vue` in Vue components, as it's globally available.
Applied to files:
web/src/components/Wrapper/mount-engine.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Test component behavior and rendered output, not internal implementation details
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Test component interactions (clicks, inputs) using await element.trigger and setValue, and verify emitted events via wrapper.emitted()
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/__test__/components/**/*.ts : Assert rendered text with wrapper.text(), check attributes via element.attributes(), and verify element existence with expect(element.exists()).toBe(true)
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to {**/*.test.ts,**/__test__/{components,store}/**/*.ts} : Use vi.spyOn() to specify return values or behavior of methods under test
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.ts
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Applied to files:
web/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Applied to files:
web/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
📚 Learning: 2025-03-27T23:33:13.215Z
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to function correctly.
Applied to files:
web/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
📚 Learning: 2025-03-27T23:52:57.888Z
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Applied to files:
web/src/components/Modals.ce.vueweb/src/components/UserProfile.ce.vue
🧬 Code graph analysis (5)
web/src/components/Wrapper/auto-mount.ts (1)
web/src/components/Wrapper/mount-engine.ts (4)
mountVueApp(130-533)getMountedApp(590-592)autoMountComponent(595-628)autoMountAllComponents(652-691)
web/public/test-pages/test-server-state.js (1)
web/public/test-pages/load-manifest.js (1)
elements(134-134)
web/public/test-pages/load-manifest.js (1)
web/scripts/add-timestamp-standalone-manifest.js (1)
key(20-20)
web/src/components/Wrapper/mount-engine.ts (2)
web/src/store/globalPinia.ts (1)
globalPinia(4-4)web/src/components/Wrapper/component-registry.ts (1)
componentMappings(32-138)
web/__test__/components/Wrapper/mount-engine.test.ts (1)
web/src/components/Wrapper/mount-engine.ts (4)
mountVueApp(130-533)unmountVueApp(535-588)getMountedApp(590-592)autoMountComponent(595-628)
🪛 ast-grep (0.38.6)
web/__test__/components/component-registry.test.ts
[warning] 127-127: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: document.head.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 128-128: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: document.body.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
web/src/components/Wrapper/mount-engine.ts
[warning] 259-259: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: element.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 270-270: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: element.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 388-388: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: mountTarget.innerHTML = ''
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🪛 GitHub Actions: CI - Main (API)
web/__test__/components/Wrapper/mount-engine.test.ts
[error] 75-75: Hook timed out in 10000ms while mounting Vue app in wrapper tests.
[error] 77-77: Hook timed out in 10000ms (custom props test).
🔇 Additional comments (12)
web/src/components/UserProfile.ce.vue (5)
2-6: LGTM: explicit imports fit Vite/test context.Good move pulling in Vue reactivity APIs and the clipboard composable explicitly; this avoids relying on Nuxt auto-imports during component tests.
Also applies to: 14-14
26-27: Comment rationale is sound.Acknowledged: using computed accessors here avoids timing issues in standalone mounts without Suspense.
32-39: Computed wrappers for store state look good.This is clear and avoids destructuring pitfalls. No action needed.
95-101: LGTM: container/id and banner layer.Structure and layering are consistent; conditional gradient render remains guarded.
136-146: LGTM: dropdown state management.v-model:open with a close event from content is clean and predictable.
web/src/components/Modals.ce.vue (3)
2-2: Good call importing computed explicitlyMatches our test/runtime rules post-Nuxt auto-imports removal.
16-21: Rationale comment is helpfulUsing computed wrappers here is reasonable given async store init without Suspense.
5-9: Remove alias check — ‘~’ is configured
Vite (vite.config.ts 87-89) and tsconfig.json (paths "~/": ["./src/"]) map~to./src, so these imports resolve correctly.web/src/components/Wrapper/auto-mount.ts (1)
55-61: DOMContentLoaded handler is fineEager path covers already-ready DOM; logs and behavior align with mount-engine.
web/src/components/Wrapper/component-registry.ts (2)
49-57: Selectors array for Modals is a nice touchCovers legacy IDs and new tags; good for resilience during migration.
10-16: Eagerly importing critical CEs is appropriateEnsures header/global UI is ready without async jitter.
web/__test__/components/component-registry.test.ts (1)
4-58: Top-level component mocks are correct and hoist-safe.Factory-form vi.mock calls at the file top level follow our testing rules and keep imports isolated.
| as?: string; | ||
| href?: string | null; | ||
| target?: string | null; | ||
| rel?: string | null; |
There was a problem hiding this comment.
Secure target='_blank' links: default rel to 'noopener' to prevent reverse tabnabbing.
If target="_blank" is used without a safe rel, the opener page can be manipulated. Provide a sensible default.
Apply this diff in the template:
- :rel="rel ?? undefined"
+ :rel="anchorRel"Add this computed (outside the shown ranges):
// place near other computeds
const anchorRel = computed(() => {
return props.target === '_blank' ? (props.rel ?? 'noopener') : (props.rel ?? undefined);
});Also applies to: 62-62
🤖 Prompt for AI Agents
In unraid-ui/src/components/common/button/Button.vue around lines 11-14 and 62,
links opened with target="_blank" lack a safe default rel; add a computed named
anchorRel (placed near the other computed properties) that returns props.target
=== '_blank' ? (props.rel ?? 'noopener') : (props.rel ?? undefined), and update
the template anchor element to bind :rel="anchorRel" instead of directly binding
props.rel so that rel defaults to 'noopener' when target is '_blank'.
| :tabindex="disabled ? -1 : 0" | ||
| :aria-disabled="disabled" | ||
| @click="handleClick" | ||
| @keydown="handleKeydown" |
There was a problem hiding this comment.
Avoid double “click” emits on anchors triggered via Enter key.
When as === 'a', the native Enter key already generates a click. Binding @keydown plus synthesizing a click in handleKeydown risks duplicate emits. Guard for anchors in the handler.
Apply this diff:
const handleKeydown = (event: KeyboardEvent) => {
- if (!props.disabled && (event.key === 'Enter' || event.key === ' ')) {
+ // Let native anchor behavior handle keyboard activation
+ if (props.as === 'a') return;
+ if (!props.disabled && (event.key === 'Enter' || event.key === ' ' || event.key === 'Spacebar')) {
event.preventDefault();
// Create a synthetic click event
const clickEvent = new MouseEvent('click', {
bubbles: true,
cancelable: true,
view: window,
});
emit('click', clickEvent);
}
};Also applies to: 42-53
🤖 Prompt for AI Agents
In unraid-ui/src/components/common/button/Button.vue around lines 42-53 and 66,
the keydown handler currently synthesizes a click on Enter which duplicates the
native click for anchors; update handleKeydown to guard against anchors by
checking if the component is rendered as an anchor (as === 'a' or the underlying
element's tagName === 'A') and if so, do not synthesize/call a click or emit a
click when the key is Enter; return early from the handler for anchor cases,
otherwise keep existing Enter behavior for non-anchor elements.
| // Track if we've already applied state to prevent duplicates | ||
| const appliedStateElements = new WeakSet(); | ||
|
|
There was a problem hiding this comment.
Dedup logic blocks updates after first apply
WeakSet causes all future applies to be skipped even when state changes. Rely on attribute equality (or track last applied state) so updates flow through.
Apply this minimal fix:
- // Track if we've already applied state to prevent duplicates
- const appliedStateElements = new WeakSet();
+ // Track last applied state per element (optional; equality check below is sufficient)
+ // const lastAppliedState = new WeakMap<Element, string>();- elements.forEach((el) => {
- // Skip if we've already applied state to this element
- if (appliedStateElements.has(el)) {
- console.log(`Skipping duplicate state application to ${selector}`);
- return;
- }
-
- // Check if the element already has the same state
- const currentState = el.getAttribute('server');
- if (currentState === stateJson) {
- console.log(`${selector} already has current state, skipping`);
- appliedStateElements.add(el);
- return;
- }
-
- el.setAttribute('server', stateJson);
- appliedStateElements.add(el);
- console.log(`Applied server state to ${selector}`);
- });
+ elements.forEach((el) => {
+ const currentState = el.getAttribute('server');
+ if (currentState === stateJson) {
+ console.log(`${selector} already has current state, skipping`);
+ return;
+ }
+ el.setAttribute('server', stateJson);
+ console.log(`Applied server state to ${selector}`);
+ });Also applies to: 215-236
🤖 Prompt for AI Agents
In web/public/test-pages/test-server-state.js around lines 197-199 (and also
update similar logic at 215-236), the current deduplication uses a WeakSet
(appliedStateElements) which prevents any future updates after the first apply;
replace this with a per-element last-applied-state check: store the last applied
state (stringified or a stable representation) on the element (e.g., via a
Symbol key or a non-enumerable property) or keep a Map<Element, lastState>,
compare the incoming state to the stored lastState and only skip applying when
they are equal, otherwise update the stored lastState and allow the DOM/state
update to proceed; remove the WeakSet usage and adjust both locations
accordingly.
| @@ -1,10 +1,8 @@ | |||
| <script lang="ts" setup> | |||
| import { onBeforeMount, onMounted, ref } from 'vue'; | |||
| import { computed, onBeforeMount, onMounted, ref } from 'vue'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add DOMPurify import for HTML sanitization.
import { computed, onBeforeMount, onMounted, ref } from 'vue';
+import DOMPurify from 'dompurify';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { computed, onBeforeMount, onMounted, ref } from 'vue'; | |
| import { computed, onBeforeMount, onMounted, ref } from 'vue'; | |
| import DOMPurify from 'dompurify'; |
🤖 Prompt for AI Agents
In web/src/components/UserProfile.ce.vue around lines 2 to 2, the file currently
imports Vue helpers but is missing a DOMPurify import for sanitizing HTML
content; add an import for DOMPurify (e.g., import DOMPurify from 'dompurify')
at the top of the file and then use DOMPurify.sanitize(...) wherever the
component injects or renders raw HTML (e.g., before binding to v-html) to ensure
HTML is properly sanitized.
| const callbackData = computed(() => callbackStore.callbackData); | ||
| const name = computed(() => serverStore.name); | ||
| const description = computed(() => serverStore.description); | ||
| const guid = computed(() => serverStore.guid); | ||
| const keyfile = computed(() => serverStore.keyfile); | ||
| const lanIp = computed(() => serverStore.lanIp); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Define a sanitized description computed.
const description = computed(() => serverStore.description);
+const safeDescription = computed(() =>
+ serverStore.description ? DOMPurify.sanitize(serverStore.description) : ''
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const callbackData = computed(() => callbackStore.callbackData); | |
| const name = computed(() => serverStore.name); | |
| const description = computed(() => serverStore.description); | |
| const guid = computed(() => serverStore.guid); | |
| const keyfile = computed(() => serverStore.keyfile); | |
| const lanIp = computed(() => serverStore.lanIp); | |
| const callbackData = computed(() => callbackStore.callbackData); | |
| const name = computed(() => serverStore.name); | |
| const description = computed(() => serverStore.description); | |
| const safeDescription = computed(() => | |
| serverStore.description ? DOMPurify.sanitize(serverStore.description) : '' | |
| ); | |
| const guid = computed(() => serverStore.guid); | |
| const keyfile = computed(() => serverStore.keyfile); | |
| const lanIp = computed(() => serverStore.lanIp); |
🤖 Prompt for AI Agents
In web/src/components/UserProfile.ce.vue around lines 32 to 37, add a new
computed property (e.g., sanitizedDescription) that derives from
serverStore.description and returns a safe, sanitized string; implement it by
using the project's HTML sanitizer utility if available (or import DOMPurify) to
strip/escape any HTML or unsafe characters, then replace template usages of
description with sanitizedDescription so the UI displays the cleaned text.
| <span | ||
| class="hidden text-center text-base md:inline-flex md:items-center md:text-right" | ||
| v-html="description" | ||
| /> |
There was a problem hiding this comment.
Sanitize before using v-html to prevent XSS.
If description can originate from user or remote sources, rendering raw HTML is unsafe.
Apply this diff to use a sanitized value in the template:
- class="hidden text-center text-base md:inline-flex md:items-center md:text-right"
- v-html="description"
+ class="hidden text-center text-base md:inline-flex md:items-center md:text-right"
+ v-html="safeDescription"Then add the supporting import and computed (see next two comments for diffs).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span | |
| class="hidden text-center text-base md:inline-flex md:items-center md:text-right" | |
| v-html="description" | |
| /> | |
| <span | |
| class="hidden text-center text-base md:inline-flex md:items-center md:text-right" | |
| v-html="safeDescription" | |
| /> |
🤖 Prompt for AI Agents
In web/src/components/UserProfile.ce.vue around lines 112-115, the template
currently uses v-html="description" which can introduce XSS; replace that with
v-html="sanitizedDescription" and add a computed property sanitizedDescription
that returns a sanitized string (e.g., DOMPurify.sanitize(description || '')).
Also add the necessary import for the sanitizer (import DOMPurify from
'dompurify') and ensure the computed reads the existing description prop/data so
the template uses the safe value.
| // Expose utility functions on window for debugging/external use | ||
| window.mountVueApp = mountVueApp; | ||
| window.getMountedApp = getMountedApp; | ||
| window.autoMountComponent = autoMountComponent; | ||
|
|
||
| // Expose Apollo client on window for global access | ||
| window.apolloClient = apolloClient; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Augment Window type for exposed globals
Without this, TS will error on custom window props.
Append to this file:
+declare global {
+ interface Window {
+ mountVueApp: typeof mountVueApp;
+ getMountedApp: typeof getMountedApp;
+ autoMountComponent: typeof autoMountComponent;
+ apolloClient: typeof apolloClient;
+ gql: typeof gql;
+ graphqlParse: typeof parse;
+ }
+}
+export {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Expose utility functions on window for debugging/external use | |
| window.mountVueApp = mountVueApp; | |
| window.getMountedApp = getMountedApp; | |
| window.autoMountComponent = autoMountComponent; | |
| // Expose Apollo client on window for global access | |
| window.apolloClient = apolloClient; | |
| // Expose utility functions on window for debugging/external use | |
| window.mountVueApp = mountVueApp; | |
| window.getMountedApp = getMountedApp; | |
| window.autoMountComponent = autoMountComponent; | |
| // Expose Apollo client on window for global access | |
| window.apolloClient = apolloClient; | |
| declare global { | |
| interface Window { | |
| mountVueApp: typeof mountVueApp; | |
| getMountedApp: typeof getMountedApp; | |
| autoMountComponent: typeof autoMountComponent; | |
| apolloClient: typeof apolloClient; | |
| gql: typeof gql; | |
| graphqlParse: typeof parse; | |
| } | |
| } | |
| export {}; |
🤖 Prompt for AI Agents
In web/src/components/Wrapper/auto-mount.ts around lines 35 to 42, TypeScript
will error because custom properties (mountVueApp, getMountedApp,
autoMountComponent, apolloClient) are being attached to window without a Window
type augmentation; append a global declaration merging block to this file that
declares these properties on the Window interface with appropriate types (use
the actual function types for mountVueApp/getMountedApp/autoMountComponent and
the Apollo client type for apolloClient) and add an export {} at the end to
ensure the file is treated as a module.
| // Expose gql tag function for GraphQL queries | ||
| window.gql = parse; | ||
|
|
||
| // Expose parse function for backend evaluation | ||
| window.graphqlParse = parse; |
There was a problem hiding this comment.
Don’t alias graphql.parse as a gql tag
parse isn’t a template tag; window.gql... would throw. Use graphql-tag’s gql (and keep parse as graphqlParse).
Apply:
-import { parse } from 'graphql';
+import { parse } from 'graphql';
+import gql from 'graphql-tag';
@@
- // Expose gql tag function for GraphQL queries
- window.gql = parse;
+ // Expose gql tag for GraphQL queries
+ window.gql = gql;
@@
// Expose parse function for backend evaluation
window.graphqlParse = parse;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Expose gql tag function for GraphQL queries | |
| window.gql = parse; | |
| // Expose parse function for backend evaluation | |
| window.graphqlParse = parse; | |
| // At the top of the file, ensure you import both `parse` and the real `gql` tag: | |
| import { parse } from 'graphql'; | |
| import gql from 'graphql-tag'; | |
| // ... | |
| // Expose gql tag for GraphQL queries | |
| window.gql = gql; | |
| // Expose parse function for backend evaluation | |
| window.graphqlParse = parse; |
🤖 Prompt for AI Agents
In web/src/components/Wrapper/auto-mount.ts around lines 43 to 47, we are
incorrectly aliasing graphql.parse as a template tag (window.gql = parse) which
will throw when used as gql`...`; instead import the proper gql template tag
(e.g. from 'graphql-tag' or from the GraphQL client library you use), assign
window.gql = gql, and keep the parse function exposed as window.graphqlParse =
parse; update imports accordingly so gql is the template tag and parse remains
available for backend evaluation.
| // Type for Vue component module | ||
| type VueComponentModule = { default: object } | object; | ||
|
|
||
| // Type for component mappings | ||
| export type ComponentMapping = { | ||
| selector: string | string[]; // Can be a single selector or array of selector aliases | ||
| appId: string; | ||
| } & ( | ||
| | { component: Component } // Static import | ||
| | { loader: () => Promise<VueComponentModule> } // Dynamic import | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen module/loader typings
Use Vue’s Component instead of object to catch mismatches at build time.
Apply:
-// Type for Vue component module
-type VueComponentModule = { default: object } | object;
+// Type for Vue component module
+type VueComponentModule = { default: Component } | Component;
@@
- | { loader: () => Promise<VueComponentModule> } // Dynamic import
+ | { loader: () => Promise<VueComponentModule> } // Dynamic import📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Type for Vue component module | |
| type VueComponentModule = { default: object } | object; | |
| // Type for component mappings | |
| export type ComponentMapping = { | |
| selector: string | string[]; // Can be a single selector or array of selector aliases | |
| appId: string; | |
| } & ( | |
| | { component: Component } // Static import | |
| | { loader: () => Promise<VueComponentModule> } // Dynamic import | |
| ); | |
| // Type for Vue component module | |
| type VueComponentModule = { default: Component } | Component; | |
| // Type for component mappings | |
| export type ComponentMapping = { | |
| selector: string | string[]; // Can be a single selector or array of selector aliases | |
| appId: string; | |
| } & ( | |
| | { component: Component } // Static import | |
| | { loader: () => Promise<VueComponentModule> } // Dynamic import | |
| ); |
🤖 Prompt for AI Agents
In web/src/components/Wrapper/component-registry.ts around lines 17 to 27, the
VueComponentModule and loader typings use generic object which loses type
safety; import Component from 'vue' and replace object with Component so
VueComponentModule is typed as { default: Component } | Component and update the
loader signature to () => Promise<VueComponentModule>, and ensure the
ComponentMapping union uses { component: Component } for the static branch so
build-time mismatches are caught.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
- Removed redundant source path from main.css to streamline Tailwind class scanning. - Added accessible focus styles for buttons in the .unapi class to improve keyboard navigation. - Updated the sandbox attribute in ChangelogModal.vue to enhance security by limiting permissions. - Added target="_blank" and rel="noopener" attributes to external links for better security and user experience. This commit aims to improve accessibility and security across the application.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
- Added a new component mapping for the UnraidToaster in the component registry to enhance UI functionality. - Corrected the source path in main.css for Tailwind class scanning to ensure proper integration with the unraid-ui components. This commit improves component management and ensures accurate styling integration.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
web/src/components/ApiKey/ApiKeyManager.vue (2)
251-253: Guard against missing “:” in IDAvoid potential undefined from
split(':')[1].- <div class="max-w-[250px] truncate text-sm md:max-w-md"> - <b>ID:</b> {{ key.id.split(':')[1] }} + <div class="max-w-[250px] truncate text-sm md:max-w-md"> + <b>ID:</b> {{ key.id.includes(':') ? key.id.split(':')[1] : key.id }}
258-263: roles may be undefined — add optional chaining and fallback in v-forPrevents runtime errors when roles is absent.
- <div v-if="key.roles.length" class="flex flex-wrap items-center gap-2"> + <div v-if="key.roles?.length" class="flex flex-wrap items-center gap-2"> <span class="text-sm"><b>Roles:</b></span> - <Badge v-for="role in key.roles" :key="role" variant="blue" size="xs">{{ + <Badge v-for="role in (key.roles ?? [])" :key="role" variant="blue" size="xs">{{ role }}</Badge>
🧹 Nitpick comments (11)
web/src/assets/main.css (6)
22-26: Include the root element in box-sizing reset.Add .unapi itself so the container also participates in border-box sizing.
- .unapi *, + .unapi, + .unapi *, .unapi *::before, .unapi *::after { box-sizing: border-box; }
37-41: Use theme token for focus ring color.Replace the hard-coded orange with a CSS var to respect theme overrides.
- .unapi button:focus-visible { - outline: 2px solid #ff8c2f; + .unapi button:focus-visible { + outline: 2px solid var(--ui-focus, #ff8c2f); outline-offset: 2px; }
49-49: Avoid transition: all for perf.Limit to properties that actually animate.
- transition: all 0.2s; + transition: background-color .2s, color .2s, opacity .2s, transform .2s;
61-65: Use theme token for dialog focus rings too.Keep this consistent with the button focus ring.
- .unapi [role="dialog"] button:focus-visible, - .unapi [data-radix-collection-item] button:focus-visible { - outline: 2px solid #ff8c2f; + .unapi [role="dialog"] button:focus-visible, + .unapi [data-radix-collection-item] button:focus-visible { + outline: 2px solid var(--ui-focus, #ff8c2f); outline-offset: 2px; }
115-119: Dark theme border consistency for checked state.Optional: also set border-color for the checked dark state to avoid mixed borders.
.unapi.dark button[role="switch"][data-state="unchecked"], .dark .unapi button[role="switch"][data-state="unchecked"] { background-color: #333; border-color: #555; } + .unapi.dark button[role="switch"][data-state="checked"], + .dark .unapi button[role="switch"][data-state="checked"] { + border-color: #555; + }
135-138: Prefer :focus-visible for link focus styling.Reduces mouse-focus flicker while keeping keyboard focus indication.
-.unapi a:hover, -.unapi a:focus { +.unapi a:hover, +.unapi a:focus-visible { text-decoration: underline; color: inherit; }web/src/components/ApiKey/ApiKeyManager.vue (5)
107-111: Remove duplicate union in parameter type
ApiKeyFragment | ApiKeyFragment | nullis redundant.-function openCreateModal(key: ApiKeyFragment | ApiKeyFragment | null = null) { +function openCreateModal(key: ApiKeyFragment | null = null) {
301-321: Pass a safe default for roles to EffectivePermissionsIf only permissions exist, roles can be undefined; pass [] to avoid defensive checks inside child.
- <EffectivePermissions - :roles="key.roles" + <EffectivePermissions + :roles="key.roles ?? []"
163-169: Whitelist template parameters before redirectPrevents unintended params (e.g., redirect-related) from being forwarded.
- // Navigate to the authorization page with these params using window.location - const authUrl = new URL('/Tools/ApiKeyAuthorize', window.location.origin); - params.forEach((value, key) => { - authUrl.searchParams.append(key, value); - }); + // Navigate to the authorization page with these params using window.location + const authUrl = new URL('/Tools/ApiKeyAuthorize', window.location.origin); + const allowed = new Set(['name', 'scopes', 'description']); + params.forEach((value, key) => { + if (allowed.has(key)) authUrl.searchParams.append(key, value); + }); window.location.href = authUrl.toString();
130-135: Drop unnecessary asyncNo await in
openCreateFromTemplate.-async function openCreateFromTemplate() { +function openCreateFromTemplate() {
83-89: Remove redundant casts on GraphQL-generated enum types
The GraphQL codegen already definesapiKeyPossibleRolesasRole[]and bothapiKeyPossiblePermissions.actionsandApiKeyFragment.permissions[].actionsasAuthAction[]. Delete theas AuthAction[](and anyas Role[]) assertions in ApiKeyManager.vue (lines 83–89 and 193–201).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
web/codegen.ts(1 hunks)web/src/assets/main.css(1 hunks)web/src/components/ApiKey/ApiKeyCreate.vue(1 hunks)web/src/components/ApiKey/ApiKeyManager.vue(15 hunks)web/src/components/ApiKey/apikey.mutation.ts(1 hunks)web/src/components/ApiKey/apikey.query.ts(0 hunks)web/src/components/UpdateOs/ChangelogModal.vue(7 hunks)web/src/components/Wrapper/component-registry.ts(1 hunks)web/src/composables/gql/gql.ts(1 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/ApiKey/apikey.query.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- web/codegen.ts
- web/src/components/Wrapper/component-registry.ts
- web/src/components/ApiKey/ApiKeyCreate.vue
- web/src/components/UpdateOs/ChangelogModal.vue
🧰 Additional context used
📓 Path-based instructions (4)
web/**/*.mutation.ts
📄 CodeRabbit inference engine (.cursor/rules/web-graphql.mdc)
GraphQL mutations must be placed in
.mutation.tsfilesPlace GraphQL mutation documents in .mutation.ts files under web/
Files:
web/src/components/ApiKey/apikey.mutation.ts
web/**/*.{query,mutation}.ts
📄 CodeRabbit inference engine (.cursor/rules/web-graphql.mdc)
All GraphQL under
web/must follow this naming convention
Files:
web/src/components/ApiKey/apikey.mutation.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use the any type; prefer proper typing
Avoid using type assertions/casting; prefer proper typing from the start
Files:
web/src/components/ApiKey/apikey.mutation.tsweb/src/composables/gql/gql.ts
**/components/**/*.vue
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
Some Vue files may require explicit imports like ref or computed due to Nuxt auto-imports not applying in tests
Files:
web/src/components/ApiKey/ApiKeyManager.vue
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.mutation.ts : Place GraphQL mutation documents in .mutation.ts files under web/
Applied to files:
web/src/components/ApiKey/apikey.mutation.tsweb/src/composables/gql/gql.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.mutation.ts : GraphQL mutations must be placed in `.mutation.ts` files
Applied to files:
web/src/components/ApiKey/apikey.mutation.ts
📚 Learning: 2025-03-12T13:48:14.850Z
Learnt from: pujitm
PR: unraid/api#1211
File: web/composables/gql/gql.ts:17-18
Timestamp: 2025-03-12T13:48:14.850Z
Learning: In the Unraid API project, the duplicate GraphQL query and mutation strings in gql.ts files are intentionally generated by GraphQL CodeGen tool and are necessary for the type system to function properly.
Applied to files:
web/src/components/ApiKey/apikey.mutation.tsweb/src/composables/gql/gql.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.query.ts : Place GraphQL query documents in .query.ts files under web/
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.query.ts : GraphQL queries must be placed in `.query.ts` files
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-07-21T14:00:25.140Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-graphql.mdc:0-0
Timestamp: 2025-07-21T14:00:25.140Z
Learning: Applies to web/**/*.{query,mutation}.ts : All GraphQL under `web/` must follow this naming convention
Applied to files:
web/src/composables/gql/gql.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to **/tailwind.config.@(js|ts|mjs|cjs) : Do not add Tailwind config files (using Tailwind v4, config is unnecessary)
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-07-17T14:10:14.613Z
Learnt from: elibosley
PR: unraid/api#1522
File: unraid-ui/src/components/ui/dialog/DialogContent.vue:45-50
Timestamp: 2025-07-17T14:10:14.613Z
Learning: In Tailwind CSS v4, `outline-hidden` is a valid utility class that replaces the v3 behavior of `outline-none`. It applies `outline: 2px solid transparent; outline-offset: 2px;` which hides the browser outline while preserving it in forced colors/high-contrast mode for accessibility. The migration pattern from v3 to v4 is to replace `focus:outline-none` with `focus:outline-hidden` for better accessibility.
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-07-17T14:10:14.613Z
Learnt from: elibosley
PR: unraid/api#1522
File: unraid-ui/src/components/ui/dialog/DialogContent.vue:45-50
Timestamp: 2025-07-17T14:10:14.613Z
Learning: The `outline-hidden` utility class is valid in Tailwind CSS and sets `outline-style: hidden`. This is different from `outline-none` which sets `outline: 2px solid transparent; outline-offset: 2px`. The change from `focus:outline-none` to `focus:outline-hidden` is part of the Tailwind CSS v4 upgrade.
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-04-02T21:21:29.168Z
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Applied to files:
web/src/assets/main.css
📚 Learning: 2025-06-13T17:14:21.739Z
Learnt from: pujitm
PR: unraid/api#1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Applied to files:
web/src/components/ApiKey/ApiKeyManager.vue
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: Applies to **/{store,stores}/**/*.ts : In store files, explicitly import Vue reactivity utilities (e.g., computed, ref, watchEffect); do not rely on Nuxt auto-imports
Applied to files:
web/src/components/ApiKey/ApiKeyManager.vue
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/{store,stores}/**/*.ts : Ensure Vue reactivity utilities (computed, ref, watchEffect) are imported in store source files
Applied to files:
web/src/components/ApiKey/ApiKeyManager.vue
📚 Learning: 2024-11-06T20:59:25.809Z
Learnt from: mdatelle
PR: unraid/api#942
File: api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts:111-113
Timestamp: 2024-11-06T20:59:25.809Z
Learning: In the Unraid API project, error handling for mutations is handled at the service level rather than in the GraphQL resolvers. Specifically, in `api/src/unraid-api/graph/resolvers/auth/auth.resolver.ts`, methods like `removeRoleFromApiKey` rely on service-level error handling.
Applied to files:
web/src/components/ApiKey/ApiKeyManager.vue
📚 Learning: 2025-09-02T19:35:14.427Z
Learnt from: elibosley
PR: unraid/api#1639
File: web/components/standalone-mount.ts:38-54
Timestamp: 2025-09-02T19:35:14.427Z
Learning: In the unraid/api project, exposing Apollo client and GraphQL utilities (apolloClient, gql, graphqlParse) on the global window object is intentional to allow developers to interact with GraphQL from the browser console without needing to be inside Vue components. Developer experience is prioritized over minimizing global exposure in this context.
Applied to files:
web/src/components/ApiKey/ApiKeyManager.vue
🧬 Code graph analysis (1)
web/src/components/ApiKey/apikey.mutation.ts (1)
web/src/composables/gql/gql.ts (1)
graphql(294-296)
🪛 Biome (2.1.2)
web/src/assets/main.css
[error] 100-100: Unexpected shorthand property background after background-color
(lint/suspicious/noShorthandPropertyOverrides)
🔇 Additional comments (4)
web/src/assets/main.css (2)
11-14: @source globs look correct now.Good fix removing the mistargeted path; scanning web/src and the root-level unraid-ui/src is appropriate.
28-35: Good accessibility-preserving reset (no outline nuking).Nice: you reset only margin/padding/border/background and avoid killing focus styles.
web/src/components/ApiKey/apikey.mutation.ts (1)
1-29: LGTM: mutations correctly split into .mutation.ts and typed via graphql registryNaming, placement, and fragment usage align with project rules. No issues spotted.
web/src/composables/gql/gql.ts (1)
20-26: New ApiKey operations are present and correctly mappedCreate/Update/Delete ApiKey entries are wired into overloads and the documents map. Looks good.
Also applies to: 66-72, 140-148
| background-color: transparent; | ||
| background: transparent; | ||
| border: 1px solid #ccc; | ||
| } |
There was a problem hiding this comment.
Fix Biome error: shorthand background overrides background-color.
Remove the redundant shorthand to satisfy lint/suspicious/noShorthandPropertyOverrides.
.unapi button[role="switch"],
.unapi button[role="switch"][data-state="checked"],
.unapi button[role="switch"][data-state="unchecked"] {
background-color: transparent;
- background: transparent;
border: 1px solid #ccc;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| background-color: transparent; | |
| background: transparent; | |
| border: 1px solid #ccc; | |
| } | |
| .unapi button[role="switch"], | |
| .unapi button[role="switch"][data-state="checked"], | |
| .unapi button[role="switch"][data-state="unchecked"] { | |
| background-color: transparent; | |
| border: 1px solid #ccc; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 100-100: Unexpected shorthand property background after background-color
(lint/suspicious/noShorthandPropertyOverrides)
🤖 Prompt for AI Agents
In web/src/assets/main.css around lines 99 to 102 the shorthand background
property is overriding the explicit background-color; remove the redundant
shorthand background: transparent so only background-color: transparent remains
(and keep border: 1px solid #ccc) to satisfy the lint rule and avoid shorthand
overriding.
- Updated the `findManifestFiles` function to only check for `standalone.manifest.json`, removing checks for `manifest.json` and `ui.manifest.json`. - Adjusted the validation logic in `validateSourceDir` to require only `standalone.manifest.json` for new standalone apps, streamlining error messages for missing files. This commit enhances the clarity and efficiency of manifest file validation in the build process.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
plugin/builder/build-txz.ts (5)
31-35: Prefer pushing fullPath for more useful diagnosticsYou currently push only the filename, which loses path context and makes the “Existing Manifest Files” log less helpful.
- files.push(entry.name); + files.push(fullPath);
27-29: Don’t assume Error shape in catch; log safelyAccessing error.message can explode under useUnknownInCatchVariables. Coerce defensively.
- console.warn(`Warning: Could not read directory ${fullPath}: ${error.message}`); + const msg = error instanceof Error ? error.message : String(error); + console.warn(`Warning: Could not read directory ${fullPath}: ${msg}`);
40-45: Guard access to error.code; keep types stricterror may not have a code property. Use a type guard to avoid implicit any and keep TS strict.
- } catch (error) { - if (error.code === 'ENOENT') { + } catch (error) { + const hasCode = (e: unknown): e is { code: string | number } => + typeof e === 'object' && e !== null && 'code' in e; + if (hasCode(error) && error.code === 'ENOENT') { console.warn(`Directory does not exist: ${dir}`); return []; } - throw error; // Re-throw other errors + throw (error instanceof Error ? error : new Error(String(error))); // Re-throw other errors
74-78: Persist what you validate/log: include vendor_url and vendor_filename in configvendorUrl and vendorFilename are required and logged but not written to vendor_archive.json. Persist them or drop from parameters to avoid confusion.
- const configData = { - // vendor_store_url: vendorUrl, - vendor_store_path: vendorFullPath, - api_version: version - }; + const configData = { + vendor_store_url: vendorUrl, + vendor_filename: vendorFilename, + vendor_store_path: vendorFullPath, + api_version: version, + };
68-68: Avoid shelling out for mkdir; use fs.promises.mkdirSaves a subshell, reduces surface area, and aligns with the rest of the file using fs/promises.
- await $`mkdir -p ${configDir}`; + const { mkdir } = await import("node:fs/promises"); + await mkdir(configDir, { recursive: true });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin/builder/build-txz.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use the any type; prefer proper typing
Avoid using type assertions/casting; prefer proper typing from the start
Files:
plugin/builder/build-txz.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
🪛 GitHub Actions: CI - Main (API)
plugin/builder/build-txz.ts
[error] 131-131: Webcomponents missing required file: standalone.manifest.json - run 'pnpm build' in web to generate standalone.manifest.json. Failing command: 'tsx builder/build-txz.ts --tag=PR1655 --base-url=https://preview.dl.unraid.net/unraid-api --api-version=4.19.1 --build-number=24'.
- Changed the download path in `build-plugin.yml` to point to the `standalone` directory for better organization. - Updated the upload path in `main.yml` to use `web/dist` instead of `web/.nuxt/standalone-apps` for consistency. - Refactored the `findManifestFiles` function in `build-txz.ts` to prioritize checking for `standalone.manifest.json` in the `standalone` subdirectory. - Enhanced the manifest validation logic to ensure proper error handling and file existence checks. This commit improves the build process and ensures that the manifest validation is more robust and organized.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
- Introduced a new `DisplaySettingsModification` class to handle modifications to the DisplaySettings.page file. - Implemented functionality to remove the "fixed" class from the locale select element in the Display Settings page. - Added tests for the new modification, including a snapshot of the modified DisplaySettings.page. - Created a patch file to apply the changes to the DisplaySettings.page. This commit enhances the customization capabilities of the Unraid UI by allowing specific modifications to the display settings.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build-plugin.yml (1)
120-131: Fix brittle file existence checks for .plg/.txz artifacts.
[ -f ./deploy/*.plg ]relies on glob expansion and can behave unexpectedly. Usecompgenorlswith globs safely.Apply:
- if [ ! -f ./deploy/*.plg ]; then + if ! compgen -G "./deploy/*.plg" > /dev/null; then echo "Error: .plg file not found in plugin/deploy/" exit 1 fi - if [ ! -f ./deploy/*.txz ]; then + if ! compgen -G "./deploy/*.txz" > /dev/null; then echo "Error: .txz file not found in plugin/deploy/" exit 1 fi
♻️ Duplicate comments (1)
plugin/builder/build-txz.ts (1)
134-141: Gate missing-manifest error in CI; fix message to reflect both locations.Throw locally, warn in CI, and avoid implying it must be under standalone/ only (root is still accepted).
- // Only require standalone.manifest.json for new standalone apps - if (!hasStandaloneManifest) { - console.log("Existing Manifest Files:", manifestFiles); - throw new Error( - `Webcomponents missing required file: standalone.manifest.json - ` + - `run 'pnpm build' in web to generate standalone.manifest.json in the standalone/ subdirectory` - ); - } + // Only require standalone.manifest.json locally; in CI warn and continue + if (!hasStandaloneManifest) { + console.log("Existing Manifest Files:", manifestFiles); + if (validatedEnv.ci) { + console.warn( + "standalone.manifest.json not found; continuing in CI. Ensure the web build publishes vendor assets for baseUrl/tag." + ); + } else { + throw new Error( + "Webcomponents missing required file: standalone.manifest.json - run 'pnpm build' in web to generate it" + ); + } + }
🧹 Nitpick comments (14)
plugin/source/dynamix.unraid.net/install/doinst.sh (1)
28-33: Make symlink creation safer and shellcheck-clean (avoid subshell cd + rm -rf; use atomic ln -sfn).cd inside subshells without guarding (SC2164) and rm -rf on files are unnecessary. Create links directly with ln -sfn using explicit paths; this is atomic and avoids cd failure modes.
-( cd usr/local/bin ; rm -rf corepack ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/corepack/dist/corepack.js corepack ) -( cd usr/local/bin ; rm -rf npm ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npm-cli.js npm ) -( cd usr/local/bin ; rm -rf npx ) -( cd usr/local/bin ; ln -sf ../lib/node_modules/npm/bin/npx-cli.js npx ) +ln -sfn ../lib/node_modules/corepack/dist/corepack.js usr/local/bin/corepack +ln -sfn ../lib/node_modules/npm/bin/npm-cli.js usr/local/bin/npm +ln -sfn ../lib/node_modules/npm/bin/npx-cli.js usr/local/bin/npxplugin/scripts/dc.sh (1)
30-33: Ensure consistent working directory so ../web/dist resolves correctly.If this script is run from plugin/scripts,
../web/distpoints to plugin/web/dist (wrong). Normalize CWD to the plugin root first.Apply:
+SCRIPT_DIR="$(cd -- "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)" +cd "$SCRIPT_DIR/.." || exit 1 # This is to prevent errors when mounting volumes in docker compose WEB_DIST_DIR="../web/dist".github/workflows/main.yml (1)
387-388: Artifact path updated to web/dist — matches the new build output.Change aligns with the Vite build. Consider setting retention and (optionally) compression for large artifacts.
Example:
- - name: Upload build to Github artifacts + - name: Upload build to Github artifacts uses: actions/upload-artifact@v4 with: name: unraid-wc-rich - path: web/dist + path: web/dist + retention-days: 7 + compression-level: 6plugin/docker-compose.yml (1)
15-15: Volume mapping is correct and no leftover Nuxt mountsConfirmed no occurrences of
unraid-components/nuxt. The binding of../web/distto theunraid-components/standalonepath is accurate.Optional: mount read-only to guard against unintended writes:
- - ../web/dist:/app/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/standalone + - ../web/dist:/app/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/unraid-components/standalone:roapi/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts (2)
72-72: Drop unnecessary await before constructor.Constructors aren’t awaitable; this can confuse readers and some linters.
- const patcher = await new testCase.ModificationClass(logger); + const patcher = new testCase.ModificationClass(logger);
117-119: Relax brittle assertion on log count.Per testing guidelines, avoid brittle exact-call-count checks for logs; assert “at least once” instead.
- expect(mockLogger.error.mock.calls.length).toBe(1); + expect(mockLogger.error.mock.calls.length).toBeGreaterThanOrEqual(1);api/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.ts (2)
6-6: Prefer readonly for id.It’s a stable identifier; mark it readonly.
- id: string = 'display-settings'; + readonly id = 'display-settings';
33-33: Remove superfluous await.applyToSource is synchronous.
- const newContent = await this.applyToSource(fileContent); + const newContent = this.applyToSource(fileContent);plugin/builder/utils/manifest-validator.ts (3)
47-56: Make catch block type-safe (avoid unknown.message).Accessing error.message without narrowing can break under useUnknownInCatchVariables. Simplify by extracting a safe message first.
- } catch (error) { - return { - isValid: false, - errors: [`Failed to parse manifest JSON: ${error.message}`], - warnings, - }; - } + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + return { + isValid: false, + errors: [`Failed to parse manifest JSON: ${msg}`], + warnings, + }; + }
72-79: Replace type assertion with a type guard.Avoid the
as ManifestEntrycast and narrow with a predicate to satisfy the “no assertions” guideline.+function isManifestEntry(value: unknown): value is ManifestEntry { + return typeof value === "object" + && value !== null + && "file" in (value as Record<string, unknown>) + && typeof (value as Record<string, unknown>).file === "string"; +} @@ - // Skip if not a manifest entry - if (typeof value !== "object" || !value || !("file" in value)) { + // Skip if not a manifest entry + if (!isManifestEntry(value)) { warnings.push(`Skipping non-entry field: ${key}`); continue; } - - const entry = value as ManifestEntry; + const entry = value;
131-138: Recognize .mjs and .cjs entry files as valid JS.Vite builds can emit .mjs; broaden the check.
- const hasJsEntry = Object.values(manifest).some( - (entry) => typeof entry === "object" && entry?.file?.endsWith(".js") - ); + const hasJsEntry = Object.values(manifest).some( + (entry) => typeof entry === "object" + && (entry as ManifestEntry)?.file + && /\.(m?js|cjs)$/i.test((entry as ManifestEntry).file!) + );plugin/builder/utils/manifest-validator.test.ts (1)
15-20: Use mkdtemp for race-free temp directories.Two tests starting in the same millisecond could collide. mkdtemp avoids that and removes the mkdir call.
-import { mkdir, writeFile, rm } from "fs/promises"; +import { mkdir, writeFile, rm, mkdtemp } from "fs/promises"; @@ - beforeEach(async () => { - // Create a temporary test directory - testDir = join(tmpdir(), `manifest-test-${Date.now()}`); - await mkdir(testDir, { recursive: true }); + beforeEach(async () => { + // Create a unique temporary test directory + testDir = await mkdtemp(join(tmpdir(), "manifest-test-")); manifestPath = join(testDir, "standalone.manifest.json"); });plugin/builder/build-txz.ts (2)
42-47: Narrow ENOENT check safely.error is unknown; guard before accessing .code to avoid type errors in strict TS configs.
- } catch (error) { - if (error.code === 'ENOENT') { + } catch (error) { + const code = (error && typeof error === 'object' && 'code' in (error as any)) + ? (error as NodeJS.ErrnoException).code + : undefined; + if (code === 'ENOENT') { console.warn(`Directory does not exist: ${dir}`); return []; } throw error; // Re-throw other errors }
52-103: Persist all validated inputs or stop validating them.storeVendorArchiveInfo requires vendorUrl and vendorFilename but doesn’t persist them. Either persist both or drop them from the required inputs.
- if (!version || !vendorUrl || !vendorFilename) { + if (!version || !vendorUrl || !vendorFilename) { throw new Error("Cannot store vendor archive info: Missing required parameters"); } @@ - const configData = { - // vendor_store_url: vendorUrl, - vendor_store_path: vendorFullPath, - api_version: version - }; + const configData = { + vendor_store_url: vendorUrl, + vendor_store_path: vendorFullPath, + vendor_filename: vendorFilename, + api_version: version + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
api/src/unraid-api/cli/generated/graphql.tsis excluded by!**/generated/**
📒 Files selected for processing (13)
.github/workflows/build-plugin.yml(1 hunks).github/workflows/main.yml(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DisplaySettings.page(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts(2 hunks)api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DisplaySettings.page.modified.snapshot.php(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.ts(1 hunks)api/src/unraid-api/unraid-file-modifier/modifications/patches/display-settings.patch(1 hunks)plugin/builder/build-txz.ts(2 hunks)plugin/builder/utils/manifest-validator.test.ts(1 hunks)plugin/builder/utils/manifest-validator.ts(1 hunks)plugin/docker-compose.yml(1 hunks)plugin/scripts/dc.sh(1 hunks)plugin/source/dynamix.unraid.net/install/doinst.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
api/src/unraid-api/**
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
Prefer adding new files to the Nest repo at api/src/unraid-api/ instead of legacy code
Prefer adding new API files under api/src/unraid-api/ instead of legacy locations
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.tsapi/src/unraid-api/unraid-file-modifier/modifications/patches/display-settings.patchapi/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.tsapi/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DisplaySettings.pageapi/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DisplaySettings.page.modified.snapshot.php
api/**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/api-rules.mdc)
api/**/*.{test,spec}.{js,jsx,ts,tsx}: Use Vitest for tests in the api; do not use Jest
Prefer not to mock simple dependencies in tests
For error testing, use.rejects.toThrow()without arguments; avoid asserting exact error messages unless the message format is the subject under test
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts
api/**/*.{spec,test}.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer not to mock simple dependencies in API tests
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts
{api,web}/**/*.{spec,test}.@(ts|tsx|js|jsx)
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{spec,test}.@(ts|tsx|js|jsx): When testing for errors, use .rejects.toThrow() without asserting specific message strings
Focus tests on behavior, not implementation details like exact error messages
Avoid brittle tests that depend on non-essential details (e.g., log formats)
Use mocks as nouns (objects), not verbs (functions)
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Never use the any type; prefer proper typing
Avoid using type assertions/casting; prefer proper typing from the start
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.tsplugin/builder/utils/manifest-validator.test.tsplugin/builder/utils/manifest-validator.tsapi/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.tsplugin/builder/build-txz.ts
api/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
In API TypeScript, use .js extensions in import specifiers for ESM compatibility
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.tsapi/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.ts
api/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
For cache-manager v7, always use TTL values in milliseconds when setting cache TTL
Files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.tsapi/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.ts
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}
📄 CodeRabbit inference engine (.cursor/rules/web-testing-rules.mdc)
{**/*.test.ts,**/__test__/{components,store}/**/*.ts}: Use .rejects.toThrow() without arguments when asserting that async functions throw; avoid checking exact error message strings unless the message format is explicitly under test
Focus tests on observable behavior and outcomes, not implementation details such as exact error messages
Use await nextTick() for DOM update assertions and flushPromises() for complex async chains; always await async operations before asserting
Place module mock declarations (vi.mock) at the top level of the test file to avoid hoisting issues
Use factory functions in vi.mock calls to define mocks and avoid hoisting pitfalls
Use vi.spyOn() to specify return values or behavior of methods under test
Reset/clear mocks between tests using vi.clearAllMocks() (and vi.resetAllMocks() when appropriate) to ensure isolation
Do not rely on Nuxt auto-imports in tests; import required Vue utilities explicitly in test files
Remember that vi.mock calls are hoisted; avoid mixing mock declarations and module mocks incorrectly
Files:
plugin/builder/utils/manifest-validator.test.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to web/**/*.{spec,test}.@(ts|tsx|js|jsx) : Use mount from Vue Test Utils for component tests
📚 Learning: 2025-01-29T16:35:43.699Z
Learnt from: elibosley
PR: unraid/api#1082
File: api/src/unraid-api/unraid-file-modifier/modifications/log-rotate.modification.ts:39-41
Timestamp: 2025-01-29T16:35:43.699Z
Learning: The UnraidFileModifierService in the Unraid API provides comprehensive error handling for all FileModification implementations, including detailed error logging with stack traces and modification IDs. Individual FileModification implementations should focus on their core functionality without duplicating error handling.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts
📚 Learning: 2025-08-11T15:10:28.150Z
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-08-11T15:10:28.150Z
Learning: This Nuxt app is tested with vitest outside the Nuxt runtime; structure tests accordingly
Applied to files:
plugin/builder/utils/manifest-validator.test.ts
📚 Learning: 2025-09-02T14:41:56.235Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T14:41:56.235Z
Learning: Applies to api/**/vitest.config.@(js|ts|mjs|cjs) : API test runner is Vitest
Applied to files:
plugin/builder/utils/manifest-validator.test.ts
📚 Learning: 2025-02-03T17:21:26.738Z
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DefaultPageLayout.php.modified.snapshot.php:203-207
Timestamp: 2025-02-03T17:21:26.738Z
Learning: The project uses patches to override existing Unraid pages rather than modifying them directly.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/display-settings.patch
📚 Learning: 2025-01-31T22:01:41.842Z
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/display-settings.patch
📚 Learning: 2025-02-03T18:57:53.577Z
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts:80-80
Timestamp: 2025-02-03T18:57:53.577Z
Learning: In the unraid/api codebase, patch files generated for file modifications always use the .patch extension, regardless of the source file type being modified.
Applied to files:
api/src/unraid-api/unraid-file-modifier/modifications/patches/display-settings.patch
📚 Learning: 2025-02-06T17:25:45.397Z
Learnt from: mdatelle
PR: unraid/api#1122
File: web/package.json:16-16
Timestamp: 2025-02-06T17:25:45.397Z
Learning: The build script in web/package.json should include type-check command (`npm run type-check`) even when NODE_ENV is production, as it provides value for local builds by catching type errors before pushing to CI/CD.
Applied to files:
plugin/builder/build-txz.ts
🧬 Code graph analysis (6)
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts (1)
api/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.ts (1)
DisplaySettingsModification(5-37)
plugin/source/dynamix.unraid.net/install/doinst.sh (1)
api/src/unraid-api/app/dependency.service.ts (1)
npm(19-24)
plugin/builder/utils/manifest-validator.test.ts (2)
web/scripts/add-timestamp-standalone-manifest.js (1)
manifestPath(6-6)plugin/builder/utils/manifest-validator.ts (3)
validateStandaloneManifest(32-146)StandaloneManifest(16-18)getStandaloneManifestPath(153-173)
plugin/builder/utils/manifest-validator.ts (1)
web/scripts/add-timestamp-standalone-manifest.js (2)
key(20-20)manifestPath(6-6)
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DisplaySettings.page.modified.snapshot.php (1)
plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/include/UnraidCheck.php (1)
_(159-159)
plugin/builder/build-txz.ts (1)
plugin/builder/utils/manifest-validator.ts (2)
getStandaloneManifestPath(153-173)validateStandaloneManifest(32-146)
🪛 Shellcheck (0.10.0)
plugin/source/dynamix.unraid.net/install/doinst.sh
[warning] 28-28: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 29-29: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 30-30: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 31-31: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 32-32: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
[warning] 33-33: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
🔇 Additional comments (9)
plugin/source/dynamix.unraid.net/install/doinst.sh (2)
29-33: Verify targets exist and are executable to avoid dangling npm/npx/corepack.If these module files aren’t present or executable at install time, npm/npx/corepack will be broken on PATH.
Optionally add guards:
+for t in \ + usr/local/lib/node_modules/corepack/dist/corepack.js \ + usr/local/lib/node_modules/npm/bin/npm-cli.js \ + usr/local/lib/node_modules/npm/bin/npx-cli.js; do + [ -x "$t" ] || echo "warning: missing or non-exec $t" +done
31-33: Symlinks align with runtime usage (execa('npm') expects npm on PATH).This ensures dependency.service.ts can resolve npm via PATH. Looks good.
.github/workflows/build-plugin.yml (1)
98-103: Approve code changes—no remaining references tounraid-components/nuxtornuxt-custom-elements. Path switch to standalone aligns with the Vite/dist migration; artifact location matches docker-compose and main.yml.api/src/unraid-api/unraid-file-modifier/modifications/patches/display-settings.patch (1)
11-13: Patch diff is minimal and targeted.Removing only class="fixed" from the locale select looks correct and aligns with the intent.
api/src/unraid-api/unraid-file-modifier/modifications/__test__/generic-modification.spec.ts (1)
11-11: Good addition to the matrix of patch tests.Coverage for DisplaySettingsModification integrates cleanly with existing generic tests.
Also applies to: 39-44
api/src/unraid-api/unraid-file-modifier/modifications/__test__/__fixtures__/downloaded/DisplaySettings.page (1)
138-147: Fixture accurately exercises the target pattern.Contains the expected class="fixed" on the locale select for the patch to modify.
api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DisplaySettings.page.modified.snapshot.php (1)
138-147: Snapshot matches intended outcome.Locale select no longer has class="fixed"; rest of markup preserved.
plugin/builder/utils/manifest-validator.ts (1)
153-173: LGTM on path resolution order.Preferred standalone/, then root, then nuxt/ for compat reads well and matches the migration plan.
plugin/builder/utils/manifest-validator.test.ts (1)
27-233: Solid coverage and alignment with test rules.Behavior-focused assertions, no reliance on Nuxt auto-imports, and warnings vs. errors are exercised well.
| if (line.includes('<select name="locale"') && line.includes('class="fixed"')) { | ||
| // Remove class="fixed" from the line, handling potential spacing variations | ||
| return line.replace(/\s*class="fixed"\s*/, ' ').replace(/\s+/g, ' '); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make class removal robust (quotes/spacing/multi-class) and avoid global whitespace collapse.
Current logic only matches class="fixed" with exact spacing and compresses all whitespace on the line. Harden matching and remove only the fixed token.
- if (line.includes('<select name="locale"') && line.includes('class="fixed"')) {
- // Remove class="fixed" from the line, handling potential spacing variations
- return line.replace(/\s*class="fixed"\s*/, ' ').replace(/\s+/g, ' ');
- }
+ // Only operate on the locale <select>; tolerate spacing/quotes and multi-class values
+ if (!/<select\b[^>]*\bname\s*=\s*(['"])locale\1/i.test(line)) return line;
+ if (!/\bclass\s*=\s*(['"])(?:[^'"]*\s)?fixed(?:\s[^'"]*)?\1/i.test(line)) return line;
+ return line
+ .replace(/\bclass\s*=\s*(['"])([^'"]*)\1/i, (_m, q: string, classes: string) => {
+ const filtered = classes.split(/\s+/).filter((c) => c && c !== 'fixed').join(' ');
+ return filtered ? ` class=${q}${filtered}${q}` : '';
+ })
+ .replace(/\s{2,}/g, ' ')
+ .replace(/\s+(>)/, '$1');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (line.includes('<select name="locale"') && line.includes('class="fixed"')) { | |
| // Remove class="fixed" from the line, handling potential spacing variations | |
| return line.replace(/\s*class="fixed"\s*/, ' ').replace(/\s+/g, ' '); | |
| } | |
| // Only operate on the locale <select>; tolerate spacing/quotes and multi-class values | |
| if (!/<select\b[^>]*\bname\s*=\s*(['"])locale\1/i.test(line)) return line; | |
| if (!/\bclass\s*=\s*(['"])(?:[^'"]*\s)?fixed(?:\s[^'"]*)?\1/i.test(line)) return line; | |
| return line | |
| .replace(/\bclass\s*=\s*(['"])([^'"]*)\1/i, (_m, q: string, classes: string) => { | |
| const filtered = classes.split(/\s+/).filter((c) => c && c !== 'fixed').join(' '); | |
| return filtered ? ` class=${q}${filtered}${q}` : ''; | |
| }) | |
| .replace(/\s{2,}/g, ' ') | |
| .replace(/\s+(>)/, '$1'); |
🤖 Prompt for AI Agents
In
api/src/unraid-api/unraid-file-modifier/modifications/display-settings.modification.ts
around lines 15 to 18, the current replacement blindly targets class="fixed" and
then collapses all whitespace on the line; instead, find the class attribute
(supporting single or double quotes), remove only the token "fixed" from the
space-separated class list (handle leading/trailing/multiple spaces), collapse
spaces only inside the class value, and if the class list becomes empty remove
the entire class attribute; do not perform a global whitespace collapse on the
whole line so other spacing remains intact.
| // Check for manifest files in expected locations | ||
| const findManifestFiles = async (dir: string): Promise<string[]> => { | ||
| const files: string[] = []; | ||
|
|
||
| // Check standalone subdirectory (preferred) | ||
| try { | ||
| const standaloneDir = join(dir, "standalone"); | ||
| const entries = await readdir(standaloneDir, { withFileTypes: true }); | ||
| for (const entry of entries) { | ||
| if (entry.isFile() && entry.name === "standalone.manifest.json") { | ||
| files.push("standalone/standalone.manifest.json"); | ||
| } | ||
| } | ||
| } catch (error) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Include nuxt/standalone.manifest.json in discovery or defer to getStandaloneManifestPath.
findManifestFiles ignores the nuxt/ fallback that getStandaloneManifestPath supports, causing false negatives and unnecessary failures.
Option A (add nuxt scan):
// Check root directory for backwards compatibility
try {
const entries = await readdir(dir, { withFileTypes: true });
for (const entry of entries) {
if (entry.isFile() && entry.name === "standalone.manifest.json") {
files.push("standalone.manifest.json");
}
}
} catch (error) {
@@
}
+
+ // Check nuxt subdirectory for backwards compatibility
+ try {
+ const nuxtDir = join(dir, "nuxt");
+ const entries = await readdir(nuxtDir, { withFileTypes: true });
+ for (const entry of entries) {
+ if (entry.isFile() && entry.name === "standalone.manifest.json") {
+ files.push("nuxt/standalone.manifest.json");
+ }
+ }
+ } catch {
+ // ignore
+ }Option B (preferred): drop duplication and just use getStandaloneManifestPath() to decide presence.
Also applies to: 33-41
🤖 Prompt for AI Agents
In plugin/builder/build-txz.ts around lines 16-29 (and similarly 33-41), the
manifest discovery duplicates logic and misses the nuxt/ fallback; replace the
custom standalone/ directory scan with a call to the existing
getStandaloneManifestPath(dir) helper, await its result, and if it returns a
path add that manifest path to the files array (instead of hardcoding
"standalone/standalone.manifest.json"); remove the try/catch block that manually
reads standalone/ and ensure the same replacement is applied to the later block
at lines 33-41 so the nuxt/ fallback is respected.
| const manifestFiles = await findManifestFiles(webcomponentDir); | ||
| const hasManifest = manifestFiles.includes("manifest.json"); | ||
| const hasStandaloneManifest = manifestFiles.includes("standalone.manifest.json"); | ||
| const hasUiManifest = manifestFiles.includes("ui.manifest.json"); | ||
| const hasStandaloneManifest = manifestFiles.some(file => | ||
| file === "standalone.manifest.json" || file === "standalone/standalone.manifest.json" | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use getStandaloneManifestPath once and reuse; fixes nuxt-fallback mismatch.
Compute manifestPath first and derive hasStandaloneManifest from it; then reuse the same manifestPath for validation.
- const manifestFiles = await findManifestFiles(webcomponentDir);
- const hasStandaloneManifest = manifestFiles.some(file =>
- file === "standalone.manifest.json" || file === "standalone/standalone.manifest.json"
- );
+ const manifestFiles = await findManifestFiles(webcomponentDir);
+ const manifestPath = getStandaloneManifestPath(webcomponentDir);
+ const hasStandaloneManifest = Boolean(manifestPath);
@@
- // Validate the manifest contents
- const manifestPath = getStandaloneManifestPath(webcomponentDir);
- if (manifestPath) {
+ // Validate the manifest contents
+ if (manifestPath) {
const validation = await validateStandaloneManifest(manifestPath);
@@
- console.log("✅ Standalone manifest validation passed");
- }
+ console.log("✅ Standalone manifest validation passed");
+ } else {
+ // Nothing to validate
+ }Also applies to: 143-164
🤖 Prompt for AI Agents
In plugin/builder/build-txz.ts around lines 129-133 (and similarly lines
143-164), we currently check for a standalone manifest by scanning manifestFiles
directly which can mismatch the path used later; instead call
getStandaloneManifestPath(webcomponentDir) once, store its result in a
manifestPath variable, set hasStandaloneManifest based on whether manifestPath
is truthy, and then reuse manifestPath for subsequent validation and file
operations (replace the manifestFiles.some(...) checks with the single
manifestPath usage in both the 129-133 block and the 143-164 block).
| // Check main file | ||
| if (entry.file) { | ||
| const filePath = join(manifestDir, entry.file); | ||
| if (!checkedFiles.has(filePath)) { | ||
| checkedFiles.add(filePath); | ||
| if (!existsSync(filePath)) { | ||
| errors.push(`Missing file referenced in manifest: ${entry.file}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check CSS files | ||
| if (entry.css && Array.isArray(entry.css)) { | ||
| for (const cssFile of entry.css) { | ||
| const cssPath = join(manifestDir, cssFile); | ||
| if (!checkedFiles.has(cssPath)) { | ||
| checkedFiles.add(cssPath); | ||
| if (!existsSync(cssPath)) { | ||
| errors.push(`Missing CSS file referenced in manifest: ${cssFile}`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard against absolute paths and path traversal when resolving manifest files.
Joining with an absolute path ignores the base dir; a malicious manifest could probe arbitrary files. Resolve within the manifest directory and reject absolute/escaping paths.
-import { join, dirname } from "path";
+import { join, dirname, resolve, isAbsolute, sep } from "path";
@@
- const manifestDir = dirname(manifestPath);
+ const manifestDir = dirname(manifestPath);
+ const manifestRoot = resolve(manifestDir) + sep;
+
+ const resolveWithinManifest = (relPath: string): string | null => {
+ if (isAbsolute(relPath)) {
+ errors.push(`Absolute paths are not allowed in manifest: ${relPath}`);
+ return null;
+ }
+ const abs = resolve(manifestDir, relPath);
+ if (!abs.startsWith(manifestRoot)) {
+ errors.push(`Path escapes manifest directory: ${relPath}`);
+ return null;
+ }
+ return abs;
+ };
@@
- if (entry.file) {
- const filePath = join(manifestDir, entry.file);
+ if (entry.file) {
+ const filePath = resolveWithinManifest(entry.file);
- if (!checkedFiles.has(filePath)) {
- checkedFiles.add(filePath);
- if (!existsSync(filePath)) {
- errors.push(`Missing file referenced in manifest: ${entry.file}`);
- }
- }
+ if (filePath && !checkedFiles.has(filePath)) {
+ checkedFiles.add(filePath);
+ if (!existsSync(filePath)) {
+ errors.push(`Missing file referenced in manifest: ${entry.file}`);
+ }
+ }
}
@@
- for (const cssFile of entry.css) {
- const cssPath = join(manifestDir, cssFile);
- if (!checkedFiles.has(cssPath)) {
- checkedFiles.add(cssPath);
- if (!existsSync(cssPath)) {
- errors.push(`Missing CSS file referenced in manifest: ${cssFile}`);
- }
- }
- }
+ for (const cssFile of entry.css) {
+ const cssPath = resolveWithinManifest(cssFile);
+ if (cssPath && !checkedFiles.has(cssPath)) {
+ checkedFiles.add(cssPath);
+ if (!existsSync(cssPath)) {
+ errors.push(`Missing CSS file referenced in manifest: ${cssFile}`);
+ }
+ }
+ }
@@
- for (const assetFile of entry.assets) {
- const assetPath = join(manifestDir, assetFile);
- if (!checkedFiles.has(assetPath)) {
- checkedFiles.add(assetPath);
- if (!existsSync(assetPath)) {
- errors.push(`Missing asset file referenced in manifest: ${assetFile}`);
- }
- }
- }
+ for (const assetFile of entry.assets) {
+ const assetPath = resolveWithinManifest(assetFile);
+ if (assetPath && !checkedFiles.has(assetPath)) {
+ checkedFiles.add(assetPath);
+ if (!existsSync(assetPath)) {
+ errors.push(`Missing asset file referenced in manifest: ${assetFile}`);
+ }
+ }
+ }
@@
- for (const importFile of entry.imports) {
- const importPath = join(manifestDir, importFile);
- if (!checkedFiles.has(importPath)) {
- checkedFiles.add(importPath);
- if (!existsSync(importPath)) {
- warnings.push(`Missing import file referenced in manifest: ${importFile} (this may be okay if it's a virtual import)`);
- }
- }
- }
+ for (const importFile of entry.imports) {
+ const importPath = resolveWithinManifest(importFile);
+ if (importPath && !checkedFiles.has(importPath)) {
+ checkedFiles.add(importPath);
+ if (!existsSync(importPath)) {
+ warnings.push(`Missing import file referenced in manifest: ${importFile} (this may be okay if it's a virtual import)`);
+ }
+ }
+ }Also applies to: 104-115, 117-128
Summary by CodeRabbit