blitz
blitz copied to clipboard
[legacy-framework] Subtemplate management
Closes: blitz-js/blitz#2038
What are the changes and their implications?
Continuing PR blitz-js/blitz#2134 , using @Maastrich 's regex functions in the generator.
This PR improves the generator scaffolding process, allowing files like mutations, pages and forms to include fields specified by the user, rather than have a default "name" field.
Sub tasks
- [x] Support the following replacement cases: field_name, Field_name, Field_Name
- [x] Update form to use this format:
{/* template: <__FieldComponent__ name="__fieldName__" label="__Field_Name__" placeholder="__Field_Name__" /> */}
- [x] Abstract out getTemplateValues into the base Generator class, and allow subclasses to add or override the default values.
- [x] support // template: ... in addition to /* template: ... */ Note: Supported, but only either one can exist in one file. Works like the previous logic we had, but we may want to support multiple template comments per file actually, so once that is decided, we can check this box. (EDIT: Leaving this as it works presently)
- [x] Don't hardcode a display component in the template comment, rather use
__FieldComponent__
, then provide a map in blitz.config.js or in templates/config- [x] Research if the new template key should be placed under the experimental key, or if it can be one of the top level keys
- [x] Determine how expensive
await loadConfigAtRuntime()
is. If it is expensive, create a proxy to hold the typeToComponentMap so that multiple fields can use the config fetched during the first call to get the config object.
- [x] Determine if the logic that sets the
BLITZ_APP_DIR
variable is correct. The following files check if the variable is already set, and if not, sets it to the current directory using "."- [x]
packages\generator\src\generators\template-builders\builder.ts
(New abstract builder class) - [x]
packages\generator\src\generators\page-generator.ts
(Existing page generator class)
- [x]
- [x] Ensure that subsequent runs of
blitz generate
use the existing file as a template if it exists.
Feature Checklist
- [ ] Integration test added (see test docs if needed)
- [x] form-generator test added
- [ ] Documentation added/updated (submit PR to blitzjs.com repo) Docs PR is https://github.com/blitz-js/blitzjs.com/pull/577
For the display component aspect of this, I was wondering if we could strongly type the field mappings in blitz.config.ts
import {LabeledTextField} from "app/core/components/LabeledTextField"
fieldComponents: {
string: {
name: LabeledTextField,
importStatement: `import {LabeledTextField} from "app/core/components/LabeledTextField"`
}
}
Based on my understanding of how JS works and some searching, this does not seem possible (In c#, there is a nameof operator, but JS does not seem to have such a concept.)
Shall we leave these untyped and in the documentation inform users to ensure that the value corresponding to the type should have the correct name and path?
Im adding the path key because I haven't found an ideal way to infer the path to an object based on its reference. I'll search for this some more, but if there is a known way to do this, please let me know.
So we could do:
fieldComponents: {
string: {
name: LabeledTextField,
importStatement: `import {LabeledTextField} from "app/core/components/LabeledTextField"`
}
}
EDIT: After discussing with Brandon, we decided the importStatement prop isn't needed because we include the imports by default in our generators
Progress update 09/03/2021:
Working on support // template: ... in addition to /* template: ... */
Currently in generator.ts, we have a single fieldTemplateRegExp
.
My current solution is create a separate regex for the // template:
Previous regex: /{?\/\* template: (.*) \*\/}?/
New regex: /\/\/ template: (.*)/
The alternative is to use a single regex but have multiple capture (/{?\/\* template: (.*) \*\/}?|\/\/ template: (.*))/
. I may go that path because we later use the fieldTemplateRegExp
. (The issue I'll need to solve is how to use the correct capture group in the right place - $2
captures within {/* */}
, $3
captures within // template:
)
Will continue to update once every couple days, feel free to pitch in with suggestions!
Today's progress update:
Working on making the base Generator class define the getTemplateValues
by unmarking it as an abstract function.
Next steps were to identify the template values that are common among all generators so that we can return an object that contains the maximum number of overlapping values instead of reusing code.
Not all these values have been created already, so I will have to go through each existing generator, see what it will need, and once that is done, update the base Generator's getTemplateValues
to return them.
One potential issue is that the app-generator does not require any model or model fields, so it won't be using any of the values that the others do.
Another issue to solve is updating the GeneratorOptions
base interface in Generator.ts to contain any properties that are needed by the base Generator to generate those values. This is because child generators, like FormGenerator
extend the base GeneratorOptions
with the properties like modelName
and add those to the object returned by their getTemplateValues
method
Progress update 09/12:
When creating the object returned by getTemplateValues
we rely on the options being passed in containing properties such as extraArgs
and so on. Will need to add these to the base Generator's options so that it can access these and build the options
Progress update 09/13:
Abstraction of getTemplateValues
is complete. The base Generator class has a default implementation that children can override. Initially, these problems were present
- One generator (model generator) does not implement any getTemplateValues
- Another generator (app generator) have a completely different set of values
All other generators did output several common fields (like parentModels, modelId, etc.) This could have been solved by making the base generator class return the most common values while app generator and model generator would override conditionally
The issue here is that in order to generate the above options, we need the "extra args" and other models provided into the options argument passed in. This would require adding all of those options into the base generator's options interface.
I personally did not like that the base generator would have a ton of parameters that would have to be set as optional (because not all generators (like the app generator) would provide "modelName", "modelNames" etc.), while their children when extending the base options, would require them to be passed in.
My solution was to create a separate Builder
class that would be responsible for generating these field template values. The base generator class would now "have" a public templateValuesBuilder
, and the base class' getTemplateValues()
will call this builder's getTemplateValues
.
Now, each child class would specify the builder it wanted to use.
Because everything other than the app generator and model generator share ~90% of the values, there are three concrete values builders, which inherit from an abstract template builder:
-
AppValuesBuilder
: Used when generatingtemplateValues
during new app generation -
NullBuilder
: Used by default in the base Generator class, and also used byModelGenerator
which does not override the base -
FieldValuesBuilder
: Used by all other Generators, includingFormGenerator
,MutationsGenerator
, etc.
The base abstract builder class also holds methods like getId
, getParam
, etc. which were originally duplicated across generators themselves.
- Getting component from blitz.config.ts map:
Using const {loadConfigAtRuntime} = await import("next/dist/server/config-shared")
to get the values stored in blitz.config.ts
. Running into the same Error: Internal Blitz Error: process.env.BLITZ_APP_DIR is not set
that Andreas is here: https://github.com/blitz-js/blitz/pull/2699
I believe the pages-generator's postWrite function is also having the same issue so will have to resolve it in all spots.
Progress update 09/14/2020:
Didn't have as much time to work on this today, but I haven't found the best solution for populating the process.env.BLITZ_APP_DIR
variable before calling the loadConfigAtRuntime
Doing process.env.BLITZ_APP_DIR = "."
works, but I might need to ask Brandon if there is a better way this variable needs to be initialized so that we can cover all use cases?
Progress update 09/15/2021:
Added the template to component map support in biltz.config.ts. This required updating the NextConfig
type in the nextjs\packages\next\server\config-shared.ts
file.
The issue is whether this should be nested under the experimental property or if it can stay at the top level. The other issue is understanding how expensive await loadConfigAtRuntime()
is. If it is expensive, we may need to create a proxy to hold the typeToComponentMap so that multiple fields can use the config fetched during the first call to get the config object.
Need to rewrite the form-generator test. It runs locally but fails during CI, I'm assuming because the builder that gets the configuration uses await loadConfigAtRuntime()
which doesn't work in CI, causing config.template
to be undefined.
OK, the fix for issue https://github.com/blitz-js/legacy-framework/issues/302 should resolve the tasks regarding BLITZ_APP_DIR for this PR. Once that is closed, I can check them off here.
09/23: We pushed out a quick fix for that issue, but we did not address
- What the best place to set the BLITZ_APP_DIR was
- What it should be set to
- Integration test to ensure generator commands work and generate the files / output needed
I will ask Brandon about 1) and 2) if it isn't clear after a few hours examining how BLITZ_APP_DIR and options.destinationRoot are used across the generators and more broadly, across the codebase
09/26: Brandon informed me that the BLITZ_APP_DIR variables function was used to find the blitz.config file at runtime. His fix is done at the "cli command" level (see PR https://github.com/blitz-js/blitz/pull/2736/files)
I will be using this same fix
@Roesh you are a gem for working on this!!
Thanks haha! Just noticed one of the CI failing tests was one I wrote, let me fix that. I noticed the failing Windows one but I think that test is being worked on elsewhere?
I'm sure there's a few corrections to be made here and there, things like naming conventions for the files etc. Maastrich's regex code is doing most of the heavy lifting in terms of adding in multiple fields so most of that should be OK. If there are any additional issues, please let me know!
@Roesh ok awesome!! I will review next week.
@beerose can you take first pass through reviewing and testing this? Then I can do a more final review.
I noticed that after generating a new model, the template line remains in the form component. Is it expected?
Yes, that way after can run blitz generate
a second time to add more fields to a model
Update 10/06/2021:
Pulled in the changes from the uuid support PR.
Finished supporting uuid in the mutations template, need to do the same for the prisma generator (currently that PR allows non-id fields to be uuids, but does not affect the primary key) EDIT: that PR also works for the primary key - it didn't work for me because I was trying to overwrite an existing model. Question is whether we want to support that. Generating a new model with id:uuid works
Also, I'm not sure what the best way is to get the parentModelIdType for scaffolding. I might need to parse the current prisma schema and get the appropriate id that way. The current model's id type is determined by an optional id:type argument the user passes in, along with the other fields.
The field values builder uses this extra id argument to update a modelIdZodType
key added to the returned values object.
- [x] Obtain the id type of an existing parent model so that the create zod schema can correctly specify it
- [ ] Check if we should support updating a prisma schema if a new cli command updates the types of the primary key or a different field
- [ ] Research if the
Fallbackable<T>
type is better to use for the component and zod maps, since we're creating fallback maps if the blitz.config.ts/js doesn't have the keys we need. In addition, should we display an error message if the fallbacks were used?
Almost done with getting the parent models id type, but having issues with the generation of the new.tsx file
IDENTICAL app\pages\testmodels\new.tsx
✕ SyntaxError: Illegal return statement
Great! All of that makes sense. I actually didn't notice the blitz g subsequent run issue because I didn't modify a generated file so I assumed we were using the generated file already, but since that isn't the case I'll need to update the generator logic to check for a generated file in the location it will be generated to, then use that file and only add or update fields.
For the maps, I think that will work. I was actually wondering if we need to have an extra Prisma type field. Based on the support uuid PR, it might make sense to update the prisma's Field Type to be dictated by this map as well?
Also, I want to confirm that the types in the map (string, boolean, etc.) will need to be specified in the documentation. The docs currently have json as an extra type, but doesn't specify bigint, so I assume we'll want to update the types map with a json key, and add bigint to the docs?
Progress update 10/17/2021:
Consolidated the types used across resource generators.
TODO:
- [ ] Noticed that the model generator isn't adding a relation field if we use the --parent flag. Looks like this is because it isn't part of the extra args which are what are used when those are generated. Need to create a separate issue to track this
- [ ] Looks like this is also not fully addressed for fields that reference another model, looks like we need to call
prisma format
after the model is generated: https://github.com/blitz-js/blitz/issues/2864 (Could either have someone work on this separately or I'll get to this after the tasks I've worked on)
@Roesh
Config map:
- Yes we can add
prismaType
if we need that. So essentially the keys inside fieldTypeMap are what we accept as arguments toblitz generate
. - I'm now thinking
codegen
would be a better top level config key thantemplate(s)
since codegen is more general.
And yes, need to update docs for field types.
Progress update 10/21/2021:
- Tried to access prisma's functionality that performs
prisma format
, but wasn't very successful. I was rummaging through what the prisma node_modules folders exposed, and none of them have the Format function I am looking for.
- [x] Research to see if there is a way to make a command line call from a generator function
- Updated the template config type to be the new version we wanted, updated the new apps defaults with Brandon's suggestions.
- [x] Need to move it below the commented out webpack config
- [ ] Update generators to work with the new structures
- [ ] Update the prisma generator to get the prisma type from the codegen map
@Roesh here's how you can run prisma format
: https://github.com/blitz-js/blitz/blob/canary/packages/cli/src/commands/new.ts#L320-L322
But do we already do that?
Good question. I've got to check, but I assumed we didn't because in https://github.com/blitz-js/blitz/issues/2864, someone is running into an issue which would have been solved if prisma format was run after model generation.
OK, so it looks like the post-write command is running prisma migrate dev but right now were not formatting before that. Maybe in the past prisma migrate dev first ran prisma format?
I added functionality to do prisma format before prompting the user for the migration and this causes the relationship to be set correctly.
Ok, there's a weird issue where Typescript isn't recognizing spawn.sync
, going to look into that. The cross-spawn index.d.ts
that's being imported seems fine, and other places in the app are able to do the same thing:
const result = spawn.sync(blitzBin, ["--version"], {
cwd: exampleProject,
})
Going to troubleshoot this for a little bit but mighth have to ultimately resort to ts-ignore if I can't figure it our (the functionality itself works)
EDIT: switching to blitz-js/blitz#2864 to discuss this, will post any updates there
Going to work on supporting multiple blitz generates updating existing files this week, I estimate it should be done sometime between Wednesday or Friday.
So, running into a strange issue, I check to ensure a file already exists using
const destinationExists = fs.existsSync(final_destinationPath)
but I get an error when trying to copy that file
✕ Error generating __modelIdParam__\edit.tsx
✕ AssertionError [ERR_ASSERTION]: Trying to copy from a source that does not exist: D:\QuickDocs\blitz\examples\auth\app\pages\test-models\[testModelId]\edit.tsx
I logged what final_destinationPath
has and its
blitz:generator destionation D:\QuickDocs\blitz\examples\auth\app\pages\test-models\[testModelId]\edit.tsx +0ms
meaning the file does exist but for some reason copy.js is throwing that error. Need to look at node_modules\mem-fs-editor\lib\actions\copy.js to see why I think its because of the "[" and "]" characters, but will need to confirm
While I'm debugging that, thought I'd check to see if the other parts work. Deleted the files that have "[" and "]" and found that the logic I've used works, but the overwrite prompt is being bypassed. Need to fix that also. EDIT: Unable to reproduce this, going back to try solve the file existence problem.
In copy.js, in the exports.copy
function
// Sanity checks: Makes sure we copy at least one file.
assert(options.ignoreNoMatch || files.length > 0, 'Trying to copy from a source that does not exist: ' + from);
is the exact location that's failing. files.length
is 0, and the value of from at this point is D:\QuickDocs\blitz\examples\auth\app\pages\test-models\[testModelId]\edit.tsx
. Logging those, I see that both
D:\QuickDocs\blitz\examples\auth\app\pages\test-models\index.tsx
and
D:\QuickDocs\blitz\examples\auth\app\pages\test-models\new.tsx
were found
files.length = 1
for these files and the appropriate source file is used as well. The form component is also copied with the correct updates when the failing files are deleted.
Need to look higher up in that same function to determine how it knows if a file exists.
Progress update 10/25/2021:
Continuing to investigate the copy issue, looks like its coming from
var diskFiles = globby.sync(fromGlob, globOptions);
fromGlob
is set on all paths, but diskFiles is only set for paths that dont end with "[" and "]"
node_modules\globby\index.js > module.exports.sync (Line 138)
10/26/2021: https://github.com/mrmlnc/fast-glob#advanced-syntax Regexp character classes ([1-5]). Escapes characters (\) — matching special characters ($^*+?()[]) as literals. https://github.com/mrmlnc/fast-glob#escapepathpattern
So I think the way this was solved before by the code was by not adding special characters before the file copy, but after it was done. We currently need to be able to copy from a file that has special characters though, so I am using the escapepath() function provided by fast-glob to make sure the paths we're passing in are OK.
Update 10/27/2021:
Need to resolve the Lint issue caused by us not explicitly specifying fast-glob in dependencies or peer dependencies
Update 10/29/2021:
The dependency of fast-glob is reached like so:
packages\generator\package.json
specifies "mem-fs-editor": "8.0.0",
node_modules\mem-fs-editor\package.json
specifies "globby": "^11.0.1"
node_modules\globby\package.json
specifies "fast-glob": "^3.1.1"
Neither globby nor mem-fs-editor expose the escapePath function, only fast-glob does.
@flybayer and @beerose, would it make more sense to specify fast-glob as a dependency to packages\generator\package.json
? I'm not sure how wise this is, even if we write tests in case there are new patterns that are escaped?
The alternative would be to copy what's in the fast-glob escape function, but this again would run into the same problem above
@Roesh you are a champ!! I'm so thankful you are working on this.
Adding fast-glob as a dep sounds goods.
Ayy, thank you :)))
Cool, I'll do that
So I think that's all of the functionality that we needed so far. I'm going to start writing tests, including integration ones where we generate files and examine the output to make sure everything's OK. I might uncover things during that process, but hope to have some tests done by end of week
And fix the Lint issues. I'm having trouble running lint and other testing locally, husky doesn't seem to be working so I've had to --no-verify my commits, but I might try to fix that too
@Roesh awesome!
OK, so almost done, based on our conversation about integration testing, it looks like we'll want to wait for the CLI cypress integration tester to be complete, so all that's left is the accompanying documentation for this
The other thing to note are improvements that can be made to the incremental generation of fields. Right now, we're defaulting to using an existing file and adding fields specified in the new command, but there could be additional generation "modes" which we can consider supporting:
- "From scratch" mode - will wipe the existing resources and replace them with the resources generated by the generate command, useful to quickly discard existing work
- "Template override" mode - will use the existing resources if they exist, but will delete all field specific lines (things like the form fields and zod inputs, basically anything generated by the output of the 'Builder' classes. Useful if someone wants to keep their pages etc., but just want to both delete and/or update resources
- "Add" mode, which is what we do now, which uses an existing file if present and adds any new fields to it, and if the file doesnt exist, generates it
This is amazing work, thank you for this.
Thanks guys! I've added the fields argument to the docs here: https://blitzjs-com-92luv5hui-blitzjs.vercel.app/docs/cli-generate
@flybayer We have an example in I'm wondering if it would make sense to update the generated files to have specific fields like description:string and priority?
We currently have this in the docs:
blitz generate all project will generate the following files:
XXX
For the above example, you can view the generated project index page at localhost:3000/projects
Im considering updating that to
blitz generate all project description:string priority:number will generate the following files:
XXX
For the above example, you can view the generated project index page at localhost:3000/projects
And the blitz new command will generate a project with those fields?
@Roesh ugh, so sorry for delay in testing this out again. I've offloaded a lot of my other blitz work now. So I can follow up here more quickly now.
- yes you can update that doc you mentioned
- [ ] looks like you have a failing unit test: https://github.com/blitz-js/blitz/runs/4245479178?check_suite_focus=true
- [ ] Adding a new field after first generation fails
- cd to
examples/auth/
- Run
blitz g all test name:string
- Run
blitz g all test description:string
- Fails with file doesn't exist, but the file does exist
✕ Error generating __modelIdParam__/edit.tsx ✕ AssertionError [ERR_ASSERTION]: Trying to copy from a source that does not exist: /Users/b/c/blitz/examples/auth/app/pages/tests/\[testId\]/edit.tsx
- cd to
- [x] Previous comment still pending. I tried adding that and looks like
__inputType__
is not used as a template variable. My intention was that every key inside the codegen config (inputType
,zodType
,component
, etc) would be available as a template variable. I guess this could be added later, but I think generating anumber
field type is common enough that it would be good to have in initial release. - [x] I also just realized there's an opportunity for standardization because we use
zodType
in both config and template, but we usecomponent
in config andFieldComponent
in template. So maybe we should useFieldComponent
in the config too? - [x] Prisma type is not being read from blitz.config correctly
- I added a
number
type to blitz.config fieldTypeMap:
number: { component: "LabeledTextField", inputType: "number", zodType: "number", prismaType: "Int", },
- Then
blitz g model apple price:number
results in prismaNumber
instead ofInt
$ blitz g model sauce price:number ✔ Model 'Sauce' created in schema.prisma: > > model Sauce { > id Int @id @default(autoincrement()) > createdAt DateTime @default(now()) > updatedAt DateTime @updatedAt > price Number > } >
- I added a
- [ ] Update mutation is still left with an extra line in the zod schema. (generated with
blitz g all test 'name:string?' price:number
) Not a big deal, but would be nice to be fixedconst UpdateProduct = z.object({ id: z.number(), name: z.string(), price: z.number(), // template: __fieldName__: z.__zodTypeName__(), })
Thats alright! Totally understandable,
I'll get to a few of these at once, and once you verify they're complete, you can check them off.
inputType issue: I made a function that gets the input type just for this, but totally forgot to add the key to the getFieldTemplateValues function. Also updated __ModelName__Form.tsx so it now generates this:
<Form<S> {...props}>
<LabeledTextField name="name" label="Name" placeholder="Name" type="text" />
{/* template: <__FieldComponent__ name="__fieldName__" label="__Field_Name__" placeholder="__Field_Name__" type="__inputType__" /> */}
</Form>
Ah just saw this:
My intention was that every key inside the codegen config (inputType, zodType, component, etc) would be available as a template variable.
Yeah I think I'll need to update the way getTemplateValues is built so that it iterates over the keys in that config file and uses those keys to build instead. So I can add that as a TODO item. EDIT: I updated the logic to add keys based on whats in the config. It basically checks if the fieldType has a map under it, then adds those keys to it. There may be a more efficient way of doing it compared to the loop Im using now (destructure into values maybe?)
Failing unit test: Updated it to take into account the inputType variable that was added. Going to wait until github actions re-runs, haven't been able to resolve the running of tests locally.
11/19/2021:
Prisma type is not being read from blitz.config correctly:
I left a comment that I ended up stashing and forgetting about, oops!
In packages/generator/src/prisma/field.ts
, line 62 needs to be modified
let fieldType = capitalize(_fieldType) //TODO: Need to make the generic function in tempalte buiolders a utility accessiable here.
// Check if it would make sense to expose that to users as well?
// Also in the case of a relationship, need to use the raw model name, cant capitalize it.
11/20/2021:
Ok, so I'm going to make Prisma get values from the codegen key in blitz config, but I'd like to confirm a change to the existing Field.parse function:
// 'name:type?[]:attribute' => Field
static parse(input: string, schema?: ast.Schema): Field[]
I might need to make that function async because we'll need to await a call to loadConfig -
const {loadConfigAtRuntime} = await import("next/dist/server/config-shared")
Its only used in one spot though: packages\generator\src\generators\model-generator.ts
line 42
let field = (extraArgs.length === 1 && extraArgs[0].includes(" ")
? extraArgs[0].split(" ")
: extraArgs
).flatMap((input) => Field.parse(input, schema))
11/21/2021:
Ok, so I'm having issues with getting the config from the field.ts
file, unsure why it isn't picking the config up, hmm
blitz:config Loading config from D:\QuickDocs\blitz\examples\auth\.blitz.config.compiled.js +2s
blitz:config Did not find custom config file +241ms
Now that I think about it more, I need to make sure the other location I use this same call to
const {loadConfigAtRuntime} = await import("next/dist/server/config-shared")
is working too. I think I do a try catch block and pass in a fallback component map if it fails so I need to make sure this isn't happening within the try block
11/23
OK, so I did some testing today and its magically working? I am not sure why, but looks like starting, then closing and restarting works? Going to push that change blitz g model test field1:number
blitz:config Loading config from D:\QuickDocs\blitz\examples\auth.blitz.config.compiled.js +405ms
✔ Model 'Test' created in schema.prisma:
model Test { id Int @id @default(autoincrement()) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt field1 Int }
I think it might make sense to add a debug type for codegen, so that we can debug things like what map was found, and what was used to generate a particular template
Adding a new field after first generation fails
Hm, this is a weird one, but after some debugging I think I know what happened. Basically, I don't think the escape characters function is working to fix the special characters issue, and when I worked on the fix, I was looking at the Form.tsx output which was working, but I was ignoring the Edit.tsx and other special character files because I wasn't escaping the line that checked whether to use an existing file or not. (It was basically outputting "IDENTICAL" instead of error, file not found) Before committing, I probably looked at that line and caught that and updated it, but didn't test it, so it looks like this is still TODO
Title: Blitzjs progress update 11/30/2021 TODO: After some consideration Brandon felt that codegen was going to be easier for upgrading users if it werent present, so we're going to remove that from the blitz.config.ts file and have it defaulted in code.
Next steps:
- [x] remove from generated code"
- [x] Make sure everything works like it did before"
- [x] New issue - make sure uuid type works and specifies the uuid decorator" (Aleksandra made sure the previous UUID PR continued to work in our new generator
Based on @Vandivier 's opened request, I realized that there is some additional default handling code that is needed. This is for two cases so far:
- unspecifed fieldType
blitz g all model field
doesn't pull a default component. Should we default in string in this case?
<Form<S> {...props}>
<__component__ name="field" label="Field" placeholder="Field" type="__inputType__" />
{/* template: <__component__ name="__fieldName__" label="__Field_Name__" placeholder="__Field_Name__" type="__inputType__" /> */}
</Form>
Logic for belongsTo must be specified, must intercept this "special name" before its processed as a template field. Right now this happens:
blitz g all model belongsTo:user
<Form<S> {...props}>
<__component__
name="belongsTo"
label="Belongs To"
placeholder="Belongs To"
type="__inputType__"
/>
{/* template: <__component__ name="__fieldName__" label="__Field_Name__" placeholder="__Field_Name__" type="__inputType__" /> */}
</Form>
- [x] Handle case when there are no field template values present
- [ ] Handle case for belongsTo field
- [x] Fix spacing issue in Form template. I fixed the issue that caused a space to be set in the mutations templates by modifying the regex to capture between newlines on the "//" template comments, might need to do the same for the {/* */} ones as well because now they are adding a space between generates
We're loading codegen a few times — I'm wondering if we can load it once somewhere top-level and pass it down as we do for other commands with loadConfigProduction
. E.g. we could pass it to generators that need it in generate.ts
file.
We're loading codegen a few times — I'm wondering if we can load it once somewhere top-level and pass it down as we do for other commands with
loadConfigProduction
. E.g. we could pass it to generators that need it ingenerate.ts
file.
Yeah, I initally was caching it in the builder, but I moved that logic into get-codegen, and I didn't know a good spot to add a cache variable since it wasn't a class. I also don't know how this is done best in Node, should we create a separate class to hold this or can we create a const config variable?
So I was doing some testing on the belongs to field to make sure it was similar to the one specified with the --parent flag, I don't see the model being generated with a User[] field, but if I use the belongsTo key, I do see those fields.
The field.ts file generates the schema values, so I'll take a look at that once I'm done with syncing up the generator behavior for the two
https://blitzjs.com/docs/cli-generate#options
Ok, so I broke my node_modules again. donig a global search while yarn dev is running always messes things up lol. I was able to update the parent zod type to use the new codegen utility, but will have to test it once the repo is re-installed.
So I was doing some testing on the belongs to field to make sure it was similar to the one specified with the --parent flag, I don't see the model being generated with a User[] field, but if I use the belongsTo key, I do see those fields.
The field.ts file generates the schema values, so I'll take a look at that once I'm done with syncing up the generator behavior for the two
https://blitzjs.com/docs/cli-generate#options
Ok, so I broke my node_modules again. donig a global search while yarn dev is running always messes things up lol. I was able to update the parent zod type to use the new codegen utility, but will have to test it once the repo is re-installed.
Let me know if you need support wrapping this up. This looks fun. I know there is the belongsTo field case and the todo from @beerose
Let me know if you need support wrapping this up. This looks fun. I know there is the belongsTo field case and the todo from @beerose
Thank you @imcodingideas! I'll fix the conflicts shortly. And if you'd want to test it briefly, we'd very much appreciate that!
I also think that the belongTo
thing can be covered later (unless it's a regression).
Yeah feel free to jump in! I have less time these days so am not be able to contribute as much as I used to :(((
Any updates on this? Will this be released in short terms? Or else, is there a prisma scaffold generator that anyone could advise. Kind regards
@Jarrodsz yeah we're trying to ship it very soon.
Ok, so I should have some free time now. I'm making a virtual machine and installing nvm as well so hopefully between the stability offered by nvm and the ability to take machine snapshots my npm installation should be stable.
I can handle the belongsTo flag in a couple days time I think
Ok, I spoke too soon. I'm having an issue within the virtual machine too. Going to troubleshoot that. It seems to be an issue with libsodium.
For anyone else on Windows, I fixed that by installing windows-build-tools (npm install -g windows-build-tools
). Also needed to install python separately because node-gyp wasn't detecting it in that PATH by itself
Ok, running into another error with the ncc_amp_optimizer package. yarn dev
runs, but doing blitz -v inside a folder doesn't work, blitz isn't recognized.
12/29 update: Moved the codegen key into the right spot since that doesn't require running the app to test
Aleksandra helped setting up a stable dev environment by recommending Volta and the following package versions:
"volta": {
"node": "14.18.1",
"yarn": "1.18.0",
"npm": "6.14.15"
},
I began working on the belongsTo:ModelName feature. I was processing special args in the field-values-builder, but a lot what I'm trying to do is done in packages\cli\src\commands\generate.ts
, around line 248:
const generators = generatorMap[args.type]
for (const GeneratorClass of generators) {
const generator = new GeneratorClass({
...
rawParentModelName: flags.parent,
parentModel: modelName(flags.parent),
parentModels: modelNames(flags.parent),
ParentModel: ModelName(flags.parent),
ParentModels: ModelNames(flags.parent),
I copied those into the field values generator for now, but we might want to make it a separate utility down the line
The prisma schema gets generated fine but the create model which is supposed to have the parent model id as a requirement doesn't work. The template file uses if (process.env.parentModel) {
to render the parentModelId field conditionally. I added process.env.parentModel = typeName
to field values builder but its still not being set, need to troubleshoot that next
01/19/2020:
Ok, I think I found out the reason for that issue.
In packages\generator\src\generator.ts line 224 we do templatedFile = this.replaceConditionals(inputStr, templateValues, prettierOptions || {})
This is where the conditional rendering is performed using the process.env.parent variable
Only after that is done, in line 241, do we perform template values replacement.
EDIT: I was looking at the wrong function on line 241. The call to modify process.env.parent happens in getTemplateValues(), which is actually called before the process() function, at const templateValues = await this.getTemplateValues()
so it should be working fine
~~My next steps are to see if I can create a utility to scan the arguments list and tack on parent to process.env before the call to this.replaceConditionals
~~
One thing to note that even if this is fixed, there are additional implications for repeated code generation using generated files. Generated files won't have conditional jsx rendering anymore E.g:
- Someone generates resources
- Runs another generate with --parent flag or belongsTo:modelname
- Parent fields won't appear because the existing generated file doesn't have the
if(process.env.xxx){ <conditional component />}
block