Reitit 1.0.0 - Githubissues

ikitommi commented 5 years ago

This issue will contain some of the things that should be changed, but would be breaking changes. We could collect them for 1.0.0, which could out soon:

Clear component lifecycle

All components (middleware, interceptors, option implementation) should be constructed separately, so there is clear separation of when things happen. Currently all interceptors work like this, e.g. (coercion/coercion-interceptor) but most middleware don't coercion/coercion-middleware. This is kinda bad as there are shared components like swagger/swagger-feature, which one needs to read the source to see how it is defined. Currently, because of this, some things fail at runtime, which is really bad.

[;; swagger feature
 swagger/swagger-feature ;; <-- eh.
 ;; query-params & form-params
 (parameters/parameters-interceptor)
 ;; content-negotiation
 (muuntaja/format-negotiate-interceptor)
 ;; encoding response body
 (muuntaja/format-response-interceptor)
 ;; exception handling
 (exception/exception-interceptor)
 ;; decoding request body
 (muuntaja/format-request-interceptor)
 ;; coercing response bodys
 (coercion/coerce-response-interceptor)
 ;; coercing request parameters
 (coercion/coerce-request-interceptor)
 ;; multipart
 (multipart/multipart-interceptor)]

Currently causing also issues like https://github.com/metosin/reitit/issues/205.

This could be applied to all router options too, e.g. :validate spec/validate => :validate (spec/validator)?

Also, having a separate constructor allows specs to be attached to them, effectively validating all component creation at router creation time. This is good.

ctrl-merge

meta-merge currently doesn't allow overriding primitive values. This is bad, as one can't for example dissoc :muuntaja value. There was a good discussion in #leiningen slack about this, would need meta-merge 2.0 or a ctrl-merge inlined in reitit. Would change how route data can be modelled, e.g. have a default :muuntaja value. Would definetely BREAKING (for the better?)

Sieppari

pushing Sieppari out of alpha will introduce some breaking changes, that could be bundled here. One big thing is is mapping functions to :enter.

Coercion

should be a common way to to configure which keys are present in the error response
all parameter errors should be reported, not just the one that fails

Others

consistent packaging
- no plurals, e.g. reitit.http.interceptors.* => reitit.http.interceptor.*
request & context diffing
- rename reitit.ring.middleware.dev/print-request-diffs as it also diffs responses
- should be in the reitit-dev module as they are not meant for production (deep diff + pretty printing is really slow).

How to break

New package and/or ns? Just break it? We broke mostly everything with compojure-api 1.0.0, but there are also thoughs on growing things.

valerauko commented 5 years ago

New package and/or ns? Just break it?

I say just break it. Semantic versioning is there for a reason, and with a major version upgrade, reasonable breakage is to be expected

esuomi commented 5 years ago

@valerauko Reitit doesn't use SemVer but Break Versioning instead... :) In this case that doesn't matter in practice though, but technically it could.

miikka commented 5 years ago

A quick note: for the var renamings (print-request-diffs), we could do right now what the growing things post suggests: let's rename the var and add a ^:deprecated alias for the old thing. If we want, we can later remove the deprecated aliases as a breaking change.

metosin / reitit