[legacy-framework] Subtemplate management #2134
Conversation
|
Hey @maastrich just an update here, I got swamped this week with client work and haven't had much time on Blitz. Reviewing #2101 is first in line. Once I finish that I'll give a thorough review and testing of this. I appreciate your patience! |
|
No problem it's not a hurry ;) |
There was a problem hiding this comment.
This is awesome, thank you!!
Let's adjust this to support multiple subtemplate types.
- change the
template:keyword tofieldTemplate:which makes also makes it more clear what it does - change
getTemplateValues.subTemplateValues: {...}to begetTemplateValues.fieldTemplate: {...}
And one other sort of big thing to address is we want to be able to update files with subtemplates without overwriting the whole file. Currently if you generate some code, customize it, then add a new model field, it will overwrite your customizations. So instead of existing files, we need to just insert the new field above the template comment. I think probably we should only do this custom insertion if the file has a subtemplate comment. If it doesn't, then use the current overwrite behavior.
| fieldName: singleCamel(valueName), | ||
| FieldName: singlePascal(valueName).replace(/(?!^)([A-Z])/g, " $1"), |
There was a problem hiding this comment.
Also add the following which will put spaces between the words.
field_nameField_nameField_Name- ex:myFirstName->My First Name
| @@ -6,7 +6,7 @@ export {FORM_ERROR} from "app/core/components/Form" | |||
| export function __ModelName__Form<S extends z.ZodType<any, any>>(props: FormProps<S>) { | |||
| return ( | |||
| <Form<S> {...props}> | |||
| <LabeledTextField name="name" label="Name" placeholder="Name" /> | |||
| {/* template: <LabeledTextField name="__fieldName__" label="__FieldName__" placeholder="__FieldName__" /> */} | |||
There was a problem hiding this comment.
| {/* template: <LabeledTextField name="__fieldName__" label="__FieldName__" placeholder="__FieldName__" /> */} | |
| {/* template: <LabeledTextField name="__fieldName__" label="__Field_Name__" placeholder="__Field_Name__" /> */} |
| modelNames: this.options.modelNames, | ||
| ModelName: this.options.ModelName, | ||
| ModelNames: this.options.ModelNames, | ||
| subTemplateValues: this.options.extraArgs?.map((arg: string) => { | ||
| const [valueName, typeName] = arg.split(":") | ||
| return { | ||
| attributeName: singleCamel(valueName), | ||
| zodTypeName: this.getZodTypeName(typeName), | ||
| } | ||
| }), |
There was a problem hiding this comment.
- Let's abstract out all these
getTemplateValuesinto the base Generator class. Currently all our generators provide the same basic values, so we need to centralize it so they stay consistant. We'll wantsubTemplateValuesavailable in all our generators- But we should allow subclasses to add or override the default values.
|
I just thought of something kind of important. Currently we have {/* fieldTemplate: <LabeledTextField name="__fieldName__" label="__Field_Name__" */}But we don't want to always use We don't need to add this in this PR, but we should consider how we can add it later. My first thought is something like this: {/* fieldTemplate: <__FieldComponent__ name="__fieldName__" label="__Field_Name__" */}
// And then provide a map in blitz.config.js or in templates/config or something
{
fieldComponents: {
LabeledTextField: ['string'],
LabeledCheckboxField: ['boolean'],
LabeledDateField: ['date'],
}
} |
|
Thanks @flybayer for your review I'll change the vars names, for the rest of what you propose I thought quite the same but I was thinking that those are more new brand features but this PR is just an "upgrade" of existing features. Anyway, what you proposed is exactly what I thought too add. May be we should call each other on Monday or something to plan what is the first feat to add etc... |
|
Hey @maastrich any update on this? :) |
|
@maastrich sweet! Can you also fix the merge conflicts? |
|
@maastrich , I was wondering if you were still free to work (or are working) on this one? I was thinking of picking it up if you have other commitments at the moment |
|
Hello @roesh, I am (un)fortunately really busy with my job and cannot spend time on Blitz those days, it would be a pleasure to see this work going to an end. |
|
Superseded by #2679 |
itsdillon
changed the title
Closes: #2038
What are the changes and their implications?
Addition of template comment management for the generator
Checklist