-
Notifications
You must be signed in to change notification settings - Fork 622
Road to No Explicit Any Part 8 (Group4) #8314
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 6 commits
f923c41
e9e4780
8f3e600
154e631
8a447b5
f2ebd59
ab5d356
bfcd642
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 | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2602,7 +2602,16 @@ export class Subgraph | |||||||||||||
| canvas.subgraph = this | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| addInput(name: string, type: string): SubgraphInput { | ||||||||||||||
| addInput(name?: string | null, type?: string | null): SubgraphInput { | ||||||||||||||
| if ( | ||||||||||||||
| name === null || | ||||||||||||||
| name === undefined || | ||||||||||||||
| type === null || | ||||||||||||||
| type === undefined | ||||||||||||||
| ) { | ||||||||||||||
| throw new Error('Name and type are required for subgraph input') | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+2606
to
+2608
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. Runtime validation does not catch The check 🐛 Proposed fix to also check for undefined addInput(name: string, type: string): SubgraphInput {
- if (name === null || type === null) {
+ if (name == null || type == null) {
throw new Error('Name and type are required for subgraph input')
}Using loose equality ( 🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||
| this.events.dispatch('adding-input', { name, type }) | ||||||||||||||
|
|
||||||||||||||
| const input = new SubgraphInput( | ||||||||||||||
|
|
@@ -2620,7 +2629,16 @@ export class Subgraph | |||||||||||||
| return input | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| addOutput(name: string, type: string): SubgraphOutput { | ||||||||||||||
| addOutput(name?: string | null, type?: string | null): SubgraphOutput { | ||||||||||||||
| if ( | ||||||||||||||
| name === null || | ||||||||||||||
| name === undefined || | ||||||||||||||
| type === null || | ||||||||||||||
| type === undefined | ||||||||||||||
| ) { | ||||||||||||||
| throw new Error('Name and type are required for subgraph output') | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+2628
to
+2630
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. Same issue: This check mirrors 🐛 Proposed fix addOutput(name: string, type: string): SubgraphOutput {
- if (name === null || type === null) {
+ if (name == null || type == null) {
throw new Error('Name and type are required for subgraph output')
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
|
Myestery marked this conversation as resolved.
Outdated
|
||||||||||||||
| this.events.dispatch('adding-output', { name, type }) | ||||||||||||||
|
|
||||||||||||||
| const output = new SubgraphOutput( | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,7 @@ import { describe, expect, it, vi } from 'vitest' | |||||||||
|
|
||||||||||
| import type { SubgraphNode } from '@/lib/litegraph/src/litegraph' | ||||||||||
| import { LGraph, Subgraph } from '@/lib/litegraph/src/litegraph' | ||||||||||
| import type { SubgraphInput } from '@/lib/litegraph/src/subgraph/SubgraphInput' | ||||||||||
|
|
||||||||||
| import { subgraphTest } from './__fixtures__/subgraphFixtures' | ||||||||||
| import { | ||||||||||
|
|
@@ -531,7 +532,7 @@ describe.skip('SubgraphNode Cleanup', () => { | |||||||||
|
|
||||||||||
| // Now trigger an event - only node1 should respond | ||||||||||
| subgraph.events.dispatch('input-added', { | ||||||||||
| input: { name: 'test', type: 'number', id: 'test-id' } as any | ||||||||||
| input: { name: 'test', type: 'number', id: 'test-id' } as SubgraphInput | ||||||||||
|
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. 🧹 Nitpick | 🔵 Trivial Consider using The mock object subgraph.events.dispatch('input-added', {
- input: { name: 'test', type: 'number', id: 'test-id' } as SubgraphInput
+ input: { name: 'test', type: 'number', id: 'test-id' } as Partial<SubgraphInput> as SubgraphInput
})This makes it clear the mock is intentionally incomplete. Based on learnings, this pattern is preferred for test mocks. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| }) | ||||||||||
|
|
||||||||||
| // Only node1 should have added an input | ||||||||||
|
|
@@ -558,7 +559,7 @@ describe.skip('SubgraphNode Cleanup', () => { | |||||||||
|
|
||||||||||
| // Trigger an event - no nodes should respond | ||||||||||
| subgraph.events.dispatch('input-added', { | ||||||||||
| input: { name: 'test', type: 'number', id: 'test-id' } as any | ||||||||||
| input: { name: 'test', type: 'number', id: 'test-id' } as SubgraphInput | ||||||||||
|
Myestery marked this conversation as resolved.
|
||||||||||
| }) | ||||||||||
|
|
||||||||||
| // Without cleanup: all 3 removed nodes would have added an input | ||||||||||
|
|
||||||||||
Uh oh!
There was an error while loading. Please reload this page.