-
Notifications
You must be signed in to change notification settings - Fork 440
Fix regression in API controllers with view_cache_dependencies helper #575
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
base: main
Are you sure you want to change the base?
Conversation
fdf4510
to
fa54436
Compare
I ran into this issue as well, but on rails 7.2.2. (I believe the API controller gained the I found I needed to add the |
Yeah, perhaps this PR should include |
66add2d
to
b928550
Compare
Added that helper. 👍 Now that Rails 8 has shipped, I imagine more people will run into this. cc @rafaelfranca Failing test is because Ruby 3.1 isn't support on Rails head. |
Yes, same issue here running Rails 8.0 too, so far I'm using @excid3 branch and works, waiting for the merge. |
@rafaelfranca apologies to re-ping but any chance this could be reviewed? It's blocking upgrades to |
+1 here, having the same issue on rails 7.2.2, ruby 3.3, jbuilder 2.13.0 applied the fix manually in my application_controller class ApplicationController < ActionController::API
helper_method :combined_fragment_cache_key
helper_method :view_cache_dependencies
...
end |
+1 here, rails 8.0.2, ruby 3.2.1, jbuilder 2.13.0 Thanks @palin, your fix worked perfectly! |
Rails 8's API controllers now includes the
Caching
module by default (to supportrate_limit
in API controllers), but does not include the Helpers module.This causes the jbuilder
cache
method to raise an exception because the Rails caching module does not add thehelper_method :view_cache_dependencies
since that's only added ifhelper_method
exists.Example error:
This adds the helper method to jbuilder to ensure that caching still works without having to include Helpers in API controllers.
Previously, this worked because you could include both the Helpers and Caching modules in API controllers in the correct order.
jbuilder doesn't have a test/dummy app to write tests for this.