search

alafr / SVG-to-PDFKit

Insert SVG into a PDF document created with PDFKit
MIT License
397 stars 111 forks source link

Fix pattern support possibly broken by minification #168

Closed perliedman closed 2 years ago

perliedman commented 2 years ago

As noted in #137, patterns are sometimes not correctly converted from SVG to PDF: in these cases, patterned areas turn into black solid fill. The issue occurs for example when using production mode from a Create React App configuration.

Digging into this, I found that the generation of PDF patterns relies on a string comparison with the constructor name: https://github.com/alafr/SVG-to-PDFKit/blob/d339cecbbee6697c94f6148dc44f21a958493268/source.js#L212

But the constructor in question belongs to a private type inside another function: https://github.com/alafr/SVG-to-PDFKit/blob/d339cecbbee6697c94f6148dc44f21a958493268/source.js#L117

At least some minifiers are smart enough to realize this name could be removed, causing the constructor name comparison to fail.

I have addressed this by replacing the private class/constructor with a simple object and a type property that I set to PDFPattern. This resolves the issue and I think it works equally well.

If types/classes are preferred, I guess we could make PDFPattern a class known throughout the library and replace the constructor name check with an instanceof check.

perliedman commented 2 years ago

Did not realize until after I posted this that #159 seems to address the same issue (but with a bit of a complicated diff...).

mickey58github commented 2 years ago

@perliedman Hi Per, I'm facing this problem in our app. Strangely, it works fine in the dev build, but shows these black fills with the production build, on Heroku cloud. Saw another issue that says it has to do with minified js being used in production...

The problem affects rectangles that use pattern fills through style = "fill: url(#my-pattern-id)". Those are rendered black using the production build.

Can you provide a fix for this issue? It is a blocker for the usage of pdfSave in our app.

perliedman commented 2 years ago

@mickey58github this PR is a fix to the issue, so you can fix it by applying this patch.

For my personal projects, I currently refer to my GitHub fork of the project like this while waiting for this PR to be merged: https://github.com/perliedman/o-scout/blob/master/package.json#L27 - it's an easy fix, but don't do it for anything really important :)

mickey58github commented 2 years ago

@perliedman - Per, thanks for responding. I'm a bit afraid of forking my own version. I need reliable PDF generation.

Meanwhile, I worked around the problem by moving from PDFSave to PDFKitTable - though that required me to move my PDF generation code from browser-side to Node server-side. So far, that works fine, but I haven't done complex PDFs yet with it.

mickey58github commented 2 years ago

@perliedman - Per, sorry for my ignorance, but I'm unsure how to leverage this fix. It looks like it is a fix to svg-to-pdfkit, but my package.json (for my client/browser side code which has the PDFsave "black rectangles" pattern problem) shows only "pdfmake": "^0.2.5", no svg-to-pdfkit.

My package.json on the server, which uses PDFKit, not PDFSave, shows: pdfkit-table": "^0.1.99", "svg-to-pdfkit": "^0.1.8" - but that is unrelated to the PDFSave "black rectangles" pattern problem....

grumd commented 2 years ago

Do you guys know if this is going to be released to npm? Last release was 3 years ago.

mickey58github commented 2 years ago

Do you guys know if this is going to be released to npm? Last release was 3 years ago.

You're right, it seems this stuff lives on Github only, while I assumed I get a fixed version through NPM.