fix: Support Rails Engines #42
Conversation
arkirchner
commented
Sep 11, 2025
arkirchner
commented
- RFC6570 paths need to include the Rails Engines prefix.
- Route parameter support keyword arguments and an options Hash.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 4 4
Lines 162 161 -1
=========================================
- Hits 162 161 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Does the engine need to be a gem? This feels like adding a lot of unrelated churn.
If the engine is only needed in tests, can it be defined/loaded only there?
lib/rails/rfc6570.rb
Outdated
| end | ||
|
|
||
| define_method(rfc6570_path_name) do |**opts| | ||
| define_method(rfc6570_path_name) do |opts = {}| |
There was a problem hiding this comment.
- Support for options hash for
*_rfc6570to support Rails Engines.
When used in a Rails engine, the options are a hash. I was able to reproduce this issue loading the engine; however, this seems to no longer be the case.
I am investigating.
lib/rails/rfc6570/formatter.rb
Outdated
| options = ctx.url_options.merge(kwargs) | ||
| options[:path] = parts.join | ||
| options[:only_path] = kwargs.fetch(:path_only, false) | ||
|
|
||
| ::Addressable::Template.new \ | ||
| ActionDispatch::Http::URL.url_for(options) | ||
| if (osn = options.delete(:original_script_name)) | ||
| options[:script_name] = osn + options[:script_name] | ||
| end | ||
|
|
||
| ::Addressable::Template.new \ | ||
| ActionDispatch::Http::URL.url_for(options) |
There was a problem hiding this comment.
This seems to be the primary change here, but it is very similar to before.
What's the important change here? It handles path_only: true much more like a full URL? Is that the important change for supporting engines?
There was a problem hiding this comment.
Yes, the original version removed the engines mount path.
/some_engine/users was resolved to just /users.
There was a problem hiding this comment.
Fix up
*_path_rfc6570to include Rails Engines mount point.
Basically the previous version would ignore script_name for paths.
There was a problem hiding this comment.
Thank you for contributing!
I've added some notes and would need some more clarification about which specific changes do what. I do need to understand the changes and intentions so that I can continue to maintain that.
Please add them to the commit(s) and PR description so that they are preserved.
|
I inlined the engine and squashed all commits but couldn't force-push correctly into this PR. I've put your changes on the pr-42 branch and will merge them. |
|
Thank you! |
|
Has been released with |
2 participants
