Skip to content
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

Use Prawn::Document.extensions in favor of custom plug-in loading #31

Merged
merged 3 commits into from
Jun 27, 2018
Merged

Use Prawn::Document.extensions in favor of custom plug-in loading #31

merged 3 commits into from
Jun 27, 2018

Conversation

gaffneyc
Copy link
Contributor

This is an alternative fix to #28 and a replacement of #29. The initial issue was that, when using Prawn plug-ins (e.g. prawn-emoji or prawn-svg), the extension methods would not be available inside of the Rails view. The root cause appears to be how Prawn handles plug-ins. When a plug-in is loaded it adds a module to Prawn::Document.extensions. When a new document is created it iterates over the list of extensions and extends the new object with each. The problem is that, when Prawn::Document is extended, it copies the list of loaded extensions into the subclass but any plug-ins loaded afterward are not added to the extending class.

The fix was to delegate class to PrawnRails::Document.extensions to Prawn::Document.extensions so it's always referencing the correct list.

Fixes mogest/prawn-svg#102

When used within a Rails app and in the context of Bundler, this code
boiled down to only loading the `prawn-table` plug-in. Any other plug-in
that is included in the Gemfile would automatically be loaded by
Bundler before the loading code was called.

For more rationale see my comment here:
#29 (comment)
This was added in 8cbea49 as a quality of life improvement and to
better match Rails's behavior for erb templates (e.g. <%= %> calls to_s).

I'm adding this test as a reminder of why the PrawnRails::Document class
exists: to provide an API adapter to Prawn::Document.
This fixes loading order issues where Prawn plug-ins are loaded after
prawn-rails and removes the need for the custom loading code.

Let's say you have prawn-rails and prawn-svg in your application. If
prawn-rails was loaded before prawn-svg, then the `svg` extension would
not be available for PrawnRails::Documents. This was caused by
PrawnRails::Documents extending a version of `Prawn::Documents` that did
not have the extension in Prawn::Document.extensions. When a subclass
extends Prawn::Document, it is assigned all of the current extensions
but no new ones are ever added. By delegating to Prawn::Document we can
be sure to always have the full set of extensions as they're applied
when a new Document is instantiated.
@westonganger westonganger merged commit a3c7b53 into cortiz:master Jun 27, 2018
@westonganger
Copy link
Collaborator

Thanks for this PR. This seems much more appropriate.

v1.2.0 has been released with this fix.

@gaffneyc
Copy link
Contributor Author

Glad to be of help, thank you for being so responsive and helpful

@gaffneyc gaffneyc deleted the remove-loading branch June 28, 2018 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Non-standard prawn extension inclusion causing issues with prawn-rails update
2 participants