controller icon indicating copy to clipboard operation
controller copied to clipboard

Fix README inaccuracies

Open cllns opened this issue 4 months ago • 0 comments

I used the README as a reference the other day and noticed it needed to be fixed. As I looked further, basically every section needed to be fixed or improved. It took a while to go through everything but I'm glad I did and we can have an accurate README. I went beyond just fixing the bare minimum and improved things as I went along. I found some bugs / potential improvements too. There's a good number of TODO's below, but I'd love feedback (and also check my work! I'm sure I made mistakes).

Summary

The biggest, nearly ubiquitous, fix was removing injected configuration objects, since we don't do that anymore.

The other major change was to make all code examples self-contained. You should be able to copy and paste them directly into IRB and they should "Just Work". This means adding some extra irrelevant boilerplate (e.g. ArticleRepo and Article), but I think we can trust readers to ignore those.

Minor changes:

  • Changed ArticleRepository to ArticleRepo, to match our naming convention in hanami-db. We don't interact with the db layer at all here, of course, but it's good to be consistent to minimize confusion.
  • We had Rack array responses, e.g. # => [200, {}, ["body"] in many places but that's inaccurate. We get back Hanami::Action::Response now. So users need to call #to_a (alias for #finish) to get the Rack-compatible array response. I started doing this throughout but I realized it could be broken up into several different lines for clarity, to only show the relevant info (via #status, #headers, #body, etc). This is more clear but also the headers used to be {} by default, and now they have Content-Type and Content-Length headers so it was very noisy (or had to be abbreviated as ... which isn't valid Ruby here).
  • Standardized on p instead of puts. I was using both, depending on the situation, but it seemed distracting. The main issue is that puts always returns nil, so #=> 123 was describing the printed value instead of the return value. I'm open to changing this, I just wanted to be consistent.

TODO

  • Structure

    • [ ] Restructure order to surface most relevant info first, rewrite accordingly
    • [ ] Add table-of-contents
    • [ ] Rewrite intro and section copy. I don't think hanami-controller can be considered a "micro library" anymore
  • Rack

    • [ ] Add note about supporting Rack 2 and Rack 3 (and rewindable input?)
    • [ ] Add relevant info about how Rack works and how this library builds on top of it
    • [ ] Add streaming request example
  • Formats

    • [ ] Add note about surprising behavior with config.formats.add
    • [ ] Add note about default format, including charsets
    • [ ] Add note about default Content-Type and Content-Length headers
  • Other:

    • [ ] Add note about using symbols for statuses, i.e. :unauthorized work as well as 401
    • [x] Change requires to require "hanami/action"? Slightly confusing but we can add a note about it.
    • [ ] Add note about how sessions work in rack
    • [ ] Do a final manual check of every example and section

Notes:

  • I removed the streaming example because it didn't work and AFAICT we don't actually support streaming responses right now
  • I removed a section about inheritance and error handling. There's nothing surprising about it, we can assume readers know how inheritance works in Ruby

Bugs / Potential improvements:

I plan to open issues for all of these. I'll replace them with links to the Issues when I do that.

  • [ ] request.cookies only allow string key access (and it's a plain ::Hash). request.session automatically coerces keys to symbols, and can only be accessed via symbols only. We should rationalize these somehow, it's confusing they're so different.
  • [ ] Fix surprising behavior with config.formats.add?
  • [ ] Using expires 600, :public, max_age: 123 adds two max-age values, e.g. Expires header is public, max-age=123, max-age=600. Maybe we can deprecate having both? I think I'd prefer max_age as a kwarg only and removing the first arg option.
  • [ ] Reading cookies works fine without include Hanami::Action::Cookies (so I removed it from that example). However, setting cookies does need that include, else you get a 'Hash#fetch': key not found: "Set-Cookie" (KeyError)
  • [ ] Setting max_age on a cookie automatically computes an Expires as well. Using the max_age setting from config does not set the Expires (you can see this in the SetCookies example)
  • [ ] Add in Streaming Body support, as defined in the Rack 3 spec: https://github.com/rack/rack/blob/main/SPEC.rdoc#streaming-body-. Ensure we support Enumerable bodies as specified too.
  • [ ] Open issue outlining a proposed backward-compatible transition to hanami-action as the gem name

cllns avatar Sep 19 '25 23:09 cllns