Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions src/components/shared/ReactFlow/FlowCanvas/FlowCanvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -373,13 +373,7 @@ const FlowCanvas = ({

setComponentSpec(updatedComponentSpec);
},
[
reactFlowInstance,
componentSpec,
nodeData,
setComponentSpec,
updateOrAddNodes,
],
[reactFlowInstance, componentSpec, setComponentSpec, updateOrAddNodes],
);

useEffect(() => {
Expand Down
26 changes: 16 additions & 10 deletions src/types/taskNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,31 @@ import type {

import type { Annotations } from "./annotations";

export type TaskType = "task" | "input" | "output";

export interface NodeData extends Record<string, unknown> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future: Do we need to have more arbitrary properties on this object? extending it from Record makes it harder to type causing weird code to appear down the road, e.g.

TaskNode:

const typedData = useMemo(() => data as TaskNodeData, [data]);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vaguely recall trying to not extend and ReactFlow threw up a huge amount of type errors. There may be some larger changes need to our type structure to make this happen, but I do agree with the idea.

readOnly?: boolean;
connectable?: boolean;
Comment on lines +12 to +13
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future: do we have any instance of Graph to be readonly, BUT connectable? I feel that this is single property "readOnly".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more the opposite scenario that's the concern: the graph is not readonly and node connection is disabled - specifically this lock button

image.png

I've been growing increasingly discontent with "readOnly" and how it's used specifically for the run route. I think in future we will want to take a look and properly make readOnly an actual read-only state regardless of the route.

nodeCallbacks?: NodeCallbacks;
}

export interface TaskNodeData extends Record<string, unknown> {
taskSpec?: TaskSpec;
taskId?: string;
readOnly?: boolean;
isGhost?: boolean;
connectable?: boolean;
highlighted?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future: highlighted might not belong to the Node properties/data, but I'm not sure yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely the node overlay or highlighter you made would solve this but we haven't gotten around to looking into it.

callbacks?: TaskNodeCallbacks;
nodeCallbacks?: NodeCallbacks;
callbacks?: TaskCallbacks;
}

export type NodeAndTaskId = {
taskId: string;
nodeId: string;
};

export type TaskType = "task" | "input" | "output";

/* Note: Optional callbacks will cause TypeScript to break when applying the callbacks to the Nodes. */
interface TaskNodeCallbacks {
export interface TaskCallbacks {
setArguments: (args: Record<string, ArgumentType>) => void;
setAnnotations: (annotations: Annotations) => void;
setCacheStaleness: (cacheStaleness: string | undefined) => void;
Expand All @@ -35,13 +40,14 @@ interface TaskNodeCallbacks {
}

// Dynamic Node Callback types - every callback has a version with the node & task id added to it as an input parameter
export type CallbackWithIds<K extends keyof TaskNodeCallbacks> =
TaskNodeCallbacks[K] extends (...args: infer A) => infer R
? (ids: NodeAndTaskId, ...args: A) => R
: never;
type CallbackWithIds<K extends keyof TaskCallbacks> = TaskCallbacks[K] extends (
...args: infer A
) => infer R
? (ids: NodeAndTaskId, ...args: A) => R
: never;

export type NodeCallbacks = {
[K in keyof TaskNodeCallbacks]: CallbackWithIds<K>;
[K in keyof TaskCallbacks]: CallbackWithIds<K>;
};

export type TaskNodeDimensions = { w: number; h: number | undefined };
4 changes: 2 additions & 2 deletions src/utils/nodes/createInputNode.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { type Node } from "@xyflow/react";

import type { TaskNodeData } from "@/types/taskNode";
import type { NodeData } from "@/types/taskNode";

import type { InputSpec } from "../componentSpec";
import { extractPositionFromAnnotations } from "./extractPositionFromAnnotations";
import { inputNameToNodeId } from "./nodeIdUtils";

export const createInputNode = (input: InputSpec, nodeData: TaskNodeData) => {
export const createInputNode = (input: InputSpec, nodeData: NodeData) => {
const { name, annotations, ...rest } = input;

const position = extractPositionFromAnnotations(annotations);
Expand Down
13 changes: 5 additions & 8 deletions src/utils/nodes/createNodesFromComponentSpec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { type Node } from "@xyflow/react";

import type { TaskNodeData } from "@/types/taskNode";
import type { NodeData } from "@/types/taskNode";
import {
type ComponentSpec,
type GraphSpec,
Expand All @@ -13,7 +13,7 @@ import { createTaskNode } from "./createTaskNode";

const createNodesFromComponentSpec = (
componentSpec: ComponentSpec,
nodeData: TaskNodeData,
nodeData: NodeData,
): Node[] => {
if (!isGraphImplementation(componentSpec.implementation)) {
return [];
Expand All @@ -27,24 +27,21 @@ const createNodesFromComponentSpec = (
return [...taskNodes, ...inputNodes, ...outputNodes];
};

const createTaskNodes = (graphSpec: GraphSpec, nodeData: TaskNodeData) => {
const createTaskNodes = (graphSpec: GraphSpec, nodeData: NodeData) => {
return Object.entries(graphSpec.tasks).map((task) =>
createTaskNode(task, nodeData),
);
};

const createInputNodes = (
componentSpec: ComponentSpec,
nodeData: TaskNodeData,
) => {
const createInputNodes = (componentSpec: ComponentSpec, nodeData: NodeData) => {
return (componentSpec.inputs ?? []).map((inputSpec) =>
createInputNode(inputSpec, nodeData),
);
};

const createOutputNodes = (
componentSpec: ComponentSpec,
nodeData: TaskNodeData,
nodeData: NodeData,
) => {
return (componentSpec.outputs ?? []).map((outputSpec) =>
createOutputNode(outputSpec, nodeData),
Expand Down
7 changes: 2 additions & 5 deletions src/utils/nodes/createOutputNode.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { type Node } from "@xyflow/react";

import type { TaskNodeData } from "@/types/taskNode";
import type { NodeData } from "@/types/taskNode";

import type { OutputSpec } from "../componentSpec";
import { extractPositionFromAnnotations } from "./extractPositionFromAnnotations";
import { outputNameToNodeId } from "./nodeIdUtils";

export const createOutputNode = (
output: OutputSpec,
nodeData: TaskNodeData,
) => {
export const createOutputNode = (output: OutputSpec, nodeData: NodeData) => {
const { name, annotations, ...rest } = output;

const position = extractPositionFromAnnotations(annotations);
Expand Down
29 changes: 17 additions & 12 deletions src/utils/nodes/createTaskNode.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,39 @@
import { type Node } from "@xyflow/react";

import type { TaskNodeData } from "@/types/taskNode";
import type { NodeData, TaskNodeData } from "@/types/taskNode";

import type { TaskSpec } from "../componentSpec";
import { extractPositionFromAnnotations } from "./extractPositionFromAnnotations";
import { generateDynamicNodeCallbacks } from "./generateDynamicNodeCallbacks";
import { taskIdToNodeId } from "./nodeIdUtils";
import { convertNodeCallbacksToTaskCallbacks } from "./taskCallbackUtils";

export const createTaskNode = (
task: [`${string}`, TaskSpec],
nodeData: TaskNodeData,
nodeData: NodeData,
) => {
const [taskId, taskSpec] = task;
const { nodeCallbacks, ...data } = nodeData;

const position = extractPositionFromAnnotations(taskSpec.annotations);
const nodeId = taskIdToNodeId(taskId);

// Inject the taskId and nodeId into the callbacks
const nodeCallbacks = nodeData.nodeCallbacks;
const dynamicCallbacks = generateDynamicNodeCallbacks(nodeId, nodeCallbacks);
const taskCallbacks = convertNodeCallbacksToTaskCallbacks(
{ taskId, nodeId },
nodeCallbacks,
);

const taskNodeData: TaskNodeData = {
...data,
taskSpec,
taskId,
highlighted: false,
callbacks: taskCallbacks,
};

return {
id: nodeId,
data: {
...nodeData,
taskSpec,
taskId,
highlighted: false,
callbacks: dynamicCallbacks, // Use these callbacks internally within the node
},
data: taskNodeData,
position: position,
type: "task",
} as Node;
Expand Down
42 changes: 0 additions & 42 deletions src/utils/nodes/generateDynamicNodeCallbacks.ts

This file was deleted.

35 changes: 35 additions & 0 deletions src/utils/nodes/taskCallbackUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type {
NodeAndTaskId,
NodeCallbacks,
TaskCallbacks,
} from "@/types/taskNode";

// Sync TaskCallbacks with NodeCallbacks by injecting nodeId and taskId
export const convertNodeCallbacksToTaskCallbacks = (
ids: NodeAndTaskId,
nodeCallbacks?: NodeCallbacks,
): TaskCallbacks => {
if (!nodeCallbacks) {
return createEmptyTaskCallbacks();
}
return {
setArguments: (args) => nodeCallbacks.setArguments?.(ids, args),
setAnnotations: (annotations) =>
nodeCallbacks.setAnnotations?.(ids, annotations),
setCacheStaleness: (cacheStaleness) =>
nodeCallbacks.setCacheStaleness?.(ids, cacheStaleness),
onDelete: () => nodeCallbacks.onDelete?.(ids),
onDuplicate: (selected) => nodeCallbacks.onDuplicate?.(ids, selected),
onUpgrade: (newComponentRef) =>
nodeCallbacks.onUpgrade?.(ids, newComponentRef),
};
};

const createEmptyTaskCallbacks = (): TaskCallbacks => ({
setArguments: () => {},
setAnnotations: () => {},
setCacheStaleness: () => {},
onDelete: () => {},
onDuplicate: () => {},
onUpgrade: () => {},
});