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

jsPDF AMD version doesn't compile Adler32 dependancy correctly #122

Closed
cwohlman opened this issue Jul 15, 2013 · 58 comments
Closed

jsPDF AMD version doesn't compile Adler32 dependancy correctly #122

cwohlman opened this issue Jul 15, 2013 · 58 comments

Comments

@cwohlman
Copy link

When using requireJs to import jspdf.amd.min.js the imported object didn't look anything like jspdf.

It turns out that the build script must be simply wrapping the adler32 module inside of the jspdf module. The adler32 module contains a define call which gets called before the define call for jspdf and gets used in the place of the jspdf define call.

@zawzawzaw
Copy link

Ye I am having same problem here!

@khornberg
Copy link

Same here. I get a TypeError: object is not a function

@skkap
Copy link

skkap commented Jan 5, 2014

Same problem, unable to use lib with Requirejs.

@diegocr diegocr closed this as completed in e6150e8 Feb 6, 2014
@danijar
Copy link

danijar commented Mar 3, 2014

How can I use jsPDF with RequireJS? The jspdf.amd.min.js from release v0.9.0 doesn't work. Instead, an object of this signature is injected.

Object {Adler32: function, from: function, fromUtf8: function, fromBuffer: function}

In the newest commit however, the is no jspdf.amd.min.js file at all. Using the jspdf.min.js results in the same error above.

@pascalPimaia
Copy link

I was expecting to use jspdf with require js and I felt a bit disappointed not to be able to find the amd version of the module in dist folder
any fix in mind ?

@danijar
Copy link

danijar commented Mar 4, 2014

I builded the library using the provided build script, but the newest commit does't seem to produce the AMD version anymore. I only got jspdf.source.js and jspdf.min.js.

@diegocr
Copy link
Collaborator

diegocr commented Mar 4, 2014

Hi there, I found the only difference between the AMD and MIN files/versions was a call to define(function() {return jsPDF}) so i got rid of that AMD file and added it to the MIN one. However, if that didn't worked on 0.9.0 it's obvious it'll continue to fail on the new min.

I'll try to fix that when i get a chance.

@diegocr diegocr reopened this Mar 4, 2014
@diegocr diegocr closed this as completed in 0ce4e39 Mar 4, 2014
@diegocr
Copy link
Collaborator

diegocr commented Mar 4, 2014

Please test the new min file and let me know if that fixed the problem, which i think it should. However, i don't use requirejs so i can't be 100% sure...

@danijar
Copy link

danijar commented Mar 5, 2014

Thanks for your effort. There are two issue with your update. First, the global instance jsPDF leaks into the application. In contrast, the variable should be undefined. Second, when requiring the library an object with only one property called jsPDF which holds the document constructor gets injected. Instead, the document constructor should be injected directly.

define(['libs/jspdf.min'], function(Pdf) {
    return {
        initialize: function() {
            if (jsPDF)
                console.error('Global instance of jsPDF leaked into application');
            console.log(Pdf);
        },
    };
});

This is the console output of the example.

