-
Notifications
You must be signed in to change notification settings - Fork 648
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
Support for rails 6 #869
Support for rails 6 #869
Conversation
46d2fdb
to
cc3774f
Compare
cc3774f
to
cd1c92f
Compare
Fixed CI tests (only rails-edge (6.1 alpha) left because of sprockets' 4 newline, but that's separate story) |
@Vasfed works for me, thanks! |
@Vasfed Awesome. Thanks for this, and also for tracking down the sprockets newline issue that has been plaguing the test suite. |
Is there a release planned for this? I'm getting annoyed by the Deprecation Warnings 😉 |
@unixmonkey I've created a PR myself for a new version: #881 |
This is now part of the gem in version 2.0. Thanks so much for the help on this! 🙇 |
ActionController::Base.send :prepend, PdfHelper | ||
else | ||
ActionController::Base.send :include, PdfHelper | ||
ActiveSupport.on_load(:action_controller) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this new approach break the compatibility of wicked_pdf and remotipart? I do observe a bug if wicked_pdf 2.0.x is used with remotipart. In Ruby 2.6, the bug is announced as a FrozenError. We get more information under Ruby <2.5 to see that there is indeed a loop of rendering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm this does break our app. I forked this repo and simply rolled back to a simpler version of railtie.rb
just to debug this and we are working again darrenterhune@ccc8615 that said, wicked_pdf still worked locally, it was just barfing on CircleCI with this #891
There is a Rails 6 deprecation warning very common in some gems: rails/rails#36546. In wicked_pdf this issue is solved in the most recent versions mileszs/wicked_pdf#869
There is a Rails 6 deprecation warning very common in some gems: rails/rails#36546. In wicked_pdf this issue is solved in the most recent versions mileszs/wicked_pdf#869
There is a Rails 6 deprecation warning very common in some gems: rails/rails#36546. In wicked_pdf this issue is solved in the most recent versions mileszs/wicked_pdf#869
There is a Rails 6 deprecation warning very common in some gems: rails/rails#36546. In wicked_pdf this issue is solved in the most recent versions mileszs/wicked_pdf#869
There is a Rails 6 deprecation warning very common in some gems: rails/rails#36546. In wicked_pdf this issue is solved in the most recent versions mileszs/wicked_pdf#869
In rails 6 wicked_pdf initializer causes
because of early access to
ActionController::Base
and can be fixed by lazy loading.This PR fixes it by using
ActiveSupport.on_load
(supported since rails 3)Also made tests pass on ruby 2.6 and fresh gem versions (had to lock sprockets to 3 because of changed newline handling)