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

PDF export does not work when using ES6 modules import #59

Closed fskpf closed 5 years ago

fskpf commented 5 years ago

Importing jspdf-yworks and svg2pdf.js as ES6 modules does not work. The jsPDF instance is missing functionality and calling svg2pdf on it will result in an excpetion:

TypeError: o.advancedAPI is not a function

However, when loading the very same svg2pdf and jspdf files via script-tags (directly from /node_modules/), the export works as expected. Thus I assume there is something broken with the ES6 import.

I've attached a sample project with webpack and ES6 modules import. To reproduce run

npm install
npm run build

and open /dist/index.html. Then check the DevTools for the exception: es6-modules-test.zip

yGuy commented 5 years ago

It really depends on how the functionality is loaded:

https://github.com/MrRio/jsPDF/issues/783#issuecomment-241316314

fskpf commented 5 years ago

Doesn't work with either approach in the test case.

HackbrettXXX commented 5 years ago

Hhm, this is interesting, for some reason, jsPDF gets loaded as RGBColor...

yGuy commented 5 years ago

Isnt't this the same problem as in https://github.com/yWorks/jsPDF/issues/12

HackbrettXXX commented 5 years ago

Kind of similar, yes ;)

I'm currently investigating it. Sadly, removing the named defines is not enough. Currently, it works for es6, but no longer for AMD.

yGuy commented 5 years ago

To fix this once and for all, the build should be adjusted to get rid of the multiple (named or unnamed) defines. However using "import" on the original sources might be an option. The minified sources are not ES6 compatible but are meant to be script-imported (or maybe AMD). What loader are you using that doesn't work? Is it webpack or browserify or is that the browser ES6 loader implementation?

HackbrettXXX commented 5 years ago

For the es6 imports I use Webpack like with @fskpf's sample project. This works now when getting rid of

However, with the new build script from MrRio, the AMD require doesn't work properly anymore:

require([
    'svg2pdf.js/dist/svg2pdf.min.js',
    'jsPDF/dist/jspdf.min.js'
  ], function (svg2pdf, jsPDF) {
  // here jsPDF is an object with "default", "rewire" and "restore" properties
  // "default" is jsPDF
}

I suppose the reason is the generated UMD header, that uses the AMD -> CommonJS wrapper:

define(["exports"], function(exports) {
  // ...
  exports.default = jsPDF;
  var _default2 = exports.default;
  function rewire($stub) {
    exports.default = $stub;
  }
  function restore() {
    exports.default = _default2;
  }

  exports.rewire = rewire;
  exports.restore = restore;

  Object.defineProperty(exports, '__esModule', { value: true });
})
HackbrettXXX commented 5 years ago

Using import/export for the original sources will not really work, since we bundle some npm modules which use different ways of exporting their modules.

We could only replace those during the build step.

yGuy commented 5 years ago

So the "new build script" broke it again? Which issue or commit is that?

HackbrettXXX commented 5 years ago

On our side: https://github.com/yWorks/jsPDF/commit/21df1eb41f0c5f90ea513da582e0adb52bbf7784

HackbrettXXX commented 5 years ago

On their side: https://github.com/MrRio/jsPDF/commit/b6b175a1a4c116a62972099eb00c05914e1a2b82 https://github.com/MrRio/jsPDF/commit/48ca33c6b04094f355ec3a4c2185727ccad3e683 https://github.com/MrRio/jsPDF/commit/ebe19ad52bca0826c1f8aadbf6de24e196d1e7ec

yGuy commented 5 years ago

That looks like a huge mess to me. However the "node" part might be interesting. They are building a node compliant version that might work in an ES6 import case.

HackbrettXXX commented 5 years ago

The node version doesn't even run on node :D I had to exclude the promise polyfill first (uses window)

HackbrettXXX commented 5 years ago

Ok, so I guess we have three options:

yGuy commented 5 years ago

Do what suits you the most - the node versions would not work for us anyway with the SVG DOM not being available on node, as far as I can tell. If fixing is simple, go ahead. If you want to redo the thing using a more modern approach (I guess this is what they tried), feel free to write a new one. Maybe @fskpf wants to help?

HackbrettXXX commented 5 years ago

Fixed it now for es6: https://github.com/yWorks/jsPDF/commit/2f18d0fff6df54acc046cc6e54e96f707bf7d5f6

For AMD, it seems to be necessary to insert something like:

jsPDF = jsPDF['default']

Seems to be the drawback, when es6 support is also wished.

micschro commented 5 years ago

Yes - as soon as both default and named exports are used, rollup will even warn about this:

(!) Mixing named and default exports
Consumers of your bundle will have to use bundle['default'] to access the default export, which may not be what you want. Use `output.exports: 'named'` to disable this warning
wxfred commented 5 years ago

Please support ES6 modules import, thx.

GrayYoung commented 5 years ago

Fixed it now for es6: yWorks/jsPDF@2f18d0f

For AMD, it seems to be necessary to insert something like:

jsPDF = jsPDF['default']

Seems to be the drawback, when es6 support is also wished.

Did not release a version.

npm ERR! notarget No matching version found for jspdf@2.0.0

HackbrettXXX commented 5 years ago

You installed the wrong jsPDF:

npm install jspdf-yworks
HackbrettXXX commented 5 years ago

The fix for es6 imports will come with 2.0.1