> Global instance of jsPDF leaked into application
v Object {jsPDF: function}
    > jsPDF: function jsPDF(orientation, unit, format, compressPdf) {
    > __proto__: Object

The condition should be false and the the library should be injected directly. The correct output might just look like this then.

> jsPDF: function jsPDF(orientation, unit, format, compressPdf) {

@diegocr
Copy link
Collaborator

diegocr commented Mar 5, 2014

Ok, thanks for the info. I'm somewhat confused, though. On 0.9.0 the constructor was being returned as you suggest it needs to do, however... you reported that version as broken, too.

Hmm, is there some conflict with Adler32? Ie, their call to define() misuses ours?

How about if you submit a PR fixing that yourself? :)

@diegocr diegocr reopened this Mar 5, 2014
@danijar
Copy link

danijar commented Mar 11, 2014

Okay, I'll try to come up with a fix.

@airandfingers
Copy link

We use RequireJS, and our jsPDF fromHTML method fails most of the time, with one of two errors:

  • Object has no method fromHTML (from our call to pdf.fromHTML)
    • This error occurs most frequently, and I've confirmed via breakpoints that jspdf.plugin.from_html.js:576 is hit before our code is run
  • Cannot read property 'widths' of undefined (from jspdf.plugin.from_html.js:169)

Is there a particular order in which we must load the jsPDF files? Currently, we're loading jspdf.js first, followed by jspdf.source.js and all plugins.

@danijar
Copy link

danijar commented Mar 18, 2014

@airandfingers It should be enough to load jspdf.min.js from the dist folder.

@diegocr What file should I edit for the fix? I think I could fix the distribution file, but the changes would be overwritten by the next build then.

@airandfingers
Copy link

@danijar Since you're the one who first posted the error I'm having (#122 (comment)), I'm assuming you mean that it should be enough to load jspdf.min.js, but that this won't work right now - thus, this thread.

@airandfingers
Copy link

I'd like to point out that this wouldn't be much of an issue for us if other versions of jsPDF were available from Bower; as it is, only the latest version of jsPDF - 0.9.0 - is there, so that a downgrade using Bower is not possible.

@pascalPimaia
Copy link

For those of you willing to use jspdf with require here is what I did w/o the AMD version of the library (replace ./frameworks/jspdf/ by the appropriate configuration path)

require.config({
    paths : {
        "jspdf" : "./frameworks/jspdf/jspdf.source"
    },

    shim : {

        "jspdf" : {
            exports : "jsPDF"
        }
    }
});

// jsPDF can be used as following into your application code:
define(["jspdf"], function (jsPDF) {
    "use strict";
    console.log('jsPDF', jsPDF);
});

@airandfingers
Copy link

@pascalPimaia That's exactly what I tried; it didn't work, as with this code, jsPDF is set to the object @danijar printed above it #122 (comment):

Object {Adler32: function, from: function, fromUtf8: function, fromBuffer: function}

Are you using the latest version of jsPDF, 0.9.0?

@danijar
Copy link

danijar commented Mar 19, 2014

First, it shouldn't be an issue to use prior versions since Git keeps them all. I built the library from source using the provided build script. It depends on two other scripts, but they should be easy to provide. This version works for me by including dist/jspdf.min.js and using the shim option as shown by @pascalPimaia. It's not perfect though, because also the global jsPDF object always leaks into your modules, regardless of how you call your parameter.

Maybe someone familiar with the source of this library can guide me to the internal file that I have to apply the changes to, so that further builds would properly work with Require.js. @MrRio, @diegocr?

@airandfingers
Copy link

Our build process currently installs jsPDF from bower, from which only 0.9.0 is available.

It sounds like we should stop using bower for jsPDF, and manually build from source, as @danijar described. I'll try this next.

@pascalPimaia
Copy link

I use dist/jspdf.source.js but I haven't tried the minified version
for the lastest update.
Update is handled by bower
Le 19/03/2014 20:09, Aaron Yoshitake a écrit :

@pascalPimaia https://github.com/pascalPimaia That's exactly what I
tried; it didn't work, as with this code, jsPDF is set to the object
@danijar https://github.com/danijar printed above it #122 (comment)
#122 (comment):

|Object {Adler32: function, from: function, fromUtf8: function, fromBuffer: function}
|

Are you using the latest version of jsPDF, 0.9.0?


Reply to this email directly or view it on GitHub
#122 (comment).

@airandfingers
Copy link

I built from source:

...and had the same problem as above - jsPDF is an object, and "object is not a function".

@danijar
Copy link

danijar commented Mar 23, 2014

Did you use the shim feature of Require.js to export jsPDF?

@airandfingers
Copy link

Yes, I'm using Require.js exactly as described above, including the shim feature.

@cwohlman
Copy link
Author

I've added a pull request which should fix this issue, it uses a powershell build script though.

@cwohlman
Copy link
Author

Good suggestion on the modules object, I'll revert the changes I made to
jspdf.js

It's easy to think you're going nuts when requirejs starts giving errors ;)
The edge case module definitions (such as bundling requirejs and
non-requirejs modules) are not intuitive.

On Mon, Mar 24, 2014 at 5:23 PM, Diego Casorran notifications@github.comwrote:

I'm not sure if i get that, what "another define call in jspdf.source.js"
? jspdf.source.js is automatically generated from jspdf.js by build.sh, so
removing the define() call there will generate a broken .source file as
well. What i'm missing? :)

Regardless, indeed it seems there's some weird problem with the nested
define() call used by adler32, i think we can workaround that by defining a
modules object so that define() call isn't reached.

In any case, are we trying to handle two different problems at the same
time or it's just my feeling? I mean, using require() to load jsPDF works
fine as i shown on my post above, but then there's the way of loading it
through define() calls from user-code, correct? So, in such case, what are
the differences from using one or the other method?

Having no prior experience with requirejs myself i think i'm getting nuts
or something ;)


