yWorks / svg2pdf.js

A javascript-only SVG to PDF conversion utility that runs in the browser. Brought to you by yWorks - the diagramming experts
MIT License
654 stars 101 forks source link

Building the the library fails #91

Closed Fadiabb closed 5 years ago

Fadiabb commented 5 years ago

After check out the repository, i used the instructions to build the minified files with yarn and npm: npm run build / yarn build throws the following Error: Error: Cannot find module 'SvgPath' from '/home/fabbud/svg2pdf.js/src' at /home/fabbud/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:46:17

execute yarn browserify:svgpath / npm run browserify:svgpath initialize the SvgPath $ browserify node_modules/svgpath/index.js -s SvgPath -o src/SvgPath.js Done in 1.20s. then yarn run browserify or yarn build will give the same above error.

yarn version v1.16.0

HackbrettXXX commented 5 years ago

I cannot reproduce this. I created a fresh clone, ran npm/yarn run build and everything works fine. Did you maybe forget to run npm install/yarn after cloning?

bernhardreiter commented 5 years ago

@HackbrettXXX thanks for checking.

We did an npm install or yarn before running the build. I'm also investigating.

This failed on an Ubuntu 18.04.2 LTS system with node.js coming from the original packages. My next idea is looking into browserify and an elder nodejs version problems. Otherwise we just put in the patches into the current build version for testing #87.

HackbrettXXX commented 5 years ago

I tested on a Windows 10 machine. Maybe the npm scripts don't run properly in Ubuntu? Could you check if the individual scripts work or if manually entering the commands makes a difference?

bernhardreiter commented 5 years ago
node node_modules/browserify/bin/cmd.js node_modules/svgpath/index.js -s SvgPath -o src/SvgPath.js

works, but the next one fails

node node_modules/browserify/bin/cmd.js src/svg2pdf.js --debug -p licensify -s svg2pdf -o dist/svg2pdf.js

Error: Cannot find module 'SvgPath' from '/home/ber/svg2pdf.js/src'
    at /home/ber/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:46:17
    at process (/home/ber/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:173:43)
    at ondir (/home/ber/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:188:17)
    at load (/home/ber/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:69:43)
    at onex (/home/ber/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:92:31)
    at /home/ber/svg2pdf.js/node_modules/browser-resolve/node_modules/resolve/lib/async.js:22:47
    at FSReqWrap.oncomplete (fs.js:154:21)

Now tested on Debian Buster with nodejs Version: 10.15.2~dfsg-2.

HackbrettXXX commented 5 years ago

Is SvgPath in node_modules? If yes, this seems to be an issue of Browserify or Node. I'm closing this issue. If you still think it's an issue with this repo, we can reopen the issue.

bernhardreiter commented 5 years ago

Found a solution to the problem:

diff --git a/src/svg2pdf.js b/src/svg2pdf.js
index 56e1276..f792422 100644
--- a/src/svg2pdf.js
+++ b/src/svg2pdf.js
@@ -2325,7 +2325,7 @@ SOFTWARE.
     });
   } else if (typeof module !== "undefined" && module.exports) {
     RGBColor = require("./rgbcolor.js");
-    SvgPath = require("SvgPath");
+    SvgPath = require("svgpath");
     FontFamily = require("font-family-papandreou");
     cssEsc = require("cssesc");
     module.exports = svg2pdf;

It is likely that the file system @HackbrettXXX uses is case-insensitive, but if you are using a case-sensitive file system SvgPath does not match. See the instructions at https://github.com/fontello/svgpath/blob/master/README.md were svgpath is lower-case.

yGuy commented 5 years ago

A @bernhardreiter - you beat me! Thanks for your help!

HackbrettXXX commented 5 years ago

We should probably change the casing in the amd define call, as well.

bernhardreiter commented 5 years ago

@yGuy no problem, thanks for svg2pdf in the first place!

@HackbrettXXX good point, I've updated the pull-request #92 accordingly. (Though not tested, because I haven't checking how to run this code path.)