-
Notifications
You must be signed in to change notification settings - Fork 491
Road to No Explicit Any Part 9 #8498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b13f5d5
1af9543
baa4809
826dcf8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -7,6 +7,15 @@ import type Load3d from '@/extensions/core/load3d/Load3d' | |||||
| import type { LGraphNode } from '@/lib/litegraph/src/litegraph' | ||||||
| import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema' | ||||||
|
|
||||||
| // Type definitions for Load3D node | ||||||
| interface SceneConfig { | ||||||
| backgroundImage?: string | ||||||
| } | ||||||
|
|
||||||
| interface Load3DNode extends LGraphNode { | ||||||
| syncLoad3dConfig?: () => void | ||||||
| } | ||||||
|
|
||||||
| const viewerInstances = new Map<NodeId, any>() | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Remaining This PR aims to remove explicit ♻️ Proposed refactor-const viewerInstances = new Map<NodeId, any>()
+const viewerInstances = new Map<NodeId, ReturnType<typeof useLoad3dViewer>>()As per coding guidelines: "NEVER use 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| export class Load3dService { | ||||||
|
|
@@ -139,7 +148,9 @@ export class Load3dService { | |||||
| .getCurrentBackgroundInfo() | ||||||
| if (sourceBackgroundInfo.type === 'image') { | ||||||
| const sourceNode = this.getNodeByLoad3d(source) | ||||||
| const sceneConfig = sourceNode?.properties?.['Scene Config'] as any | ||||||
| const sceneConfig = sourceNode?.properties?.['Scene Config'] as | ||||||
| | SceneConfig | ||||||
| | undefined | ||||||
| const backgroundPath = sceneConfig?.backgroundImage | ||||||
| if (backgroundPath) { | ||||||
| await target.setBackgroundImage(backgroundPath) | ||||||
|
|
@@ -179,8 +190,9 @@ export class Load3dService { | |||||
| await viewer.applyChanges() | ||||||
|
|
||||||
| // Sync configuration back to the node's UI | ||||||
| if ((node as any).syncLoad3dConfig) { | ||||||
| ;(node as any).syncLoad3dConfig() | ||||||
| const load3DNode = node as Load3DNode | ||||||
| if (load3DNode.syncLoad3dConfig) { | ||||||
| load3DNode.syncLoad3dConfig() | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 146
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 775
🏁 Script executed:
Repository: Comfy-Org/ComfyUI_frontend
Length of output: 970
Consider importing the existing
SceneConfiginterface instead of defining a local duplicate.There's already a
SceneConfiginterface defined insrc/extensions/core/load3d/interfaces.tswith more complete properties (showGrid,backgroundColor,backgroundImage,backgroundRenderMode). The local definition here is a subset that may drift out of sync.♻️ Proposed refactor to use the existing interface
📝 Committable suggestion
🤖 Prompt for AI Agents