orval/core - generating factory method for schema interfaces #1334
Conversation
mironbalcerzak
force-pushed
the
feature/factory-method
branch
from
78b0a4e to
581b773
Compare
| } else { | ||
| model += `export type ${name} = ${scalar.value};\n`; | ||
| } | ||
| model += `export function create${name}(): ${name} ${scalar.factoryMethodValue}\n`; |
There was a problem hiding this comment.
You will add this in the model file here. It's a bit weird no?
There was a problem hiding this comment.
Yeah... was thinking whether it should be in separate file or not. I've decided to leave it with model. Anyone who has imported a model, can either instantiate it through "new", or right away call a factory method - which increases the chances that random dev will learn about it and use it.
People are lazy and do not read the code - especially the one that's generated.
|
Hi, @mironbalcerzak |
mironbalcerzak
force-pushed
the
feature/factory-method
branch
4 times, most recently
from
64df5ba to
76f3ec5
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
from
76f3ec5 to
9e5e347
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
from
9e5e347 to
32e6fab
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
2 times, most recently
from
0fb9a7d to
27dbc05
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
from
27dbc05 to
e68e30d
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
from
e68e30d to
5e5ece4
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
2 times, most recently
from
e0fde50 to
b77a14c
Compare
|
@soartec-lab - ive been using this branch in few projects of mine over last months - seems ok. |
|
It looks like the tests are failing? |
|
@melloware, yeah, i am aware and that will be fixed shortly - i meant on a broader scale - should i implement/change something in PR? |
|
i think once its fixed and passing we will re-review. |
|
@mironbalcerzak |
soartec-lab
left a comment
soartec-lab
left a comment
There was a problem hiding this comment.
@mironbalcerzak
I made some comments 👍
| } | ||
|
|
||
| return `import ${!values ? 'type ' : ''}{ ${name}${ | ||
| return `import { create${name}, ${name}${ |
There was a problem hiding this comment.
Since create exists in the http status, I want to change the function name to the new prefix to avoid confusion.
| return `import { create${name}, ${name}${ | |
| return `import { new${name}, ${name}${ |
| } else { | ||
| model += `export type ${name} = ${scalar.value};\n`; | ||
| } | ||
| model += `export function create${name}(): ${name} ${scalar.factoryMethodValue}\n`; |
There was a problem hiding this comment.
Factory methods are just noise for people who don't need them, so I'd like to be able to choose whether to generate them with a property.
For example, generateFactoryMethod: boolean.
And by making it a property, people who want to use it will be aware that it will be generated. Therefore, you will be able to notice it even if you separate the file like .factory.ts, right?
mironbalcerzak
force-pushed
the
feature/factory-method
branch
5 times, most recently
from
2cc176f to
c9e3bb3
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
4 times, most recently
from
c20568d to
91101bf
Compare
mironbalcerzak
force-pushed
the
feature/factory-method
branch
from
91101bf to
45f4d4d
Compare
|
Where are we at with this PR? |
|
@melloware - sorry for late reply, very hectic time. keep it open if you may, i am slowly working on it to make it good enough and eventually i will notify you guys that it's ready for proper review / merge |
|
No problem. |
4 participants
Status
WIP
Description
Fix #1331
This PR is providing simple implementation.
Opening for discussion.
Bugs? Possible - definitely "stackoverflow" in case of self referencing or cycle
Todos