Reply to this email directly or view it on GitHubhttps://github.com//issues/122#issuecomment-38508944
.

@cwohlman
Copy link
Author

I've updated my pull request #218 and problem/solution demo page: http://cwohlman.github.io/jsPDF/examples/requirejs.html

Does any see any more changes needed?

@airandfingers
Copy link

@diegocr Thanks, I didn't see any build instructions, so I just ran build.sh without the --recursive option. The file you linked got rid of the saveAs error I pasted, but the PDFs being generated are all wrong, with pages of blank space, no text at all, and tables with rows that are each a page in height (despite having no text). What's going on here?

Also, I still have the other error, only for some PDFs:

Uncaught TypeError: Cannot read property 'length' of undefined jspdf.source.js:3817

Any idea what might cause this error? It looks like this error only occurs when I create the PDF with certain HTML values, though all the values I'm trying are div elements with headers and multi-row tables.

@diegocr
Copy link
Collaborator

diegocr commented Mar 25, 2014

@airandfingers: The recursive option is for git, build.sh takes no arguments. Well, indeed there is still something wrong in your side, because on line 3817 there's no length usage:

https://github.com/MrRio/jsPDF/blob/master/dist/jspdf.source.js#L3817

Are you sure you're using the correct jspdf.source.js file? :-/

@cwohlman i'll review next your PR replying there.

@airandfingers
Copy link

@diegocr Hm sorry, I think I pasted my error from earlier, when I was using a different jspdf.source file.

Uncaught TypeError: Cannot read property 'length' of undefined jspdf.source.js:3416

As you can see at https://github.com/MrRio/jsPDF/blob/master/dist/jspdf.source.js#L3416, this line does have .length; the line[0].length part is the one throwing the Exception.

@diegocr
Copy link
Collaborator

diegocr commented Mar 25, 2014

@airandfingers: Yeah, i see. That is a bug in one of the plugins. Can you file a separate issue so that i can easily reference it when fixing it? thanks!

@diegocr
Copy link
Collaborator

diegocr commented Mar 25, 2014

Guys, I've just uploaded new dist files which hopefully should fix the Adler32 conflict by using the jspdf.min.js file, and no variables should be leaked in the global scope. Please give it a try and let me know if there's still any issue.

Tested using:

require(['dist/jspdf.min.js'], function(jsPDF) {
    console.log('jsPDF:', jsPDF);

    var pdf = new jsPDF({
        compress: true
    });

    console.log('pdf: '+ pdf.output('datauristring').substr(0,50));
});

As you can see, i've changed it so that the jsPDF instance is returned directly rather than having to deal with objects...

https://github.com/MrRio/jsPDF/blob/master/dist/jspdf.min.js

@cwohlman
Copy link
Author

