feat(ui): add support for custom field types

[psychedelicious]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update
• Community Node Submission

Have you discussed this change with the InvokeAI team?

• Yes
• No, because:

Description

Node authors may now create their own arbitrary/custom field types. Any pydantic model is supported.

Two notes:

1. Your field type's class name must be unique.

Suggest prefixing fields with something related to the node pack as a kind of namespace.

2. Custom field types function as connection-only fields.

For example, if your custom field has string attributes, you will not get a text input for that attribute when you give a node a field with your custom type.

This is the same behaviour as other complex fields that don't have custom UIs in the workflow editor - like, say, a string collection.

QA Instructions, Screenshots, Recordings

Clone this repo into your invokeai/nodes/ dir: https://github.com/psychedelicious/test-arbitrary-fields

It contains two nodes:

• Custom Field Test 1
• Custom Field Test 2

These two nodes use a custom field called CustomField.

Try adding those nodes and connecting them. Should work like any node.

[psychedelicious]

We have some fields that are have no UI - for example, some of the model fields are only used via connection and will never be used any other way. We could probably handle those fields as "custom" fields, per this PR. It wouldn't reduce any complexity exactly, but it would reduce verbosity.

[RyanJDick]

Tested this and everything works as expected!

I'm glad to see that no backend changes were needed to support this.

There are couple of natural future improvements that I think would make this broadly more useful. You're probably already thinking about these, but figured I'd call them out anyways:

1. UI should only allow connections between unknown types with the same type name. Right now you can connect incompatible unknown types and it won't be caught until graph execution time.
2. I think we could hash the unknown type name and map it to a color to still get a unique color for each custom type in the UI.

[psychedelicious]

Oops, #1 is unintentional. I'll figure out a fix for that in this PR as it may change the implementation.

I suspect different colors for every type will end up looking rather chaotic, but it's worth trying out. I'll save that for the future.

[psychedelicious]

Fixing the validation was a bit more involved than I would have hoped, but not terrible.

I updated the nodes in https://github.com/psychedelicious/test-arbitrary-fields with an extra field type to facilitate testing multiple custom field types.

@RyanJDick Would you mind giving this a spin again?

Notes:

feat(ui): custom field types connection validation

In the initial commit, a custom field's original type was added to the field templates only as originalType. Custom fields' type property was "Custom"*. This allowed for type safety throughout the UI logic.

*Actually, it was "Unknown", but I changed it to custom for clarity.

Connection validation logic, however, uses the field instance of the node/field. Like the templates, field instances with custom types have their type set to "Custom", but they didn't have an originalType property. As a result, all custom fields could be connected to all other custom fields.

To resolve this, we need to add originalType to the field instances, then switch the validation logic to use this instead of type.

This ended up needing a bit of fanagling:

• If we make originalType a required property on field instances, existing workflows will break during connection validation, because they won't have this property. We'd need a new layer of logic to migrate the workflows, adding the new originalType property.

While this layer is probably needed anyways, typing originalType as optional is much simpler. Workflow migration logic can come layer.

(Technically, we could remove all references to field types from the workflow files, and let the templates hold all this information. This feels like a significant change and I'm reluctant to do it now.)

• Because originalType is optional, anywhere we care about the type of a field, we need to use it over type. So there are a number of field.originalType ?? field.type expressions. This is a bit of a gotcha, we'll need to remember this in the future.

• We use Array.prototype.includes() often in the workflow editor, e.g. COLLECTION_TYPES.includes(type). In these cases, the const array is of type FieldType[], and type is is FieldType.

Because we now support custom types, the arg type is now widened from FieldType to string.

This causes a TS error. This behaviour is somewhat controversial (see microsoft/TypeScript#14520). These expressions are now rewritten as COLLECTION_TYPES.some((t) => t === type) to satisfy TS. It's logically equivalent.

[RyanJDick]

Haven't tested thoroughly or looked at the code yet, but in my initial tests it wasn't working for list types. I.e. the following wasn't working:

class CustomFieldOutput(BaseInvocationOutput):
    """Base class for nodes that output a single some field"""

    my_field: list[CustomField] = OutputField()

Edit: I don't think I ever tested this on the original commit, so this may have been an issues from the start.

[psychedelicious]

Yep, I forgot about Collection and Polymorphic types.

I've updated the PR to handle these cases, and my custom node test repo to include both Collections and Polymorphics.

(This part of the UI really could use some unit tests)

[RyanJDick]

Tested manually and didn't run into any problems.

I did my best to review, but have admittedly limited familiarity with how the frontend type handling works.

[RyanJDick]

nit: Here and in several other places in this PR we use FieldType | string, which is equivalent to just string.

I'd tend to just use the string type. Unless you think including FieldType improves readability?

[psychedelicious]

Yep, just for readability. TS also widens it to string, so when you use intellisense, you also see string.

I could be convinced it's less readable than just string, though.

[RyanJDick]

I'm not entirely convinced that this is a desired outcome. I definitely have less context than you do, but on the surface I'd tend towards not wanting to rely on any particular type name structure.

Happy to save this discussion for when the time comes though!

[RyanJDick]

I'm certain that I'm not aware of all of the constraints here, but seeing this makes me think that we should definitely try to move toward a structured type representation rather than relying on type names for everything.

i.e. something like:

type FieldType = {
  name: string;
  isCollection: boolean;
  isPolymorphic: boolean;
  // Maybe?
  isCustom: boolean;
}

Is this something you're already thinking about? Are there constraints that make this hard to do today?

[psychedelicious]

Yeah I was wishing I made it like that initially. The collection and polymorphic support wasn't in the initial design so it was tacked on like you see now.

The only constraint is that existing workflows have field types baked in to them - another thing that probably isn't optimal, in hindsight. Changing this would need some migration logic.

I'm going to take a step back and think about this a bit more. I'm working on the workflow library now. It may be the best time to make some improvements like this.

[psychedelicious]

superseded by #5175