🪟🧹 Improve connector form types #20057
Conversation
octavia-squidington-iv
added
area/platform
| "default" | "examples" | "description" | "pattern" | "order" | "const" | "title" | "airbyte_hidden" | "enum" | ||
| >; | ||
|
|
||
| interface FormItem extends FormRelevantJSONSchema { |
There was a problem hiding this comment.
Is there a reason to keep order, title, description, and airbyte_hidden in this interface, now that they are coming from FormRelevantJSONSchema?
There was a problem hiding this comment.
There is not, good catch.
| const partialSchema: Partial<AirbyteJSONSchema> = { | ||
| ...Object.fromEntries(Object.entries(schema).filter(([k]) => defaultFields.includes(k as keyof AirbyteJSONSchema))), | ||
| }; | ||
| type DefaultFields = Pick<AirbyteJSONSchema, typeof defaultFields[number] | "enum">; |
There was a problem hiding this comment.
A couple questions here:
- What is
typeof defaultFields[number]doing? It seems like this is just trying to pick all of the fields in thedefaultFieldsarray fromAirbyteJSONSchema, but I don't know what thatnumberin the brackets is being defined. Is this some functionality specific toPick? - Why not just add
"enum"to thedefaultFieldsarray? This way you wouldn't need to do| "enum"unless I've missed something
There was a problem hiding this comment.
I simplified this part of the code, by changing a bit the branching it gets much more readable IMHO.
What is typeof defaultFields[number] doing
It's a way to get a union type of all items in the const array:
The logic is that basically the type of a const array is this:
{
0: "a",
1: "b",
2: "c"
}
so, if you get the types of all number properties of that type, you end up with "a" | "b" | "c". This is called an index access type: https://www.typescriptlang.org/docs/handbook/2/indexed-access-types.html
Just as a side note here, the new code doesn't need that anymore because it adds "enum" in all cases, then removes it again for the "single value enum with default" case which is converted to const. The old code would omit it at first and not add it in this special situation, but the code becomes more readable that way and it also makes pick do the heavy lifting for determining the type which is nice.
|
Based on your comments I simplified a bit further, let me know if there are other questions. |
What
Improve some types in relation to #17352
#17352 itself can be tackled once #14250 is done.
How
FormItemtypes inherit some properties from the underlying JSON schema, and the types should reflect that (also we need the extradefaultfor the linked related task)pickimplementation and use the lodash one to reduce the number of places where typing is lost in the intermediate processing (Object.fromEntries returns an object with almost no type information)