Crop box for ImageCrop Node #7014
Conversation
jtydhr88
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
dosubot
bot
added
the
size:XL
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a Vue image-crop feature: a Vue SFC component, a composable managing crop state and handlers, litegraph widget types/renderer, extension registration wiring, widget registry entry, and English localization strings. Changes
Sequence Diagram(s)sequenceDiagram
participant ExtSvc as Extension Service
participant ImgCropExt as imageCrop Extension
participant NodeGraph as Node Graph
participant WidgetReg as Widget Registry
participant VueComp as WidgetImageCrop
participant Composable as useImageCrop
participant Handlers as cropWidgetChangeHandlers
ExtSvc->>ImgCropExt: initialize
ImgCropExt->>NodeGraph: on ImageCrop node created (ensure size ≥300×400)
ImgCropExt->>Handlers: register change callback for nodeId
ImgCropExt->>WidgetReg: add imagecrop widget (Vue-only)
WidgetReg->>VueComp: mount (props: nodeId)
VueComp->>Composable: useImageCrop(nodeId)
Composable->>NodeGraph: read node outputs / image URL
Composable->>VueComp: provide refs, styles, handlers
VueComp->>VueComp: render loading / placeholder / crop UI
par User moves crop box
VueComp->>Composable: drag handlers update crop state
Composable->>Handlers: emit x,y updates for nodeId
Handlers->>NodeGraph: persist parameters
and User resizes crop
VueComp->>Composable: resize handlers update crop state
Composable->>Handlers: emit width,height updates for nodeId
Handlers->>NodeGraph: persist parameters
end
VueComp->>VueComp: update cropBoxStyle & cropImageStyle (reactive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎭 Playwright Test Results⏰ Completed at: 11/29/2025, 01:35:11 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/29/2025, 01:26:02 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🔴 +2.31 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 949 kB (baseline 949 kB) • ⚪ 0 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 139 kB (baseline 139 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 17.2 kB (baseline 2.94 kB) • 🔴 +14.3 kBHelpers, composables, and utility bundles
Status: 2 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.85 MB (baseline 3.84 MB) • 🔴 +5.48 kBBundles that do not match a named category
Status: 18 added / 17 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/lib/litegraph/src/widgets/ImageCropWidget.ts (1)
15-42: Consider using canvas save/restore for context state.The current approach of manually saving and restoring context properties works, but the canvas API provides
ctx.save()andctx.restore()methods for this purpose, which is more idiomatic:drawWidget(ctx: CanvasRenderingContext2D, options: DrawWidgetOptions): void { const { width } = options const { y, height } = this - const { fillStyle, strokeStyle, textAlign, textBaseline, font } = ctx + ctx.save() ctx.fillStyle = this.background_color ctx.fillRect(15, y, width - 30, height) ctx.strokeStyle = this.outline_color ctx.strokeRect(15, y, width - 30, height) ctx.fillStyle = this.text_color ctx.font = '11px monospace' ctx.textAlign = 'center' ctx.textBaseline = 'middle' const text = 'ImageCrop: Vue-only' ctx.fillText(text, width / 2, y + height / 2) - Object.assign(ctx, { - fillStyle, - strokeStyle, - textAlign, - textBaseline, - font - }) + ctx.restore() }However, the current implementation is acceptable and follows patterns seen elsewhere in the codebase.
src/composables/useImageCrop.ts (4)
73-98: Tighten widget helper typing to prevent name typos
getWidgetValue/setWidgetValuecurrently acceptname: string, but all callers use the fixed set'x' | 'y' | 'width' | 'height'. Making that union explicit will give you compile‑time protection against typos and keep the helpers aligned withCropWidgetChangeHandler.You can refactor like this:
-type CropWidgetChangeHandler = ( - name: 'x' | 'y' | 'width' | 'height', - value: number -) => void +type CropWidgetField = 'x' | 'y' | 'width' | 'height' + +type CropWidgetChangeHandler = (name: CropWidgetField, value: number) => void - const getWidgetValue = (name: string): number => { + const getWidgetValue = (name: CropWidgetField): number => { @@ - const setWidgetValue = (name: string, value: number) => { + const setWidgetValue = (name: CropWidgetField, value: number) => {Call sites to
setWidgetValue/getWidgetValuealready match this union.Also applies to: 345-347, 420-427
38-40:isLoadingis never set totrue— spinner state won’t work
isLoadingis only set tofalseinhandleImageLoad/handleImageError, but it’s never set totruewhen a new image URL is assigned. Any consumer using this ref to show a loading spinner won’t see a “loading” state.You can flip it on when the URL changes:
const updateImageUrl = () => { - imageUrl.value = getInputImageUrl() + const newUrl = getInputImageUrl() + imageUrl.value = newUrl + isLoading.value = !!newUrl }This keeps the load/error handlers correct (they already reset it to
false).Also applies to: 133-135, 299-307
309-314: Defensively handle pointer capture to avoid runtime errors on non‑HTMLElement targets
capturePointer/releasePointercaste.target as HTMLElementand call the pointer APIs directly. In normal usage these will be bound to elements, but it only takes one mis‑bound handler or synthetic event to trigger a runtime error (setPointerCaptureundefined).A small guard keeps this safe without changing behavior:
- const capturePointer = (e: PointerEvent) => - (e.target as HTMLElement).setPointerCapture(e.pointerId) - - const releasePointer = (e: PointerEvent) => - (e.target as HTMLElement).releasePointerCapture(e.pointerId) + const capturePointer = (e: PointerEvent) => { + const target = e.target + if (target instanceof HTMLElement && target.setPointerCapture) { + target.setPointerCapture(e.pointerId) + } + } + + const releasePointer = (e: PointerEvent) => { + const target = e.target + if (target instanceof HTMLElement && target.releasePointerCapture) { + target.releasePointerCapture(e.pointerId) + } + }Same semantics in the happy path, safer in edge cases.
Also applies to: 315-325, 356-370, 430-436
438-456: Initialize nodeId check and guardResizeObserverfor non‑browser/test environmentsTwo small robustness points in
initialize:
- The
if (nodeId)check treats0/''as “no node”. IfNodeIdever includes0, this would skipgetNodeById. Since you already usenodeId == nullelsewhere, it’s safer to be consistent:- const initialize = () => { - if (nodeId) { - node.value = app.rootGraph?.getNodeById(nodeId) || null - } + const initialize = () => { + if (nodeId != null) { + node.value = app.rootGraph?.getNodeById(nodeId) || null + }
- Directly constructing
ResizeObservercan throw in non‑browser or older test environments. A light guard avoids that without affecting normal usage:- resizeObserver = new ResizeObserver(() => { - if (imageEl.value && imageUrl.value) { - updateDisplayedDimensions() - } - }) - - if (containerEl.value) { - resizeObserver.observe(containerEl.value) - } + if (typeof ResizeObserver !== 'undefined') { + resizeObserver = new ResizeObserver(() => { + if (imageEl.value && imageUrl.value) { + updateDisplayedDimensions() + } + }) + + if (containerEl.value) { + resizeObserver.observe(containerEl.value) + } + }Both are small tweaks but improve resilience.
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: Crop box for ImageCrop Node (#7014)
Impact: 692 additions, 1 deletions across 9 files
Issue Distribution
- Critical: 0
- High: 1
- Medium: 5
- Low: 1
Category Breakdown
- Architecture: 1 issues
- Security: 1 issues
- Performance: 3 issues
- Code Quality: 2 issues
Key Findings
Architecture & Design
The implementation introduces a new interactive crop box widget that follows Vue 3 Composition API patterns well. However, it uses a global Map for state management instead of the established Pinia store pattern, which could impact maintainability and debugging capabilities.
Security Considerations
Found a potential XSS vulnerability where imageUrl is directly interpolated into CSS background-image properties without sanitization. This could allow injection of malicious CSS/JavaScript through image URLs.
Performance Impact
Several performance concerns were identified:
- No debouncing of frequent widget updates during drag/resize operations
- Expensive computations in computed properties that run on every reactive change
- Manual implementations instead of leveraging VueUse composables
Integration Points
The widget integrates well with the existing Vue node system and LiteGraph architecture. The extension registration follows established patterns and the widget type definitions are properly integrated.
Positive Observations
- Clean Vue 3 Composition API usage with proper reactivity
- Comprehensive TypeScript typing throughout the implementation
- Good separation of concerns between the composable and Vue component
- Proper internationalization support
- Thoughtful UI/UX design with proper visual feedback and cursor states
- Robust mathematical calculations for image scaling and crop positioning
- Good error handling for image load failures and edge cases
References
Next Steps
- Address the high-priority security issue with URL sanitization before merge
- Consider implementing performance optimizations for better user experience
- Evaluate architectural decisions for long-term maintainability
- Add accessibility features for better inclusive design
- Consider adding unit tests for the crop calculation logic
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
github-actions
bot
removed
the
claude-review
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/composables/useImageCrop.ts (2)
78-95: Add runtime validation for widget values to improve type safety.The type assertion
as numberon line 83 and unvalidated assignments on lines 92-93 bypass TypeScript's type checking. If a widget's value is not a number or its callback expects a different signature, this could cause runtime errors.Consider adding runtime validation:
const getWidgetValue = (name: string): number => { if (!node.value) return 0 const widget = node.value.widgets?.find((w) => w.name === name) + const value = widget?.value + return typeof value === 'number' ? value : 0 - return (widget?.value as number) ?? 0 } const setWidgetValue = (name: string, value: number) => { if (!node.value) return const widget = node.value.widgets?.find((w) => w.name === name) if (widget) { + if (typeof value !== 'number' || !isFinite(value)) return widget.value = value widget.callback?.(value) } }
321-453: Consider using VueUse'susePointercomposable for pointer event handling.The manual pointer capture/release and event handling could be simplified using VueUse's
usePointercomposable, which provides reactive pointer position and state. This would reduce boilerplate and align with the project's preference for VueUse utilities.Example pattern:
import { usePointer } from '@vueuse/core' const { x, y, pressure, pointerType } = usePointer() // Use reactive x, y in watchers or computed instead of manual event handlersHowever, the current implementation is functional and correct, so this is a nice-to-have improvement rather than a priority.
Based on learnings: The project prefers VueUse composables for performance-enhancing utilities.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/composables/useImageCrop.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/composables/useImageCrop.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/composables/useImageCrop.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/composables/useImageCrop.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/composables/useImageCrop.ts
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/composables/useImageCrop.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/composables/useImageCrop.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/composables/useImageCrop.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/composables/useImageCrop.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/composables/useImageCrop.ts
src/**/{services,composables}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/{services,composables}/**/*.{ts,tsx}: Useapi.apiURL()for backend endpoints instead of constructing URLs directly
Useapi.fileURL()for static file access instead of constructing URLs directly
Files:
src/composables/useImageCrop.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/composables/useImageCrop.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/composables/useImageCrop.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/composables/useImageCrop.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/composables/useImageCrop.ts
**/composables/use*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables in the format
useXyz.ts
Files:
src/composables/useImageCrop.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.{vue,ts,tsx} : Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
📚 Learning: 2025-11-24T19:47:34.324Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:34.324Z
Learning: Applies to src/**/*.{ts,tsx,vue} : Sanitize HTML with DOMPurify to prevent XSS attacks
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.{vue,ts,tsx} : Leverage VueUse functions for performance-enhancing utilities
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing styles
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Utilize ref and reactive for reactive state in Vue 3
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Use ref/reactive for state management in Vue 3 Composition API
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T19:47:02.860Z
Learning: Applies to src/**/*.vue : Utilize ref and reactive for reactive state
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition API
Applied to files:
src/composables/useImageCrop.ts
📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-24T19:46:52.279Z
Learning: Applies to **/*.vue : Do not use deprecated PrimeVue components (Dropdown, OverlayPanel, Calendar, InputSwitch, Sidebar, Chips, TabMenu, Steps, InlineMessage). Use replacements: Select, Popover, DatePicker, ToggleSwitch, Drawer, AutoComplete, Tabs, Stepper, Message respectively
Applied to files:
src/composables/useImageCrop.ts
🧬 Code graph analysis (1)
src/composables/useImageCrop.ts (1)
src/scripts/app.ts (1)
app(1708-1708)
🪛 ESLint
src/composables/useImageCrop.ts
[error] 1-1: Resolve error: EACCES: permission denied, open '/EwBTEioqGB'
at Object.writeFileSync (node:fs:2409:20)
at l (file:///home/jailuser/git/node_modules/.pnpm/[email protected]/node_modules/get-tsconfig/dist/index.mjs:7:13670)
at createFilesMatcher (file:///home/jailuser/git/node_modules/.pnpm/[email protected]/node_modules/get-tsconfig/dist/index.mjs:7:14422)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/[email protected][email protected]_@typescript-eslin_da4796079dab5a32abf73f9910d12370/node_modules/eslint-import-resolver-typescript/lib/index.js:70:65)
at Object.resolve (file:///home/jailuser/git/node_modules/.pnpm/[email protected][email protected]_@typescript-eslin_da4796079dab5a32abf73f9910d12370/node_modules/eslint-import-resolver-typescript/lib/index.js:147:20)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:69
at setRuleContext (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-import-context/lib/index.js:23:20)
at fullResolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:30)
at relative (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:215:12)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:220:16)
at ExportMap.get (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/export-map.js:88:22)
at processBodyStatement (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/rules/namespace.js:9:31)
at Program (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/rules/namespace.js:100:21)
at ruleErrorHandler (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1173:33)
at /home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-visitor.js:76:46
at Array.forEach ()
at SourceCodeVisitor.callSync (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-visitor.js:76:30)
at /home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-traverser.js:291:18
at Array.forEach ()
at SourceCodeTraverser.traverseSync (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-traverser.js:290:10)
at runRules (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1214:12)
at #flatVerifyWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2101:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2189:43)
at Linter._verifyWithFlatConfigArray (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2292:15)
at Linter.verify (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1677:10)
at Linter.verifyAndFix (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2557:20)
at verifyText (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint-helpers.js:1179:45)
at readAndVerifyFile (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint-helpers.js:1320:10)
(import-x/namespace)
[error] 1-1: Resolve error: EACCES: permission denied, open '/RJnaDzrHZO'
at Object.writeFileSync (node:fs:2409:20)
at l (file:///home/jailuser/git/node_modules/.pnpm/[email protected]/node_modules/get-tsconfig/dist/index.mjs:7:13670)
at createFilesMatcher (file:///home/jailuser/git/node_modules/.pnpm/[email protected]/node_modules/get-tsconfig/dist/index.mjs:7:14422)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/[email protected][email protected]_@typescript-eslin_da4796079dab5a32abf73f9910d12370/node_modules/eslint-import-resolver-typescript/lib/index.js:70:65)
at Object.resolve (file:///home/jailuser/git/node_modules/.pnpm/[email protected][email protected]_@typescript-eslin_da4796079dab5a32abf73f9910d12370/node_modules/eslint-import-resolver-typescript/lib/index.js:147:20)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:69
at setRuleContext (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-import-context/lib/index.js:23:20)
at fullResolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:30)
at relative (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:215:12)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:220:16)
at checkSourceValue (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/rules/no-unresolved.js:31:34)
at checkSourceValue (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:14:9)
at checkSource (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:17:9)
at ruleErrorHandler (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1173:33)
at /home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-visitor.js:76:46
at Array.forEach ()
at SourceCodeVisitor.callSync (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-visitor.js:76:30)
at /home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-traverser.js:291:18
at Array.forEach ()
at SourceCodeTraverser.traverseSync (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-traverser.js:290:10)
at runRules (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1214:12)
at #flatVerifyWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2101:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2189:43)
at Linter._verifyWithFlatConfigArray (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2292:15)
at Linter.verify (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1677:10)
at Linter.verifyAndFix (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2557:20)
at verifyText (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint-helpers.js:1179:45)
at readAndVerifyFile (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint-helpers.js:1320:10)
(import-x/no-unresolved)
[error] 1-1: Resolve error: EACCES: permission denied, open '/lYoUuhKhJu'
at Object.writeFileSync (node:fs:2409:20)
at l (file:///home/jailuser/git/node_modules/.pnpm/[email protected]/node_modules/get-tsconfig/dist/index.mjs:7:13670)
at createFilesMatcher (file:///home/jailuser/git/node_modules/.pnpm/[email protected]/node_modules/get-tsconfig/dist/index.mjs:7:14422)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/[email protected][email protected]_@typescript-eslin_da4796079dab5a32abf73f9910d12370/node_modules/eslint-import-resolver-typescript/lib/index.js:70:65)
at Object.resolve (file:///home/jailuser/git/node_modules/.pnpm/[email protected][email protected]_@typescript-eslin_da4796079dab5a32abf73f9910d12370/node_modules/eslint-import-resolver-typescript/lib/index.js:147:20)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:69
at setRuleContext (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint-import-context/lib/index.js:23:20)
at fullResolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:170:30)
at relative (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:215:12)
at resolve (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/resolve.js:220:16)
at importType (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/import-type.js:126:63)
at checkImportForRelativePackage (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/rules/no-relative-packages.js:15:38)
at file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/rules/no-relative-packages.js:59:40
at checkSourceValue (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:14:9)
at checkSource (file:///home/jailuser/git/node_modules/.pnpm/eslint-plugin-import-x@4.16.1_@typescript-eslint[email protected]_eslint@[email protected]__d4b6b79e6f12f59d34d55ebbf27dc73f/node_modules/eslint-plugin-import-x/lib/utils/module-visitor.js:17:9)
at ruleErrorHandler (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1173:33)
at /home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-visitor.js:76:46
at Array.forEach ()
at SourceCodeVisitor.callSync (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-visitor.js:76:30)
at /home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-traverser.js:291:18
at Array.forEach ()
at SourceCodeTraverser.traverseSync (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/source-code-traverser.js:290:10)
at runRules (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1214:12)
at #flatVerifyWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2101:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2189:43)
at Linter._verifyWithFlatConfigArray (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2292:15)
at Linter.verify (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:1677:10)
at Linter.verifyAndFix (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/linter/linter.js:2557:20)
at verifyText (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint-helpers.js:1179:45)
at readAndVerifyFile (/home/jailuser/git/node_modules/.pnpm/[email protected][email protected]/node_modules/eslint/lib/eslint/eslint-helpers.js:1320:10)
(import-x/no-relative-packages)
[error] 1-1: Unable to resolve path to module '@vueuse/core'.
(import-x/no-unresolved)
[error] 2-2: Unable to resolve path to module 'vue'.
(import-x/no-unresolved)
[error] 5-5: Unable to resolve path to module '@/scripts/app'.
(import-x/no-unresolved)
[error] 6-6: Unable to resolve path to module '@/stores/imagePreviewStore'.
(import-x/no-unresolved)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint-and-format
- GitHub Check: test
- GitHub Check: collect
- GitHub Check: setup
🔇 Additional comments (1)
src/composables/useImageCrop.ts (1)
327-453: Drag and resize implementation is already performant.The drag and resize handlers correctly optimize widget updates by:
- Updating local reactive state (
cropX,cropY,cropWidth,cropHeight) during pointer move events- Only calling
setWidgetValueonce at the end of the drag/resize operationThis approach provides smooth visual feedback without triggering excessive widget callbacks, which is the optimal pattern for interactive operations.
Note: A past review comment suggested adding debouncing/throttling, but that optimization is unnecessary here since widget updates already occur only at operation completion.
| const updateDisplayedDimensions = () => { | ||
| if (!imageEl.value || !containerEl.value) return | ||
|
|
||
| const img = imageEl.value | ||
| const container = containerEl.value | ||
|
|
||
| naturalWidth.value = img.naturalWidth | ||
| naturalHeight.value = img.naturalHeight | ||
|
|
||
| const containerWidth = container.clientWidth | ||
| const containerHeight = container.clientHeight | ||
|
|
||
| const imageAspect = naturalWidth.value / naturalHeight.value | ||
| const containerAspect = containerWidth / containerHeight | ||
|
|
||
| if (imageAspect > containerAspect) { | ||
| displayedWidth.value = containerWidth | ||
| displayedHeight.value = containerWidth / imageAspect | ||
| imageOffsetX.value = 0 | ||
| imageOffsetY.value = (containerHeight - displayedHeight.value) / 2 | ||
| } else { | ||
| displayedHeight.value = containerHeight | ||
| displayedWidth.value = containerHeight * imageAspect | ||
| imageOffsetX.value = (containerWidth - displayedWidth.value) / 2 | ||
| imageOffsetY.value = 0 | ||
| } | ||
|
|
||
| if (naturalWidth.value <= 0 || displayedWidth.value <= 0) { | ||
| scaleFactor.value = 1 | ||
| } else { | ||
| scaleFactor.value = displayedWidth.value / naturalWidth.value | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Guard against zero naturalHeight to prevent Infinity in aspect ratio calculation.
Line 154 divides by naturalHeight.value without checking if it's zero. While rare, corrupt or incompletely loaded images could cause this, resulting in imageAspect = Infinity and broken layout calculations.
Add a guard at the start of the function:
const updateDisplayedDimensions = () => {
if (!imageEl.value || !containerEl.value) return
const img = imageEl.value
const container = containerEl.value
naturalWidth.value = img.naturalWidth
naturalHeight.value = img.naturalHeight
+
+ if (naturalWidth.value <= 0 || naturalHeight.value <= 0) {
+ scaleFactor.value = 1
+ return
+ }
const containerWidth = container.clientWidth
const containerHeight = container.clientHeight
const imageAspect = naturalWidth.value / naturalHeight.value🤖 Prompt for AI Agents
In src/composables/useImageCrop.ts around lines 142 to 174, the code computes
imageAspect by dividing naturalWidth.value by naturalHeight.value without
guarding against naturalHeight being zero; add a guard at the top of
updateDisplayedDimensions to detect if naturalHeight.value is null/undefined/0
(or img.naturalHeight is falsy) and in that case set sensible defaults (e.g.,
displayedWidth=containerWidth, displayedHeight=containerHeight, imageOffsetX=0,
imageOffsetY=0, scaleFactor=1) or early-return after setting those defaults so
no division by zero occurs; also ensure later scaleFactor computation similarly
checks naturalWidth and displayedWidth before dividing.
Summary
Limitations
canvas mode
Screenshots
2025-11-27.20-07-32.mp4
┆Issue is synchronized with this Notion page by Unito