Kononnable / typeorm-model-generator

Generates models for TypeORM from existing database.
MIT License
1.51k stars 281 forks source link

Add custom decorators (as extra templates) #229

Open boenrobot opened 4 years ago

boenrobot commented 4 years ago

Right now, if I want to add anything to an entity that requires a decorator, I have to either edit the generated result, or inherit from the entity, only to redefine all affected properties.

This is not at all scalable. I think a better approach might be to enable the ability of custom additional handlerbars templates that would receive the item (entity, column, relation) being processed, and have their output inserted at the assigned spot (one before the generated decorators, one afer the generated decorators, one after the property/class)

That would allow custom decorators, as well as comments or even additional classes (instead of entity decorators), additional properties (instead of property decorators). There could be default empty templates, used when ones are not provided on the command line.

It may also be good to pass the handlebars instance to a file (similarly to the naming strategy), to allow definitions of additional handlebar helpers.

Use cases in point: nestjs CRUD, swagger and class validator. It would be much neater to define validation rules in such custom templates that would look at what the target entity/column is, and output the expected validation decorators. In my case, some of my validation rules can be applied based on the column options (e.g. max length, whether it is required, range of numeric columns, etc.), and others by my own convention (e.g. if I have DATE, DATETIME, INT, FLOAT or DECIMAL columns ending in "from" and "to" in the same table, then "from" must not be greater than "to", and "to" must not be less than "from"), which would make the DB model significantly scalable for such typical cases.

On an even more superficial level, adding TypeORM's @CreateDateColumn(), @UpdateDateColumn() and @VersionColumn decorators would also become possible, and be more flexible than an option about them.

Kononnable commented 4 years ago

Sorry for the delay.

There is a reason for not allowing users to change handlebars template as a parameter - versioning. Overriding default template would break with new version of the package. Even renaming an internal field could cause an issue which would be hard to find sometimes(e.g. if such field is rarely used). For usecases when overriding template file I would go with two workflows described in USECASES.md:

Both of them will produce predictable results when typeorm-model-generator is updated. First one is much easier to implement, but may require manual fixes on every merge conflict.

boenrobot commented 4 years ago

Isn't the same true for a naming strategy though?

If the key difference is type safety, maybe switch the template engine to this one: https://www.npmjs.com/package/typesafe-templates

Either way, doing generation with a separate tool would be much harder, as one needs to re-parse the code, alter it, and re-serialize it. Sure, there's "schematics", but that one is a huge PITA to use.

Kononnable commented 4 years ago

Implementation of naming strategy doesn't really change over time. 0.4 will introduce first breaking change in naming strategy.

Thea issue is much larger than just type safety. As an example we can use last change which was introduced to template file: https://github.com/Kononnable/typeorm-model-generator/commit/f701f6974f1ab92e28e6b892a8abdf4fc2c9c51a#diff-2ecb5703231ff856d4c87fc5b40ba41d In order to fix a bug there were two new properties(joinColumn, inverseJoinColumn) added which were never used before in template file. If someone uses theirs own template file they wouldn't really get this fix - data will be there but it won't be written to output file. You can't really avoid such issues unless you get something to shout at you(merge conflict from first workflow) or your code is completely independent of typeorm-model-generator code(second workflow). Depending on your needs both of the solutions might be good fit(sometimes simple string replacement is the key).

boenrobot commented 4 years ago

But I'm talking about additional "child" templates... "partials" (see https://handlebarsjs.com/api-reference/runtime.html#handlebars-registerpartial-name-partial ), that add additional data in the output, not ones replacing the existing template.

In such a setup, a fix to the main template would still apply if custom partials are used. Partials simply won't take advantage of fixes for their own output.

Kononnable commented 4 years ago

Ok, to be sure that I understand you correctly: You propose to introduce template placeholders which will enable users to introduce theirs code there?

Something like:

{{beforePropertyDecorator}}{{#generated}}@PrimaryGeneratedColumn({ type:"{{type}}", {{/generated}}{{^generated}}@Column("{{type}}",{ {{#primary}}primary:{{primary}},{{/primary}}{{/generated}}{{json options}}{{#default}},default: {{.}},{{/default}} })
{{beforePropertyVisibility}}{{printPropertyVisibility}}{{toPropertyName tscName}}{{strictMode}}:{{tscType}};

where beforePropertyDecorator,beforePropertyVisibility are not used by typeorm-model-generator.

boenrobot commented 4 years ago

Exactly.

typeorm-model-generator may have empty files to appease handlerbars, but would not itself use those partials for any "actual" output. Any changes would be made into the template, not the partials.

Kononnable commented 4 years ago

Ok, but where such partial templates should be put? There are a lot of places which someone might want to extend - e.g. someone might want to add a decorator of some sorts, other use cases would be to implement some method.

boenrobot commented 4 years ago

I think a sufficient list of partials inside the main template would be

Or alternatively, there could be default partials, and custom ones aliased to the default ones by default. A partial wishing to only extend will call the default inside it, and add whatever it needs to before or after. In that scenario, the templates needed would be

And of course, a default* for each of the above, to be called by extenders.

In this second scenario, you're allowing the extender to opt out of the typeorm-model-generator behaviour by not invoking the default partials inside of them.

If you think that's unacceptable, the more complete list would work to allow extensions, while also preventing extenders to opt out or replace the default template behaviour.

Personally, I would prefer the second approach (despite this meaning my custom partials would be more fragile across versions), but all of my use cases would be covered by the first, so that works too.

joshterrill commented 3 years ago

I needed the ability to create custom templates, so I added it in this forked repo. I can submit a PR if the maintainer would like me to. @Kononnable if not, that's fine too. https://github.com/joshterrill/typeorm-model-generator

Kononnable commented 3 years ago

Sorry for the delay. Copying and modifying template file is fine as long as same version of typeorm-model-generator is used. If someone updates package version and use old template file things might get very messy. That's why I don't think it would be a good idea to add such feature in its current form - it can do harm after some time. I thought of adding some version control to template file, but it isn't as easy as it might be seem(I prefer not to change version number manually - such things can be forgotten easily.