domkm / silk

Routing for Clojure & ClojureScript
222 stars 13 forks source link

ring-handler clobbers :params #21

Open patrickthebold opened 8 years ago

patrickthebold commented 8 years ago

It seems that if one uses the standard ring middle-ware 'wrap-params', that map gets lost when using ring-handler. I was thinking we could either merge the :params map provided by wrap-params with the one provided by silk, or put the silk params under a different key. Thoughts?

Deraen commented 8 years ago

The usual solution is to merge params to :params and add them under another key, e.g. :path-params, :query-params.

domkm commented 8 years ago

Silk can extract params from the query. Are you sure you need wrap-params? wrap-params can also extract form params but that is available under the form-params which is not clobbered by Silk. If you need wrap-params, I think you should follow @Deraen's suggestion, but I would first evaluate whether you really need wrap-params.

patrickthebold commented 8 years ago

I'm using sente which requires wrap-params and wrap-keyword-params. Of course, I can work around it, but I think the "right thing to do" would be if silk didn't clobber the params. That way it would play nicer with other libraries. Would you accept a PR that merges the :params and adds the silk ones under, say, :route-params?

Deraen commented 8 years ago

Btw. wrap-params doesn't only read query string parameters but also form parameters from request body, so it's quite common to use it.

domkm commented 8 years ago

Thanks for the clarification. I'd accept a PR that merges Silk params into the request params instead of displacing them. :)