search

flowup / api-client-generator

Angular REST API client generator from Swagger YAML or JSON file with camel case settigs
MIT License
115 stars 21 forks source link

Fixes for array params. #38

Closed warmans closed 6 years ago

warmans commented 6 years ago

closes #31 (and #36?)

So as it turns out the code already had some support for array parameters it was just not working (isArray was not being set). So this MR just fixes that bug and changes set to append since set in the original code doesn't really make sense.

Also in cases where there are no external definitions/models (as with the example swagger file I added) then it now suppresses the import statement since it breaks the client.

Finally I've also just changed the paths in the package.json scripts to use the vendored binaries instead of global version since I guess that was the intention of vendoring them in the first place (unless npm is smart about checking for a local bin before checking for a global version?). yes it is.

One wierd thing is in fixing the IsArray property it seems to now generate any[] arrays instead of proper types. I'm not sure why... it'll still work but is a bit of a regression.

warmans commented 6 years ago

I don't know what's going on with the commits here. The only commit for this change is the last one. I guess the older ones will be ignored.

vmasek commented 6 years ago

Thank you for a PR, I started some work around this at the weekend because the problem layed much deeper (as you also mentioned). I have some fixes that are correcting the parameter types and might have conflicts with this.

I will push it and after that we'll solve the merge.

warmans commented 6 years ago

I've removed the new example I added since gcloud firestore already has some array parameters to test with.

vmasek commented 6 years ago

Almost all PR changes got covered by the #36 fix, however, take a closer look at the append in generated params/headers. I have looked at doc and headers support array as param, but in params, I've used a reduce. In my test app it had worked, so let me know if there will be any further problems.

warmans commented 6 years ago

Cool yeah looks like it. When do you think you'll tag a new release?

vmasek commented 6 years ago

Tomorrow it should be out, just need to finish #8 that will change passing parameters from flat to args object (it will allow API client to support optional parameters)

vmasek commented 6 years ago

@warmans I have a troubles publishing the package, I've written an email to an npm support. After that, it should publish a 3.0.x version to npm