Supported nested fields #1621
Supported nested fields #1621
Conversation
Ben-Ho
force-pushed
the
benji-admin-generator
branch
from
9e2b9eb
to
90d0345
Compare
|
|
||
| type Prev = [never, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, ...0[]]; | ||
| type Join<K, P> = K extends string | number ? (P extends string | number ? `${K}${"" extends P ? "" : "."}${P}` : never) : never; | ||
| export type DeepKeyOf<T, D extends number = 10> = [D] extends [never] |
There was a problem hiding this comment.
Fancy. Why does the first, commented out, version not work?
There was a problem hiding this comment.
because it throws an error for a recursive structure like in the demo project. This is handled with Prev-Type.
I also tried to replace Prev and Join with the same-named Types of ts-toolbelt but they behave differently. And I'm actually not so experienced with typescript to find a library-supported solution in an reasonable amount of time.
There was a problem hiding this comment.
typing is now solved 👍
| .replace(/: true,/g, "") | ||
| .replace(/: true/g, "") | ||
| .replace(/"/g, "") | ||
| .replace(/:/g, ""); |
There was a problem hiding this comment.
Hm, you are messing with the JSON output string, wouldn't a recursive function be relatively easy to implement?
There was a problem hiding this comment.
that's right. but I wanted to return nice indentations and json is somewhat similar. I didn't check, but is the code-format done with some prettier command-call? As I saw no "ugly" code. So indentations are probably not even needed, and a flat string would be much easier generated so I could change this json-messing.
There was a problem hiding this comment.
please try it out. (adding an indentation level to the recursion also doesn't sound too complicated)
There was a problem hiding this comment.
done. changed it to recursive function
| @@ -16,13 +17,12 @@ export function generateForm( | |||
| const gqlQueries: Record<string, string> = {}; | |||
| const imports: Imports = []; | |||
|
|
|||
| const fieldNamesFromConfig = config.fields.map<string>((field) => field.name); | |||
There was a problem hiding this comment.
@johnnyomair solved it by adding <string>. Hovering over field.name said type is string. so this was probably some typescript bug.
There was a problem hiding this comment.
I don't understand your comment :)
There was a problem hiding this comment.
regarding the question in slack, how I should fix the typescript issue with recommended refactor using for...of instead of map vs ts-ignore.
There was a problem hiding this comment.
this is also solved 👍
| @@ -58,6 +58,8 @@ export function generateForm( | |||
| \${${`${instanceGqlType}FormFragment`}} | |||
| `; | |||
|
|
|||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | |||
| // @ts-ignore TS2589: Type instantiation is excessively deep and possibly infinite. | |||
There was a problem hiding this comment.
this hint is true, as we use recursion to create fields. but this is required. I didn't look up a different solution or a workaround for this typescript-error.
There was a problem hiding this comment.
this is not a hint, its an error :)
And it says the types are recursive (not the runtime code) which you should avoid.
There was a problem hiding this comment.
alright, then I will look into it 👍
| @@ -17,7 +18,7 @@ export type FormFieldConfig<T> = ( | |||
| | { type: "staticSelect"; values?: string[] } | |||
| | { type: "asyncSelect"; values?: string[] } | |||
| | { type: "block"; block: BlockReference } | |||
| ) & { name: keyof T; label?: string; required?: boolean }; | |||
| ) & { name: Leaves<T> | Paths<T>; label?: string; required?: boolean }; | |||
There was a problem hiding this comment.
this needs more explanation
- what is Leaves and Paths, why do we need both?
- and: we really, really need a demo using this
- there are already a few nested objects for product type
- please do yourself and everyone else a favor and develop using demo
There was a problem hiding this comment.
I faced the problem with the demo-code when I was moving my implementation. (in my specific project the commented-out version did work, as there was no recursion or block-structure)
I tried to support this:
Entity: {
foo: string;
address: {
city: string;
zip: string;
};
block: {
.... some properties
}
}
const formConfig: {
fields: [
{ name: "foo", ...},
{ name: "address.zip", ...}
{ name: "block", ... }
]
}
Leaves only supports "foo", "address.zip", "block.some_property"
Paths only supports "foo", "address", "block"
so I combined those two. I'll add an example for name: "packageDimensions.height" as this is the only case missing.
Would you say this is something that requires a comment next to Leaves | Paths ?
There was a problem hiding this comment.
why do you need Paths? packageDimensions is not a possible value, only packageDimensions.height is
There was a problem hiding this comment.
because of properties like image-block and probably special form-fields supporting other object-structures. (but there may be other solutions like defining multiple properties, e.g. Pick/ShowCoordinates-Field need to set {type: "coordinates", longitude: "place.lng", latitude: "place.lat"} instead of {type: "coordinates", name: "place"}
There was a problem hiding this comment.
I also thought of make this configurable with separate generic parameter FormConfig<T, ObjectFields>
const type MyConfig = FormConfig<Product, "image" | "place">;
but I don't know how to implement this kind of typing-behaviour and another reason: if, as the developer, I'm required to define the special fields, it shouldn't be to much of a problem to just don't use them in the first place.
better typing is probably helpful if my entity is changed afterwards. but to support this we might need some way to detect, at least for now, any block-type.
now using recursive function
…benji-admin-generator
- grid generator - support for blocks in form - support for enum (staticSelect), number and boolean in form - add generated code to demo admin, as 'Products Future' menu item
use v4 instead of v5 to work around esm issues in nodejs
…benji-admin-generator
Co-authored-by: Johannes Obermair <48853629+johnnyomair@users.noreply.github.com>
Co-authored-by: Ben-Ho <bh@vivid-planet.com>
as it removes type-specific fields
|
@nsams I think this pull-request is ready now. It works as expected. 
|
|
closed because of #1695 |
No description provided.