yWorks / jsPDF

JavaScript PDF generation to work with SVG
MIT License
56 stars 23 forks source link

Mismatch anonymous defines - problems when bundled with requirejs #12

Closed pekkakyllonen closed 6 years ago

pekkakyllonen commented 6 years ago

I'm encountering such an error there when the build is minimized/combined into main.js, with version 1.2.70 and also newer version 1.3.2. After googling a bit it appears to be a problem in requirejs with (multiple) anonymous defines and after some searching I tried the following hack to the very start of the jspdf.min/debug.js :

typeof define === 'function' && define.amd ? define(factory) : ... => typeof define === 'function' && define.amd ? define('jspdf',factory) : ...

That caused the "mismatch" go away. I was wondering if others have had similar problems. (I noticed there were similar issue on the main project side and some changes done related but to other locations. I don't know of the need to include the changes everywhere if this was an issue with outermost define being anonymous...?) Perhaps there would be plans to include such changes to this fork too?

(Sorry for not including links or files here, that was the only change needed to fix the issue)

HackbrettXXX commented 6 years ago

Could you give us more information, please?

pekkakyllonen commented 6 years ago

We use jspdf for the creating offline (local) exporting / printing with highcharts libraries - i.e. the highcharts export service is not used for creation of pdf. Packages are bundled together (around 50) and only production build is causing problem (bundling+minify). Dev build works ok. Packages are listed in main.js require.config.

I'll look to list some relevant posts/commits.

yGuy commented 6 years ago

only production build is causing problem (bundling+minify)

I'd say this indicates a clear bug in the bundler/minifier - the bundler/minifier ideally should not have an influence on the correctness of the code, but obviously it has.

If you still think this is a problem with our jsPDF fork, please provide a complete self-contained minimized repro. Thanks!

pekkakyllonen commented 6 years ago

It's obviously a requireJS problem. There seems to be plenty of similar cases around. I'm trying to find a workaround for this one.

yGuy commented 6 years ago

Yes - the original JsPdf sources are problematic because they contain multiple define statements that simply don't work in a requireJS environment. RequireJS won't tell you about that, though, unfortunately.

pekkakyllonen commented 6 years ago

Sorry for the trouble and thank you for your time! I was having some wild ideas about making a fork/branch where would go/stab around that problem within the dist/jspdf.min.js but appears we could be having another solution soon. Let's see how it goes.

HackbrettXXX commented 6 years ago

I'm gonna have a look at the changes of https://github.com/MrRio/jsPDF/pull/1646 and apply them to our fork.

HackbrettXXX commented 6 years ago

@pekkakyllonen could you check if the changes I made fix your problem? See this branch: https://github.com/yWorks/jsPDF/tree/anonymous-defines

pekkakyllonen commented 6 years ago

I'll have a look at it as soon as I find the time. Thanks!

mbrodala commented 6 years ago

@yGuy Can you have a look at this? We also try to use this library with the offline export of Highcharts but as soon as we load jsPDF everything breaks in rather obscure ways.

HackbrettXXX commented 6 years ago

We need more details in order to help you. What error messages do you get? Which version of svg2pdf/jsPDF do you use? How do you load the libraries? Can you provide an minimal repro?

mbrodala commented 6 years ago

The error message I currently get is this:

exporting.src.js:49 Uncaught TypeError: Cannot read property 'navigator' of undefined
    at exporting.src.js:49
    at exporting.src.js:21
    at export.js:1
    at Object.execCb (require.min.js:1)
    at e.check (require.min.js:1)
    at e.<anonymous> (require.min.js:1)
    at require.min.js:1
    at require.min.js:1
    at each (require.min.js:1)
    at emit (require.min.js:1)

For some odd reason jsPDF is loaded instead of highcharts which breaks everything. If I don't load pdfJS, everything works fine. (Except PDF export of course.)

We load everything via RequireJS and currently try to bundle all export-related logic in a custom module which basically looks like this:

define(
  ['highcharts', 'highcharts-exporting', 'highcharts-offline-exporting', 'jspdf', 'svg2pdf'],
  function(Highcharts, HighchartsExporting, HighchartsOfflineExporting, jsPDF, svg2pdf) {
  // Highcharts.win.jsPDF = jsPDF;
  // Highcharts.win.svg2pdf = svg2pdf;
  HighchartsExporting(Highcharts);
  HighchartsOfflineExporting(Highcharts);

  return {
    options: {
    },
  }
});

Here's a pen showing the issue.

HackbrettXXX commented 6 years ago

This in fact seems to be an issue of jsPDF. When I use the version of the branch with the anonymous defines fix (https://github.com/yWorks/jsPDF/tree/anonymous-defines), it works.

As @pekkakyllonen did not report if the fix works for him, I'm gonna take @mbrodala's case now as a proof and merge it into master.

As a quick workaround for you, you can check out the version of the fix branch until our next release.

Hope, this fixes your issue.

mbrodala commented 6 years ago

@HackbrettXXX thanks, this indeed looks better now. Looking forward to the next release. :-)

Fredo70 commented 5 years ago

I updated highcharts to 7.1.1 which provide version 2.0.0 of jsPDF (https://code.highcharts.com/7.1.1/lib/jspdf.js and get exactly this problem. Sometime it works, mainly it doesn't. Get either mismatched error or the callback references of other modules are messed up.

yGuy commented 5 years ago

Please provide a jsfiddle or pen or whatever repro that shows that this is still an issue. The one in https://github.com/yWorks/jsPDF/issues/12#issuecomment-425045680 seemed to work.

Fredo70 commented 5 years ago

The difficulty is that sometimes it works. You have to run again and again and there is an exception.

https://stackoverflow.com/questions/55920688/highcharts-version-7-1-1-how-to-offline-exporting-pdf-with-amd-modules

https://jsfiddle.net/zbc149wh/2/

I solve the problem by set a name to all define in the file dist/jspdf.debug.js. Then it always works.

But I am sorry that I am unable to correct this error in the Project. My knowledge of the whole structure is limited.

yGuy commented 5 years ago

Thanks for the fiddle. Can you simplify the repro to make this a true test case. Most importantly, please don't use the highcharts version of the library, but use our official release. Otherwise I would argue that you should file the issue against highcharts. Thank you for your help!

yGuy commented 5 years ago

It looks as if this issue should have been fixed in this changeset quite some time ago - maybe a later merge broke the workaround? https://github.com/yWorks/jsPDF/commit/4eb63c49e4370a36594bb29dc742c264fc53d8a3

HackbrettXXX commented 5 years ago

That seems to be the case.