Maryhipp/canvas workflows

[maryhipp]

Summary

Related Issues / Discussions

QA Instructions

Merge Plan

Checklist

• The PR has a short but descriptive title, suitable for a changelog
• Tests added / updated (if applicable)
• ❗Changes to a redux slice have a corresponding migration
• Documentation added / updated (if applicable)
• Updated What's New copy (if doing a release after this PR)

[psychedelicious]

Overall I think this will work just fine. Some design suggestions below

[psychedelicious]

Can omit persistConfig instead of adding denylist to skip persistence for the whole slice

[psychedelicious]

But that said, I think we do want persistence here... If I load a workflow into canvas and change some of the fields, I'd expect them to persist.

[psychedelicious]

Think we should skip these and also not add metadata - up to the workflow author to build metadata if they want it.

[maryhipp]

Removed metadata but kept this for the return

[psychedelicious]

This isn't working quite right. The nodes slice always handles the actions even if this listener triggers and dispatches the canvas-specific action also.

For example, if I change the field in the workflow editor, it also changes it in the canvas workflow.

There's an "action router" pattern I've been exploring in #8553 that I think could solve this. The idea is to inject metadata into actions and use it to decide who handles the action. In that PR, the metadata is the canvas ID. For this PR, the metadata would be a flag that says "Is this action for the node editor workflow, or canvas workflow?". Roughed out:

// flag injection/matching utilities, use symbol to prevent collisions
const CANVAS_WORKFLOW_KEY = Symbol('CANVAS_WORKFLOW_KEY');
const NODES_WORKFLOW_KEY = Symbol('NODES_WORKFLOW_KEY');

// call this on the action before dispatching if we are in the canvas workflow, this will go in the useDispatch wrapper below
const injectCanvasWorkflowKey = (action: UnknownAction) => {
  Object.assign(action, { meta: { [CANVAS_WORKFLOW_KEY]: true } }); // could be a canvas id instead of boolean
};

// call this on the action before dispatching if we are in node editor
const injectNodesWorkflowKey = (action: UnknownAction) => {
  Object.assign(action, { meta: { [NODES_WORKFLOW_KEY]: true } });
};

// type guards
const isCanvasWorkflowAction = (action: UnknownAction) => {
  return isPlainObject(action?.meta) && action?.meta?.[CANVAS_WORKFLOW_KEY] === true;
};

const isNodesWorkflowAction = (action: UnknownAction) => {
  return isPlainObject(action?.meta) && action?.meta?.[NODES_WORKFLOW_KEY] === true;
};

// cut and paste all the feild reducers from nodesSlice to this new slice
// but this slice doesn't get added to the store, we just use it as a convenient way to define the reducers
const fieldSlice__DO_NOT_ADD_TO_STORE = createSlice({
  name: 'fields',
  initialState: getInitialState(),
  reducers: {
    fieldValueReset: (state, action: FieldValueAction<StatefulFieldValue>) => {
      fieldValueReducer(state, action, zStatefulFieldValue);
    },
    fieldStringValueChanged: (state, action: FieldValueAction<StringFieldValue>) => {
      fieldValueReducer(state, action, zStringFieldValue);
    },
    //... rest of field reducers
  },
});

// this will match any field action, regardless of its flag
const isFieldAction = isAnyOf(...Object.values(fieldSlice__DO_NOT_ADD_TO_STORE.actions).map((a) => a.match))

Then in nodesSlice and canvasWorkflowSlice we can do this:

const nodesSlice = createSlice({
  // ...
    extraReducers: (builder) => {
    builder.addMatcher(
      isFieldAction,
      (state, action) => {
        if (!isNodesWorkflowAction(action)) {
          return;
        }

        fieldSlice__DO_NOT_ADD_TO_STORE.reducer(state, action);
      }
    );
  },
})

const canvasWorkflowSlice = createSlice({
  // ...
    extraReducers: (builder) => {
    builder.addMatcher(
      isFieldAction,
      (state, action) => {
        if (!isCanvasWorkflowAction(action)) {
          return;
        }

        fieldSlice__DO_NOT_ADD_TO_STORE.reducer(state, action);
      }
    );
  },
})

Wrap useAppDispatch to automatically inject the flags:

export const useAppDispatch = () => {
  const isCanvasWorkflow = useCanvasWorkflowContext() // bool | null
  const isNodesWorkflow = useNodesWorkflowContext() // bool | null
  
  const dispatch = useDispatch();
  
  return useCallback((action) => {
    if (isCanvasWorkflow && isNodesWorkflow) {
      throw new Error('A component cannot be in both a canvas workflow and a nodes workflow');
    }
    
    if (isCanvasWorkflow) {
      injectCanvasWorkflowKey(action);
    } else if (isNodesWorkflow) {
      injectNodesWorkflowKey(action);
    }
    
    return dispatch(action);
    
  }, [dispatch, isCanvasWorkflow, isNodesWorkflow])
}

This pattern is similar to what I'm proposing in #8553 . Plays nicely with it and a future where we have multiple workflow editor instances

[maryhipp]

Lmk what you think of claude's implementation on this