-
-
Notifications
You must be signed in to change notification settings - Fork 120
Make config.formats clearer and more flexible #485
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
Conversation
|
Sounds great to me! I love that we can add a new way of doing it without breaking existing API. What do you think about potentially deprecating |
|
@cllns thanks for checking this out, glad you're happy with the direction. I actually thought of deprecating immediately too! For a framework such as ours, I think we can take some liberties with moving a little faster (especially in this case where we have an easy mechanical change to replace to deprecated API), so with your encouragement I'll go ahead and just deprecate. |
|
I think this should help clear up a LOT of confusion new users might feel 🙌! Great work on this. Can't wait to give this a try on my apps~.
Interesting, would the idea be to default apps apps to
I think deprecating the previous, confusing behavior is a win too. |
|
Added the deprecation notices (using Ruby's built-in |
|
Guides PR: hanami/guides#306 |
|
And changes on Hanami's side: hanami/hanami#1544 |
|
Just an update on this, I'm making another round of changes so that formats can be registered with separately configured media_types (checked in the This should address the issue of the I'll leave another update here once this is done. |
3d5100c to
f632ce7
Compare
request methods -> response methods -> utility methods
Make them consistent with the enforce_accept methods
|
OK, I've made a few more changes to this to get the new behaviour working just so. I've updated the top-level PR description to reflect the latest changes, but I'll echo that here as well, for anyone following the PR:
I've also changed all references to "MIME types" to "media types", since that is the modern term for these. The only places I've left "MIME" in place is for our own |
12b8cad to
0535c43
Compare
This documents the behavioural change introduced in hanami/hanami-controller#485.
This documents the behavioural change introduced in hanami/hanami-controller#485.
Provide new methods on the
config.formatsobject that make it more flexible and easier to work with.These are all new methods, intended to become the recommended usage of
config.formatsfrom Hanami 2.3 onwards.For the purposes of compatibility, the existing methods remain in place, but will no longer be documented, and we will include notes on how to switch to the new methods in the upgrade guide. In a future release, these older methods will emit deprecation warnings, and will eventually be removed.
Fixes hanami/hanami#1396
Add
formats.register, with types for distinct purposesformats.registerreplacesformats.add. Its purpose is to register format to media type mappings only. Unlike.add, it does not set any of the given formats as being “accepted” by the action (see next section for the new way to do this).This change makes it easier to
registera range of formats e.g. in a base action class or in the app-levelconfig.actions.formats, while ensuring you do not cause any inadvertent format restrictions in child action classes.Example:
.registeralso allows you to register one or more media types for the distinct stages of request processing.Acceptrequest headers, provide them asaccept_types:Content-Typerequest headers, provide them ascontent_types:Content-Typeresponse header for requests that match the formatExample:
Add
formats.acceptedandformats.acceptformats.acceptedreplacesformats.values. This name better reflects its purpose: it is the array of formats that the action will accept when handling requests.To populate this array, you can still manually assign it, e.g.
formats.accepted = [:json]as you could do previously with#values=.However, the nicer (and recommended) way to do this is via
config.accept, which takes a splat of format names. Example:This method is also a replacement for the higher-level
config.formatmethod that we currently have documented. In making these changes, I decided that it's better to teach users about the existence of theconfig.formatsobject and its own methods rather than trying to hide it away behind a single (and therefore inflexible) convenience method.Add
formats.default=This is a brand new capability: you can now assign a default format via e.g.
formats.default = :json.The default format is used to set the default
Content-Typeresponse header if one of the configured accepted formats cannot be matched to the request'sAcceptheader.Previously, the default format was set to be the first of the formats that the
config.formatsbecomes aware of, either via runningconfig.formats.addorconfig.formatorconfig.formats.values=. This was a bad arrangement because the setting of the default was implicit. It was not obvious from any of the methods being called and led to unexpected surprises. It could also not easily be overridden, which made things awkward in situations where action config is inherited from parent classes or app-level config.Now we have a simple setter that allows the default format to be configured independently.
I should note that I have preserved the automatic setting of a default when calling
formats.accept, but only if theformats.defaultis not already set with a non-nil value. This way we hopefully have the best of both worlds: a one-liner when someone wants it, but the ability to explicitly and separately configure the default format when that is required.Next steps
I'm going to merge this and include this in 2.3.0.beta2, which will release soon.
In the meantime, I would also love to hear feedback from anyone who shared their experiences in #1396.
Perhaps separate to this PR, we should decide whether we want to make
config.formats.accepted # => [:html]andconfig.formats.default # => :htmlthe standard setup when inside full Hanami apps.As a follow-up after merging this, I'll also begin an upgrade notes page in our guide to describe the changes above.