TypeScript code generation fixes and type updates #33
Conversation
|
@Sunjammer thanks for the PR! I probably won't get time until the weekend to give it the once over and test it out. Will report back then when I do 👍 |
| status: number;; | ||
| code: number;; | ||
| status: number = -1;; | ||
| code: number = -1;; |
There was a problem hiding this comment.
Just a quick note on this one. Changing these to -1 defaults I'm hesitant to do as it would be classed as a breaking change so require a major bump. Also some people may genuinely have a -1 error code from the server (I know unlikely).
Let me know your thinking though, perhaps theres a good case I'm missing.
There was a problem hiding this comment.
This can be fixed in a different way given that the class has a constructor that assigns these mandatory fields, or that the methods that construct the objects never leave any of them undefined, but it's either this or flagging status and code as optional. I personally detest optional fields so my preferred way would probably be to rewrite the type use... What thinkest thou?
There was a problem hiding this comment.
Was TS complaining about these not having a value assigned?
There was a problem hiding this comment.
Yes, as is openapi-client is not compatible with strict mode projects
| return '[]' | ||
| default: | ||
| return 'undefined' | ||
| } |
There was a problem hiding this comment.
Looking at this it appears to be assigning defaults to empty optional params, which may rightly have no value. This would unfortunately also be breaking (also looks like it would swap 0 for -1).
undefined for all of these is a valid default. If a param should have a true default then that should be defined in the open api spec.
Let me know if i'm overlooking something.
| @@ -1,6 +1,6 @@ | |||
| { | |||
| "name": "openapi-client", | |||
| "version": "1.0.1", | |||
| "version": "1.0.2", | |||
There was a problem hiding this comment.
version gets auto bumped when a release is made off master.
|
Hello! I just discovered this great project, but like @Sunjammer, I prefer to use Typescript with its strict mode enabled, and the currently-generated code doesn't work with that. I think there is probably a way to let it pass without breaking backwards compatibility, though an alternative would be to break it in favor of providing more user-friendly typings (although I literally just started using this project an hour ago, it seems like there might be some opportunities for improvement, but I could be wrong). Anyways, I'd be happy to "pick up the torch" on this PR if @Sunjammer's unavailable, or I am happy to help move it along in whatever other way might be useful, just let me know. Thanks again for creating this tool, @mikestead! |
|
@toolness thanks for your offer, I'm very open to this. I'd really prefer to not break backwards compatibility if we can help it as there are some large sites with this in production. That said, if there's a fairly painless upgrade path that respects the openapi 2 spec then I'd consider a major bump. |
This PR fixes some generated types that are invalid in strict mode TS, adds appropriate type assertions, narrowly improves assignment of optional params according to their type and maps 'file' param types to 'File' objects, which may or may not be correct :-P
Open for criticism/style change requests etc. I'm new to OpenAPI so I expect there to be unforeseen things or misunderstandings. Here to help!