Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swagger with spec record (cljs support) #130

Merged
merged 2 commits into from
Oct 21, 2018
Merged

Swagger with spec record (cljs support) #130

merged 2 commits into from
Oct 21, 2018

Conversation

nenadalm
Copy link
Contributor

Hi. I was playing with reitit and Macchiato (macchiato-framework/macchiato-core#7) but I am not sure how to generate spec. I've tried to follow guide for ring (https://metosin.github.io/reitit/ring/swagger.html).

It looks like reitit is converting given spec in :parameters into spec records and then I get following error in cljs:

#object[Error Error: No protocol method IKVReduce.-kv-reduce defined for type : [object Object]]

it comes from this function:

(defn expand-qualified-keywords [x options]

It seems to sort of work for clj, the result is just a bit different with spec records in the test (added test is just copy-paste of the test above, just with converted spec to spec records). Spec record is missing :title key in body.schema.

@nenadalm
Copy link
Contributor Author

It looks like the problem is in clojurescript, because records don't implement the protocol: https://dev.clojure.org/jira/browse/CLJS-1297

@nenadalm nenadalm changed the title Swagger test with spec record (failing test) Swagger with spec record (cljs support) Sep 19, 2018
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 97.432% when pulling 963ba9a on nenadalm:swagger-issue into cb4b448 on metosin:master.

:description "",
:required true,
:schema {:type "object",
;; :title "spec-tools.swagger.core-test/address",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? If spec is used instead of spec records, the :title is present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the title should be present in both.

@ikitommi ikitommi merged commit bfecc81 into metosin:master Oct 21, 2018
@ikitommi
Copy link
Member

Thanks, will fix the title and push out a release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants