feat: improve dom content loading by being more efficient about component mounting #1716
Conversation
|
Claude finished @elibosley's task —— View job Code Review - Final AssessmentReviewing only the diff/changeset for high-priority issues:
Issues Found:
Security Improvements:
✅ No critical runtime issues found in changes - the breaking change is safely handled with fallbacks. |
|
Caution Review failedThe pull request is closed. WalkthroughRemoves global debug exposures and related tests, updates window typings, refactors the Vue mount engine to batch-mount components with improved prop parsing and locale retrieval from Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant DOM as DOM
participant Auto as autoMountAllComponents()
participant Engine as mountUnifiedApp()
participant I18n as setupI18n
participant App as Vue App (shared ctx)
participant C as Component Instances
DOM->>Auto: page ready / init
Auto->>Engine: delegate and return App
Engine->>DOM: query all selectors (batch)
Engine->>Engine: parse props for each element\n(skip common attrs, decode entities, JSON-if starts with { or [)
Engine->>I18n: read window.LOCALE_DATA
I18n-->>Engine: i18n instance
Engine->>App: create shared app/context
loop for each target element
Engine->>DOM: replaceChildren(target)
App->>C: render wrapped component into target
Engine->>DOM: set data-vue-mounted, add .unapi
Engine->>Engine: store unmount handler
end
Engine-->>Auto: return App instance
Note over Engine,Auto: No runtime globals (no window.__unifiedApp / __mountedComponents)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1716 +/- ##
==========================================
+ Coverage 52.56% 52.58% +0.01%
==========================================
Files 858 858
Lines 48074 48091 +17
Branches 4784 4792 +8
==========================================
+ Hits 25270 25287 +17
Misses 22735 22735
Partials 69 69 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Removed global exposure of `__unifiedApp` and `__mountedComponents` from the `mountUnifiedApp` function to enhance encapsulation. - Improved efficiency in the `parsePropsFromElement` function by adding early exit for elements without attributes and optimizing attribute skipping. - Updated component mounting logic to utilize a map for quick selector lookups, reducing DOM queries and improving performance.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
web/src/components/Wrapper/mount-engine.ts (1)
8-17: ESM: add .js extensions to local importsPer repo guidelines for TS ESM, local/aliased imports should end with .js.
-import { componentMappings } from '@/components/Wrapper/component-registry'; -import { client } from '~/helpers/create-apollo-client'; -import { createHtmlEntityDecoder } from '~/helpers/i18n-utils'; -import en_US from '~/locales/en_US.json'; +import { componentMappings } from '@/components/Wrapper/component-registry.js'; +import { client } from '~/helpers/create-apollo-client.js'; +import { createHtmlEntityDecoder } from '~/helpers/i18n-utils.js'; +import en_US from '~/locales/en_US.json'; -import { globalPinia } from '~/store/globalPinia'; +import { globalPinia } from '~/store/globalPinia.js';web/__test__/components/Wrapper/mount-engine.test.ts (6)
5-6: ESM: add .js extension to local type importAlign test imports with ESM rule.
-import type { ComponentMapping } from '~/components/Wrapper/component-registry'; +import type { ComponentMapping } from '~/components/Wrapper/component-registry.js';
26-28: ESM: add .js extension to vi.mock path-vi.mock('~/components/Wrapper/component-registry', () => ({ +vi.mock('~/components/Wrapper/component-registry.js', () => ({Repeat for other local mocks below.
37-38: ESM: fix remaining mock import specifiersUpdate remaining mocks to use .js extensions per guideline.
-vi.mock('vue-i18n', () => ({ +vi.mock('vue-i18n', () => ({ createI18n: vi.fn(() => mockI18n), })); -vi.mock('~/helpers/create-apollo-client', () => ({ +vi.mock('~/helpers/create-apollo-client.js', () => ({ client: mockApolloClient, })); -vi.mock('~/store/globalPinia', () => ({ +vi.mock('~/store/globalPinia.js', () => ({ globalPinia: mockGlobalPinia, })); -vi.mock('~/helpers/i18n-utils', () => ({ +vi.mock('~/helpers/i18n-utils.js', () => ({ createHtmlEntityDecoder: vi.fn(() => (str: string) => str), }));Also applies to: 41-43, 53-55, 61-63
72-81: Dynamic import path should also end with .js- const module = await import('~/components/Wrapper/mount-engine'); + const module = await import('~/components/Wrapper/mount-engine.js');
161-177: This test doesn’t validate JSON prop parsingAsserting the attribute string remains unchanged doesn’t prove props parsing. Make the component render message.text when an object is provided.
- it('should handle JSON props from attributes', () => { + it('should handle JSON props from attributes', () => { const element = document.createElement('div'); element.id = 'test-app'; element.setAttribute('message', '{"text": "JSON Message"}'); document.body.appendChild(element); - mockComponentMappings.push({ + const JsonComponent = defineComponent({ + name: 'JsonComponent', + props: { message: { type: [Object, String], default: '' } }, + setup(props) { + // if object, render its text; otherwise render as string + return () => + h('div', { class: 'test-component' }, typeof props.message === 'object' ? (props.message as any).text : String(props.message)); + }, + }); + + mockComponentMappings.push({ selector: '#test-app', appId: 'test-app', - component: TestComponent, + component: JsonComponent, }); mountUnifiedApp(); - // The component receives the parsed JSON object - expect(element.getAttribute('message')).toBe('{"text": "JSON Message"}'); + expect(element.textContent).toContain('JSON Message'); });
179-194: Same issue: HTML-encoded JSON test doesn’t verify behaviorAssert rendered output after decoding + parsing.
- it('should handle HTML-encoded JSON in attributes', () => { + it('should handle HTML-encoded JSON in attributes', () => { const element = document.createElement('div'); element.id = 'test-app'; element.setAttribute('message', '{"text": "Encoded"}'); document.body.appendChild(element); - mockComponentMappings.push({ + const JsonComponent = defineComponent({ + name: 'JsonComponent', + props: { message: { type: [Object, String], default: '' } }, + setup(props) { + return () => + h('div', { class: 'test-component' }, typeof props.message === 'object' ? (props.message as any).text : String(props.message)); + }, + }); + + mockComponentMappings.push({ selector: '#test-app', appId: 'test-app', - component: TestComponent, + component: JsonComponent, }); mountUnifiedApp(); - expect(element.getAttribute('message')).toBe('{"text": "Encoded"}'); + expect(element.textContent).toContain('Encoded'); });
🧹 Nitpick comments (10)
web/src/components/Wrapper/mount-engine.ts (7)
19-27: Type-safety for window globals; remove stale __unifiedAppAccessing window.apolloClient without typing can break TS checks, and __unifiedApp is no longer used.
-import type { App as VueApp } from 'vue'; +import type { App as VueApp } from 'vue'; +import type { ApolloClient } from '@apollo/client/core'; declare global { interface Window { globalPinia: typeof globalPinia; - __unifiedApp?: VueApp; + apolloClient?: ApolloClient<unknown>; + LOCALE_DATA?: string; } } -const apolloClient = (typeof window !== 'undefined' && window.apolloClient) || client; +const apolloClient = (typeof window !== 'undefined' && window.apolloClient) || client;Also remove the __unifiedApp property from this declaration since it’s no longer exposed.
41-49: Avoid double-casting window for LOCALE_DATANow that Window.LOCALE_DATA is declared, you can drop the as unknown as cast.
- const windowLocaleData = (window as unknown as { LOCALE_DATA?: string }).LOCALE_DATA || null; + const windowLocaleData = window.LOCALE_DATA || null;
82-99: JSON detection should trim leading whitespacevalue[0] fails when the JSON starts after whitespace. Trim before testing.
- const value = attr.value; + const value = attr.value; + const first = value.trimStart()[0]; @@ - if (value[0] === '{' || value[0] === '[') { + if (first === '{' || first === '[') {Optional: reuse createHtmlEntityDecoder() here for consistency with i18n decoding.
199-202: Reliance on internal vnode.appContext APIDirectly assigning vnode.appContext uses a private field; stable in Vue 3.x but not guaranteed.
Please confirm our Vue minor range guarantees this. Alternative is createApp(wrappedComponent) per target and copy root app._context fields, but that reintroduces multiple app instances.
203-206: Use replaceChildren() instead of innerHTML=''Same effect, avoids innerHTML lint noise and is marginally clearer.
- element.innerHTML = ''; - render(vnode, element); + element.replaceChildren(); + render(vnode, element);Note: clearing to an empty string is not an XSS risk here, but this quiets static analyzers.
211-216: Unmount bookkeeping exists but isn’t exposedYou collect unmount handlers, but there’s no way to invoke them.
Consider exporting a cleanupUnifiedMounts() from this module or returning a handle with unmountAll for consumers that need teardown.
223-226: Return the app from autoMountAllComponents for parityReturning the app makes this API symmetrical with mountUnifiedApp and helps tests/integration.
-export function autoMountAllComponents() { - mountUnifiedApp(); -} +export function autoMountAllComponents() { + return mountUnifiedApp(); +}web/__test__/components/Wrapper/mount-engine.test.ts (3)
104-109: Remove dead teardown commentComment-only teardown block can be dropped to reduce noise.
408-419: Strengthen i18n window data test by asserting createI18n argsEnsure the parsed locale is used.
- it('should parse window locale data', () => { + it('should parse window locale data', () => { + const mockCreateI18n = vi.fn(() => mockI18n); + vi.doMock('vue-i18n', () => ({ createI18n: mockCreateI18n })); + vi.resetModules(); + const { mountUnifiedApp: freshMount } = require('~/components/Wrapper/mount-engine.js'); const localeData = { fr_FR: { test: 'Message de test' }, }; (window as unknown as Record<string, unknown>).LOCALE_DATA = encodeURIComponent( JSON.stringify(localeData) ); - mountUnifiedApp(); + freshMount(); + expect(mockCreateI18n).toHaveBeenCalledWith(expect.objectContaining({ locale: 'fr_FR' })); delete (window as unknown as Record<string, unknown>).LOCALE_DATA; });Note: using vi.doMock here redefines the factory after initial mocks; adjust if your test runner disallows late mocks.
442-459: Consider asserting debug log or rename testThe code logs console.debug with the mount count. Current test checks console.warn, which will always pass.
Option A: spy on console.debug and assert it contains “[UnifiedMount] Mounted”.
Option B: rename to clarify it’s not checking timing logs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/__test__/components/Wrapper/mount-engine.test.ts(1 hunks)web/src/components/Wrapper/auto-mount.ts(0 hunks)web/src/components/Wrapper/mount-engine.ts(2 hunks)
💤 Files with no reviewable changes (1)
- web/src/components/Wrapper/auto-mount.ts
🧰 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
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
web/__test__/components/Wrapper/mount-engine.test.tsweb/src/components/Wrapper/mount-engine.ts
{api,web}/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{api,web}/**/*.{test,spec}.{ts,tsx}: For error tests, use .rejects.toThrow() without arguments; avoid asserting exact error messages unless that format is the subject
Focus tests on behavior, not implementation details
Avoid brittle tests tied to exact error or log wording
Use mocks as nouns, not verbs
Always await async operations before making assertions
Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between tests
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
web/__test__/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Vue component tests under web/test; run with pnpm test
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
web/__test__/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
web/__test__/**/*.{test,spec}.{ts,tsx}: Use mount from Vue Test Utils for component testing
Stub complex child components that aren’t the focus of the test
Mock external dependencies and services in component tests
Test component behavior and output, not implementation details
Use createTestingPinia() for mocking stores in components
Prefer semantic queries like find('button') over data-test IDs
Use await nextTick() for DOM updates before assertions
For store tests, use createPinia() and setActivePinia
Only use createTestingPinia when its special features are needed
Let stores initialize with natural default state; don’t mock the store under test
Files:
web/__test__/components/Wrapper/mount-engine.test.ts
🧠 Learnings (7)
📚 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.tsweb/src/components/Wrapper/mount-engine.ts
📚 Learning: 2025-09-12T01:36:59.838Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T01:36:59.838Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,tsx} : Use mount from Vue Test Utils for component testing
Applied to files:
web/__test__/components/Wrapper/mount-engine.test.tsweb/src/components/Wrapper/mount-engine.ts
📚 Learning: 2025-09-12T01:36:59.838Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T01:36:59.838Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,tsx} : Test component behavior and output, not 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 : 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-12T01:36:59.838Z
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T01:36:59.838Z
Learning: Applies to {api,web}/**/*.{test,spec}.{ts,tsx} : Place all mock declarations at the top level; use factory functions for module mocks; clear mocks between 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} : 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
🧬 Code graph analysis (1)
web/src/components/Wrapper/mount-engine.ts (1)
web/src/components/Wrapper/component-registry.ts (1)
componentMappings(23-149)
🪛 ast-grep (0.38.6)
web/src/components/Wrapper/mount-engine.ts
[warning] 203-203: 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)
🔇 Additional comments (4)
web/src/components/Wrapper/mount-engine.ts (4)
67-69: Props parser early-exit LGTMGood fast-path when no attributes exist.
145-153: Selector batching LGTMSingle querySelectorAll over a joined selector is a nice win; early-exit is clean.
154-166: No duplicate selectors found — shadowing not occurringChecked web/src/components/Wrapper/component-registry.ts with the provided script; output: "OK: no duplicates". Resolving this comment. Optional: harden by mapping selector → array of mappings or emit a dev-time warning if a selector is registered more than once.
71-79: OK to skip forwardingid— no component declares a top-levelidprop in web/srcScanned web/src for props/defineProps/withDefaults and top-level
iddeclarations; found only nesteditem.idusages (e.g. Docker/Preview.vue, Docker/Edit.vue) and GraphQL/typeidfields (composables/gql/graphql.ts). No evidence components expect a host-elementidprop.
- Replaced global `__unifiedApp` with `LOCALE_DATA` in the window interface for better clarity. - Simplified access to `LOCALE_DATA` in the `setupI18n` function. - Improved JSON parsing logic in `parsePropsFromElement` by using `trimStart()` for initial character checks. - Updated `mountUnifiedApp` to use `replaceChildren()` for more efficient DOM manipulation. - Adjusted `autoMountAllComponents` to return the result of `mountUnifiedApp` for better function chaining.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/types/window.d.ts (1)
1-1: Use ApolloClient for precise typing.
unknownis safe but loses the canonical cache type; Apollo’s default isNormalizedCacheObject. Tighten the generic.-import type { ApolloClient } from '@apollo/client/core'; +import type { ApolloClient, NormalizedCacheObject } from '@apollo/client/core'; @@ - apolloClient?: ApolloClient<unknown>; + apolloClient?: ApolloClient<NormalizedCacheObject>;Also applies to: 14-14
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/Wrapper/mount-engine.ts(4 hunks)web/types/window.d.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript import specifiers with .js extensions for ESM compatibility
Never use the any type; prefer precise typing
Avoid type casting; model proper types from the start
Files:
web/types/window.d.tsweb/src/components/Wrapper/mount-engine.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-12T01:36:59.838Z
Learning: Applies to web/__test__/**/*.{test,spec}.{ts,tsx} : Use mount from Vue Test Utils for component testing
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
📚 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/types/window.d.tsweb/src/components/Wrapper/mount-engine.ts
📚 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/types/window.d.ts
📚 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.ts
🧬 Code graph analysis (1)
web/src/components/Wrapper/mount-engine.ts (1)
web/src/components/Wrapper/component-registry.ts (1)
componentMappings(23-149)
🔇 Additional comments (13)
web/src/components/Wrapper/mount-engine.ts (11)
23-23: LGTM! Proper Window type extension.Adding
LOCALE_DATA?: stringto the global Window interface is correctly implemented with proper optional typing.
39-39: Simplified and more direct locale data access.The refactoring to directly access
window.LOCALE_DATAeliminates unnecessary type assertions and improves code readability.
65-66: Efficient early exit optimization.Good performance optimization by early returning when the element has no attributes, avoiding unnecessary processing.
69-70: Pre-compiled attribute skip set for O(1) lookups.Excellent performance improvement using a Set for constant-time attribute filtering instead of repeated array searches.
76-76: Comprehensive attribute filtering logic.The combined skip condition efficiently handles both common HTML attributes and Vue-specific data attributes with proper prefix matching.
80-84: Smart JSON detection and parsing enhancement.The character-based JSON detection (checking first non-whitespace character) is more efficient than regex-based approaches and properly handles HTML-encoded JSON values.
134-142: Efficient batch processing with selector mapping.Excellent architectural improvement using a Map for O(1) selector-to-mapping lookups and building all selectors for a single DOM query.
147-152: Smart early exit for zero selectors.Good defensive programming with early return when no selectors exist, preventing unnecessary DOM queries and providing helpful debug output.
153-167: Optimized component mounting with deduplication.The batch processing approach with
processedMappingsSet efficiently prevents duplicate mounts while processing multiple selectors in a single pass.
169-218: Comprehensive component mounting with proper cleanup.The refactored mounting logic provides:
- Proper error handling for undefined components
- Efficient prop parsing from DOM attributes
- UApp wrapping for Nuxt UI compatibility
- Shared app context for consistency
- DOM cleanup with
replaceChildren()- Proper mount marking and CSS class addition
- Cleanup handler storage for unmounting
This is a well-architected solution for batch component mounting.
224-224: Updated public API return type.Correctly updating
autoMountAllComponents()to return the App instance aligns with the architectural changes and provides useful access to the unified app context.web/types/window.d.ts (2)
14-14: apolloClient is optional — verified call sites are safe.
Found references in web/src/components/Wrapper/mount-engine.ts and web/src/components/Wrapper/auto-mount.ts; mount-engine uses (typeof window !== 'undefined' && window.apolloClient) || client (fallback), auto-mount assigns window.apolloClient and tests import it. No unguarded reads found.
12-23: Centralize LOCALE_DATA on Window (add LocaleData import + property)Declare LocaleData in web/types/window.d.ts and add LOCALE_DATA?: LocaleData to the Window interface; remove duplicate Window augmentations if present.
import type { autoMountComponent, getMountedApp, mountVueApp } from '~/components/Wrapper/mount-engine'; +import type { LocaleData } from '~/components/Wrapper/mount-engine'; @@ graphqlParse: typeof parse; + + // i18n payload provided by WebGUI + LOCALE_DATA?: LocaleData;Repo-wide search for other Window augmentations declaring LOCALE_DATA did not complete in this run — verify no duplicate declarations remain.
- Commented out the import statement for "@nuxt/ui" to prevent its usage while maintaining the rest of the CSS structure intact.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.24.0](v4.23.1...v4.24.0) (2025-09-18) ### Features * improve dom content loading by being more efficient about component mounting ([#1716](#1716)) ([d8b166e](d8b166e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Chores