Thanks, great fix!

@danijar
Copy link

danijar commented Mar 25, 2014

@diegocr Thanks for pointing me to the right file and line. I think currently, the jsPDF variable would still leak into the global space, but I'll test that soon.

@diegocr
Copy link
Collaborator

diegocr commented Mar 25, 2014

@danijar: The whole code is wrapped into a closure, hence i don't see how it should still leak.

@cwohlman
Copy link
Author

It didn't leak in my tests.

@danijar
Copy link

danijar commented Mar 25, 2014

Nice, then the issue is solved! I must have overseen that it was wrapped into an anonymous function.

@diegocr
Copy link
Collaborator

diegocr commented Mar 25, 2014

Yeah, the min file is that way since the inclusion of the build.sh script, which uses uglifyjs's --wrap option for that.

@danijar
Copy link

danijar commented Mar 26, 2014

Ahh, I knew it! Thanks for solving the issue.

@airandfingers
Copy link

Thanks for the fix! Any idea when this will get pushed to Bower?

@diegocr
Copy link
Collaborator

diegocr commented Mar 27, 2014

@airandfingers: Probably when #184 gets done :)

@throrin19
Copy link

I have the same error in the latest build with bower : Uncaught TypeError: object is not a function :

Object {Adler32: function, from: function, fromUtf8: function, fromBuffer: function}

@diegocr
Copy link
Collaborator

diegocr commented Jun 25, 2014

@throrin19 With RequireJS you have to use the min file, not the debug one.

@dineshWizni
Copy link

fromHTML is not working in any case with requireJS , I tried everything. if you have some example working example with requireJS for fromHTML method , please share.Thanks

@christrude
Copy link

I'm using the min file, and am still getting jsPDF is undefined. Heres some snippets:

require.config.js
require.config({
baseUrl: 'app',
paths: {
'jspdf': '../vendor/jspdf.min',
'swf': '../vendor/downloadify/swfobject',
'downloadify': '../vendor/downloadify/downloadify.min',
}
});

module.js
define([
'angular',
'app',
'lodash',
'jquery',
'kbn',
'jspdf',
'jquery.flot',
'jquery.flot.pie',
], function (angular, app, _, $, kbn, jsPDF) {
'use strict';

var module = angular.module('kibana.panels.hits', []);
app.useModule(module);

module.controller('hits', function($scope, querySrv, dashboard, filterSrv) {
console.log(jsPDF);
$scope.test = function() {
var pdf = new jsPDF({compress:true});
var source = angular.element('#main-content')[0];
var margins = {
top: 80,
bottom: 60,
left: 40,
width: 522
};
pdf.addHTML(document.body, 10, 10, {
'width': margins.width
}, function(dispose){
pdf.save('IE9Dashboard' + jQuery.now() + '.pdf');
}, margins
);
}
}});

Any ideas? I've tried putting it into the shim. Been all over the place and thought this thread would show me the answer, but where everyone else seems to have it fixed, I still don't...frustrating. Arg!

@diegocr @cwohlman Please help!

@mryellow
Copy link

Bower install, Version 1.0.272-git.

Without shim.'jspdf': '../lib/jspdf/jspdf.min':

define(['durandal/app', 'knockout', 'viewmodels/settings', 'jspdf', 'ko-validation', 'ko-helpers'], function (app, ko, ModelSettings, jsPDF) {

jsPDF is undefined.

Shimmed, jspdf: { exports: "jsPDF" }, undefined.

Shimmed, jspdf: { exports: ["jsPDF"] }, value.split is not a function

@metaman
Copy link

metaman commented Jan 30, 2015

I am also getting undefined when requiring in the latest min version. The last working version I can determine is 1.0.178.

It looks like the cause of the problem is the explicit naming of the require module:

'define('jsPDF', function() {' 

If you remove the module name then the latest seems to work as well. Not sure why this is causing an issue.

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 a pull request may close this issue.