coexistence with wicked_pdf

[ocarreterom]

Steps to reproduce

Add wicked_pdf to your Gemfile.

gem 'wicked_pdf'

Expected behavior

Work well.

Actual behavior

I get an error:

20:07:18 web.1   |  Completed 500 Internal Server Error in 425ms (ActiveRecord: 48.6ms | Allocations: 173487)
20:07:18 web.1   |  FrozenError (can't modify frozen fatal)

System configuration

Rails version: 6.0.2.2

Ruby version: 2.6.5

Gem version: 2.2.1

[joelhawksley]

@ocarreterom thank you for the report!

This was also being discussed here: #257

Do you have a proposed solution in mind? Unfortunately the conflict is due to competing monkey patches of render.

I could see having an option to turn off the ViewComponent monkey patch, potentially allowing some sort of alternate render method such as render_component. What do you think @jonspalmer?

[ocarreterom]

This monkey patch is due to the backport and it might will be fixed in Rails 6.1?

[ocarreterom]

Also, render_component looks good to me.

[joelhawksley]

@ocarreterom correct. If you're running 6.1, the monkey patch isn't used.

[jonspalmer]

@joelhawksley Providing a view helper like render_component and then a config option to prevent the monkey patch seems very sensible and simple to implement. I'd agree that seems like the simplest approach.

I do wonder if we implemented the monkey patch like wicker_pdf could they coexist: https://github.com/mileszs/wicked_pdf/blob/08a104c2115147410b8008079cba08c5d8a5f226/lib/wicked_pdf/pdf_helper.rb#L37

There is explicit support there for alias_chaining there. We could try that pattern. If it worked we'd have zero config compatibility. While Rails 6.1 isn't going to have this problem I wonder if we should make the effort to build it "the right way" for older Rails versions.

[joelhawksley]

@jonspalmer oh, interesting. It would certainly be worth a shot!

@ocarreterom would you be interested in giving that approach a try? Our CI builds against Rails 5, 6, and master in a matrix, so we should be able to be fairly confident in test feedback here.

[jonspalmer]

@ocarreterom I have a spike of this approach going...stay tuned.

[jonspalmer]

Actually scratch that. As I dig into the wicked_pdf code it's functionally equivalent to the code we have already given we're prepending the monkey patch. For some reason wicked_pdf is also trying to handle the included case as well as prepend. I don't think we care about that.

So I think we're back to our original idea. That said I'd love to know why this breaks in the first place. Is it view_component doing something unexpected or is it wicked_pdf?

[spencersteiner]

One hacky workaround is to use the render_with_wicked_pdf method included with wicked_pdf (instead of using render) and remove the render method in the wicked_pdf initializer:

class WickedPdf
  module PdfHelper
    remove_method(:render)
  end
end

Obviously not the most desirable workaround haha, but it will allow the two to exist together until a more legitimate solution exists (I've been experimenting with this in my rails 5.2 environment where it is currently functioning)

[johannesengl]

That said I'd love to know why this breaks in the first place. Is it view_component doing something unexpected or is it wicked_pdf?

@jonspalmer
I think the issue is at WickedPdf::PdfHelper#L60:
https://github.com/mileszs/wicked_pdf/blob/08a104c2115147410b8008079cba08c5d8a5f226/lib/wicked_pdf/pdf_helper.rb#L60

Incase ViewComponent and WickedPdf is included method(:render).super_method actually references the monkey patch render method on WickedPdf::PdfHelper#L29: https://github.com/mileszs/wicked_pdf/blob/08a104c2115147410b8008079cba08c5d8a5f226/lib/wicked_pdf/pdf_helper.rb#L29

method(:render).super_method
=> #<Method: WickedPdf::PdfHelper#render(options=..., *args, &block) /Users/johannesengl/.rvm/gems/ruby-2.7.0/gems/wicked_pdf-2.0.2/lib/wicked_pdf/pdf_helper.rb:29>

This then causes an infinity loop.

Incase only WickedPdf is included I get the following:

method(:render).super_method
=> #<Method: ActionController::Instrumentation#render(*args) /Users/johannesengl/.rvm/gems/ruby-2.7.0/gems/actionpack-6.0.3.1/lib/action_controller/metal/instrumentation.rb:41>

[johannesengl]

I created the following draft PR: #358
Could someone please review and see if this goes into a good direction. Is there any specific person I should request a PR from?

[fsateler]

I believe wicked_pdf is doing magic it doesn't need, so I posted mileszs/wicked_pdf#925 removing it, replacing with a patch very similar to view_component

[johannesengl]

#358 got merged.

In order to make wicked_pdf compatible with view_component please set the config option render_monkey_patch_enabled to false in use render_component instead of render for rendering view-components. @ocarreterom could you please confirm that this resolves the problem so we can close this issue?

[ocarreterom]

Currently I have stopped using wicked_pdf.
Feel free to close this issue if you consider that was resolved.

Thank for your dedication! ❤️

[joelhawksley]

I consider this issue resolved. Thanks for your hard work and dedication @johannesengl!

[fsateler]

For the record, I'm currently using (not in production yet, migration to ViewComponent is in progress) the following WickedPdf monkey patch to avoid having to use render_component. This way I won't have to do a massive search and replace when rails 6.1 arrives.

class WickedPdf
  # Wicked Pdf magic breaks ViewComponent
  # https://github.com/mileszs/wicked_pdf/pull/925
  module PdfHelper
    def render(*args)
      options = args.first
      if options.is_a?(Hash) && options.key?(:pdf)
        render_with_wicked_pdf(options)
      else
        super
      end
    end

    def render_to_string(*args)
      options = args.first
      if options.is_a?(Hash) && options.key?(:pdf)
        render_to_string_with_wicked_pdf(options)
      else
        super
      end
    end
  end
end